On Thu, 10 Nov 2016 12:18:55 +0200 Pekka Paalanen <[email protected]> wrote:
> 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) Wait... why did I put scanner_test_data_files as scanner-test.sh dependencies... scanner-test.sh is a script, it's not built from these. The data files are not built either. The script runs unconditionally on 'make check'. So, no need to have the data files as deps, AFAIU. However, the dependency to wayland-scanner is needed, because the script wants to run it, and it gets built. It should be enough to put the data files just in EXTRA_DIST and without $(top_srcdir). Right? Thanks, pq
pgpt4Dl3Z6aLw.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
