[PATCH wayland 3/3] tests: add scanner tests

[Pekka Paalanen]

On Thu, 10 Nov 2016 11:11:51 +0100
Emilio Pozuelo Monfort <pochu27 at gmail.com> wrote:

> On 10/11/16 10:57, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > 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 <pekka.paalanen at collabora.co.uk>
> > ---
> >  .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 <emilio.pozuelo at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161110/fce9568c/attachment.sig>