On Thu, 10 Nov 2016 11:11:51 +0100 Emilio Pozuelo Monfort <[email protected]> wrote:
> On 10/11/16 10:57, Pekka Paalanen wrote: > > From: Pekka Paalanen <[email protected]> > > > > Add tests that ensure that wayland-scanner output for a given input does > > not change unexpectedly. This makes it very easy to review > > wayland-scanner patches. > > > > Before, when patches were proposed for wayland-scanner, I had to > > build wayland without the patches, save the generated files into a > > temporary directory, apply the patches, build again, and diff the old > > vs. new generated file. > > > > No more. Now whenever someone makes intentional changes to > > wayland-scanner's output, he will also have to patch the example output > > files to match. That means that reviewers see the diff of the generated > > files straight from the patch itself. Verifying the diff is true is as > > easy as 'make check'. > > > > The tests use separate example XML files instead of wayland.xml > > directly, so that wayland.xml can be updated without fixing scanner > > tests, avoiding the churn. > > > > example.xml starts as a copy of wayland.xml. If wayland.xml starts using > > new wayland-scanner features, they should be copied into example.xml > > again to be covered by the tests. > > > > This patch relies on the previous patch to actually add all the data > > files for input and reference output. > > > > The scanner output is fed through sed to remove parts that are allowed > > to vary: the scanner version string. > > > > Task: https://phabricator.freedesktop.org/T3313 > > Signed-off-by: Pekka Paalanen <[email protected]> > > --- > > .gitignore | 1 + > > Makefile.am | 34 +++++++++++++++++++++++++++++++++- > > tests/scanner-test.sh | 51 > > +++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 85 insertions(+), 1 deletion(-) > > create mode 100755 tests/scanner-test.sh > > > > diff --git a/.gitignore b/.gitignore > > index 33e809c..8da9861 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -38,6 +38,7 @@ ctags > > /missing > > /stamp-h1 > > /test-driver > > +/tests/output/ > > Makefile > > Makefile.in > > exec-fd-leak-checker > > diff --git a/Makefile.am b/Makefile.am > > index d35231c..cd21915 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -168,7 +168,15 @@ if ENABLE_CPP_TEST > > built_test_programs += cpp-compile-test > > endif > > > > -TESTS = $(built_test_programs) > > +AM_TESTS_ENVIRONMENT = > > \ > > + export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner' \ > > + TEST_DATA_DIR='$(top_srcdir)/tests/data' \ > > + TEST_OUTPUT_DIR='$(top_builddir)/tests/output' \ > > + SED=$(SED) \ > > + ; > > + > > +TESTS = $(built_test_programs) \ > > + tests/scanner-test.sh > > > > check_PROGRAMS = \ > > $(built_test_programs) \ > > @@ -245,4 +253,28 @@ os_wrappers_test_LDADD = libtest-runner.la > > > > exec_fd_leak_checker_SOURCES = tests/exec-fd-leak-checker.c > > exec_fd_leak_checker_LDADD = libtest-runner.la > > + > > +scanner_test_data_files = \ > > + $(top_srcdir)/tests/data/example.xml \ > > + $(top_srcdir)/tests/data/example-client.h \ > > + $(top_srcdir)/tests/data/example-server.h \ > > + $(top_srcdir)/tests/data/example-code.c \ > > + $(top_srcdir)/tests/data/small.xml \ > > + $(top_srcdir)/tests/data/small-code.c \ > > + $(top_srcdir)/tests/data/small-client.h \ > > + $(top_srcdir)/tests/data/small-server.h \ > > + $(top_srcdir)/tests/data/small-code-core.c \ > > + $(top_srcdir)/tests/data/small-client-core.h \ > > + $(top_srcdir)/tests/data/small-server-core.h > > + > > +tests/scanner-test.sh: \ > > + $(top_builddir)/wayland-scanner \ > > + $(scanner_test_data_files) > > Dependencies should be in the same line. Otherwise it's easy to confuse them > for > rules. Hmm, yeah, a different indentation scheme from rules would be in place indeed. I changed it to this: tests/scanner-test.sh: $(top_builddir)/wayland-scanner \ $(scanner_test_data_files) > Other than that, the whole series looks good to me and is: > > Reviewed-by: Emilio Pozuelo Monfort <[email protected]> Thanks, pq
pgpILtYXbF8dc.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
