[Piglit] [PATCH v2 3/5] util: Add a -list-subtests option that will list all the subtests

[Dylan Baker]

On Monday, October 28, 2013 11:38:07 AM Ian Romanick wrote:
> On 10/28/2013 10:29 AM, Kenneth Graunke wrote:
> > On 10/15/2013 05:32 PM, Ian Romanick wrote:
> >> From: Ian Romanick <ian.d.romanick at intel.com>
> >> 
> >> The utility is that all.tests can do 'sometest -list-subtests' to
> >> automatically get the list of subtests to run.  Since the syntax for
> >> selecting subtests is regularized, this works quite well.
> >> 
> >> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> >> Cc: Chad Versace <chad.versace at linux.intel.com>
> >> ---
> >> 
> >>  tests/util/piglit-framework-gl.c | 15 +++++++++++++++
> >>  tests/util/piglit-framework-gl.h |  8 ++++++++
> >>  2 files changed, 23 insertions(+)
> > 
> > Sorry for coming late to the party on this one, but...a few comments:
> > 
> > I really like the idea of having consistent --subtest <foo> and
> > --list-subtests <foo> command line options.  It would be great to have a
> > consistent -h/--help as well (which would probably list subtests, when
> > available).
> > 
> > However, I am very concerned about the plan to use --list-subtests to
> > enumerate things in all.tests.
> > 
> > As I understand your current plan, piglit-run would have to invoke the
> > binary of every test that has subtests in order to get the list of
> > tests.  As we convert more tests to use subtests (or add new ones), this
> > becomes progressively more overhead: it has to run dozens of binaries
> > before it can even start running tests.
> 
> Not necessarily.  It's necessary for this specific test because not all
> of the subtests play nice with each other in the presence of failures.
> Certain classes of (observed!) driver bugs can cause a failure in one
> test to cause all following tests to also fail.  There is also one test
> case that must be run last because it wrecks the GL state.
> 
> I think tests where subtests should be run individually are likely to be
> rare.
> 
> > That said, processing all.tests already involves trawling all over disk
> > to find .frag, .vert, .geom, and .shader_test files, which is kind of
> > slow too.  Which is one reason some of us have been thinking about
> > moving the "produce the list of tests" step to be part of the build
> > process, instead of at every piglit-run startup.
> > 
> > But if the build process involved calling --list-subtests, then we would
> > need binaries built for both the host and the target systems, which
> > makes cross-compiling a nightmare.  I'd really like to avoid that.
> > 
> > So I think I would prefer to see us continue to explicitly list subtests.
> 
> Hmm... I see your point(s).  It sounds like I need to come up with a
> plan C that also accounts for these goals.  As it stands, I think we want:
> 
> 1. Eliminate the need to maintain subtest list information in multiple
> places (i.e., the test itself and all.tests).
> 
> 2. Allow individually-run subtests to have a "pretty" name in the test log

> 
> 3. Reduce the uglification of all.tests.  There are a bunch of places in
> all.tests that do things like:
> 
> for api_suffix, possible_options in [('', [[], ['interface']]),
>                                      ('_gles3', [[]])]:
>     for subtest in ['basic-struct', 'struct-whole-array',
>                     'struct-array-elem', 'array-struct',
>                     'array-struct-whole-array', 'array-struct-array-elem',
>                     'struct-struct', 'array-struct-array-struct']:
>         for mode in ['error', 'get', 'run', 'run-no-fs']:
>             for options in possible_options:
>                 args = [subtest, mode] + options
>                 test_name = 'structs{0} {1}'.format(
>                         api_suffix, ' '.join(args))
>                 ext_transform_feedback[test_name] = concurrent_test(
>                         'ext_transform_feedback-{0}'.format(test_name))


I really, really, really don't like the all.tests format. And I have several 
reasons for it, but I'd really like to move to a true data storage format, 
sql, xml, json, something, anything other than an executed python script.

I'd be willing to put in the effort to convert from one format to another.

> 
> While I completely understand why they exist (many have my R-b), I find
> most of these to be pretty horrible to look at.
> 
> 4. Allow tests to generate the list of subtests at run-time or
> compile-time. (I'm a little dubious of this one, but it was requested by
> Paul.)
> 
> 5. Keep test run-time overhead to a minimum.
> 
> 6. Avoid future cross-compile nightmares.
> 
> I think #4 and #6 are mutually exclusive. :(  Does anyone actually
> cross-compile piglit?

Android and ChromeOS come to mind as possible cases.

> 
> In the meantime, I will:
> 
>  - Push the first four patches in this series.
> 
>  - Re-re-re-re-re-send (I think that's the correct number of re-) the
> last patch with the subtests explicitly listed in all.tests.
> 
>  - I wrote a Python script that generates the all.tests code from the
> test binary so that you can copy-and-paste it in.  Should I include a
> patch that adds that somewhere?  If yes, where?
> 
> > --Ken

I see a larger problem of confusing what a subtest is. The piglit subtest 
mechanism was created to allow multiple, similar tests to be run at once to 
reduce the overhead of reinitializing (it came from CL land to allow them to 
get around the overhead of initializing CL tests, which is aparently huge). 
What it seems you want Ian, it to reuse code by running multiple tests from a 
single binary; we already many examples of that throughout piglit.

> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131028/97cecabe/attachment.pgp>