[PATCH v1 weston 05/11] tests: Allow tests to use customized command line parameters

[Bryce Harrington]

On Fri, Nov 21, 2014 at 04:56:03PM +0200, Pekka Paalanen wrote:
> On Wed, 19 Nov 2014 15:06:20 -0800
> Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
> > From: Derek Foreman <derekf at osg.samsung.com>
> > 
> > Tests will now return the extra command line parameters they need
> > when run with --params
> > 
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > ---
> >  tests/weston-test-runner.c | 8 ++++++++
> >  tests/weston-tests-env     | 1 +
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
> > index ef45bae..ce0a670 100644
> > --- a/tests/weston-test-runner.c
> > +++ b/tests/weston-test-runner.c
> > @@ -34,6 +34,8 @@
> >  
> >  #define SKIP 77
> >  
> > +char __attribute__((weak)) *server_parameters="";
> > +
> >  extern const struct weston_test __start_test_section, __stop_test_section;
> >  
> >  static const struct weston_test *
> > @@ -154,6 +156,12 @@ int main(int argc, char *argv[])
> >  			exit(EXIT_SUCCESS);
> >  		}
> >  
> > +		if (strcmp(testname, "--params") == 0 ||
> > +		    strcmp(testname, "-p") == 0) {
> > +			printf("%s", server_parameters);
> > +			exit(EXIT_SUCCESS);
> > +		}
> > +
> >  		t = find_test(argv[1]);
> >  		if (t == NULL) {
> >  			fprintf(stderr, "unknown test: \"%s\"\n", argv[1]);
> > diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> > index e332354..aaf3ee1 100755
> > --- a/tests/weston-tests-env
> > +++ b/tests/weston-tests-env
> > @@ -46,5 +46,6 @@ case $TESTNAME in
> >  			--shell=$SHELL_PLUGIN \
> >  			--log="$SERVERLOG" \
> >  			--modules=$TEST_PLUGIN,$XWAYLAND_PLUGIN \
> > +			$($abs_builddir/$TESTNAME --params) \
> >  			&> "$OUTLOG"
> >  esac
> 
> Hi,
> 
> this is pretty clever. I do wonder though if we are going to need a
> test-specific weston.ini at some point, too. If we do, do we also need
> to control command line options? Well, apart from specifying the config
> file location.

There are three different external, direct mechanisms for adjusting
weston's behavior:

  * command line options and parameters
  * weston.ini config file
  * environment variables

Now, afaik we don't have any policies that one of these should be
preferred and/or a superset of the others, so presumably future tests
are going to want to test settings against any of the three mechanisms.
Which means we will need to enable tests to have control over server
command line options, environment variables, and weston.ini.

If we were to set as a policy that, say, all command line options (and
maybe environment variables) must have an equivalent setting in
weston.ini, then we could provide only that interface inside tests.
This would simplify things from a testing perspective, but it would
require the rest of the project to adhere to this constraint.

OTOH, we could take the inverse route, and add a command line option
that allows setting/overriding any arbitrary parameter that can be set
in weston.ini.  (We took this approach in Inkscape with a --verb
command, that ended up handy both for testing and for other mechanized
uses of Inkscape.)

With respect to the environment variables, fortunately there's not too
many of them so far.  I've started compiling a list and documenting what
they do.
 
> While this patch is fine for now, we are probably going to need
> something more flexible soon. Currently we only run tests on a single
> backend at a time, and a single shell at a time. There will become time
> when we need to test different shell plugins. Oh hey, but this could
> also work for shell plugins, as tests would likely be shell-specific.
> 
> It might also be nice to have a bit of heuristics and by default run on
> all backends that might work:
> - always headless with each renderer
> - if DISPLAY is set, x11-backend
> - if WAYLAND_DISPLAY is set, wayland-backend
> - if neither DISPLAY nor WAYLAND_DISPLAY set, drm-backend, if DRM
>   available
> Hm, or maybe that doesn't make much sense... but being able to list
> which all backends should run through all (the specified) tests would
> be nice.

So like a --list-backends?  That does sound handy.

Then, an automated test could iterate over those and then just call with
--backend=XXX as needed, and can do whatever logic it wants if a given
backend is missing.

For an individual tester, usually the preference is for it to run
quickly, and with as little fiddling as possible.  I imagine they
probably only care about one backend at a time anyway, so in this case I
think you're right that running against more than one may not be the
desired behavior.  For this use case heuristics should probably be used
only insofar as to figure that out when they're not stating it
explicitly.  And they'd be able to utilize --list-backends to help
sleuth out why their desired backend isn't available.

> Oh well, random thoughts for the future, nothing to complain about
> this now. Anyway, I started to look at this series and got this far this
> week, I wanted to say. Looks good so far. :-)
> 
> I figured I want to merge this series first, so we have some tests
> in place when I look at the matrix rewrite series, since that has some
> potential to break some rendering.

Nice, sounds good.

Bryce