On 11/11/16 15:56, Pekka Paalanen wrote: > 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.
Indeed. > 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? Yeah, that sounds right. Cheers, Emilio _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
