[Piglit] [PATCH 1/2] tests/all.py: Fix name of arb_tessellation_shader test

Ilia Mirkin imirkin at alum.mit.edu
Thu Aug 28 16:51:34 PDT 2014


On Thu, Aug 28, 2014 at 7:39 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Thursday, August 28, 2014 07:19:27 PM Ilia Mirkin wrote:
>> On Thu, Aug 28, 2014 at 7:06 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
>> > '/' is not allowed in the name of tests because it is the delimiter for
>> > group names. This test includes '/' which causes the HTML summary to
>> > render in odd ways.
>> >
>> > Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
>> > ---
>> >
>> > Hi Chris,
>> >
>> > This is a pretty easy problem to solve, I've added an assertion in the
>> > second patch to catch this bug.
>> >
>> > Hopefully this works for you?
>> >
>> > -Dylan
>> >
>> >  tests/all.py | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/tests/all.py b/tests/all.py
>> > index 20ebec2..2adaf64 100644
>> > --- a/tests/all.py
>> > +++ b/tests/all.py
>> > @@ -1513,7 +1513,7 @@ add_concurrent_test(arb_tessellation_shader, 'arb_tessellation_shader-minmax')
>> >  add_concurrent_test(arb_tessellation_shader, 'arb_tessellation_shader-invalid-get-program-params')
>> >  add_concurrent_test(arb_tessellation_shader, 'arb_tessellation_shader-invalid-patch-vertices-range')
>> >  add_concurrent_test(arb_tessellation_shader, 'arb_tessellation_shader-invalid-primitive')
>> > -add_concurrent_test(arb_tessellation_shader, 'built-in-constants tests/spec/arb_tessellation_shader/minimum-maximums.txt')
>> > +profile.test_list['spec/ARB_tessellation_shader/built-in-constants'] = concurrent_test('built-in-constants tests/spec/arb_tessellation_shader/minimum-maximums.txt')
>>
>> Isn't the way that every other place does it more like
>>
>> arb_tessellation_shader['built-in-constants'] = concurrent_test(...)
>
> Well, there was a request from the person who implemented the test_list
> for people to stop using the nested group structure because it's awful,
> but everyone ignored him.

Did this unnamed individual consider the fact that people don't like
change? :) It could just as well be said that having the full path is
awful because it's longer and encourages typos... I think if you
created a syntax that was pleasing to the eye but used the test list
directly under the covers, no one would object.

>
> It does reduce the number of tests handled by the flattening pass that
> happens during TestProfile.run(), but if you'd prefer it the other way.

In units of seconds, how much time is that?

>
>> If you want to convert everything over to doing
>> profile.test_list[...], that's best not done one-at-a-time...
>
> I've tried to do it all at once, it's a much larger and more painful
> problem than you would think :)

Well, I think mixing and matching, esp within one test group, is way
worse than almost any other alternative...

  -ilia


More information about the Piglit mailing list