[Piglit] [RFC v2 00/39] Rework profile modification (all.py)

[Ilia Mirkin]

On Mon, Feb 2, 2015 at 8:37 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This version incorporates changes suggested by Ilia, require_platforms
> and exclude_platforms are now easier to work with. This version also
> includes patches to flip the meaning of run_concurrent for PiglitGLTest,
> ShaderTest, and GLSLParserTest. It also simplifies the way that
> PiglitCLTest handles it's run_concurrent attribute.
>
> One suggestion from Ilia that I implemented but ultimately decided
> against was the ability to specify a prefix to be removed or added to
> the arguments. To me it's very important that the arguments to the test
> be exact and explicit, there should not be any magic going on, since
> these are what makes the test, so specifying a prefix to remove from the
> arguments to make the name of the test seemed fine, however, the number
> of tests that actually would make use of the feature was relatively
> small, and it required in many cases calling group_manager multiple
> times for a single group. It also has a good chance of surprising
> someone, so I opted to not do that.

I'd still encourage you to allow run_concurrent to be specified by the
context manager, and then letting individual tests override as needed.
Not a _huge_ deal I suppose, but definitely my personal preference. In
fact making it always explicitly specified in the context might be a
nice improvement for readability.

I also can't say I'm a huge fan of g(['asdf']). What's wrong with
g('asdf')? Use the list when there are multiple args... or just use
multiple positional args, i.e. g('asdf', 'arg1')?

Other than that, the final result looks quite reasonable. One final question --

with profile.group_manager(
  PiglitGLTest,
  grouptools.join('spec', 'arb_draw_buffers_blend')) as g:

But the current code reads

spec['ARB_draw_buffers_blend'] = arb_draw_buffers_blend

It looks like you've lower-cased things which will cause _huge_ churn
wrt an old results file. Can you run a before-and-after comparison
with your changes? That will make sure that (a) you got all (or most)
the tests and (b) none of the names changed accidentally.

Cheers,

  -ilia