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

[Pekka Paalanen]

On Thu, 10 Nov 2016 12:18:55 +0200
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> 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)

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
-------------- 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/20161111/96aef5bf/attachment.sig>