[igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Antonio Argenziano]

On 31/01/18 00:01, Daniel Vetter wrote:
> On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
>> On 25/01/18 03:37, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-01-25 01:00:03)
>>>> +               }
>>>> +
>>>>                   gem_context_get_param(fd, &arg);
>>>>                   old_value = arg.value;
>>>> @@ -312,9 +338,13 @@ igt_main
>>>>                   }
>>>>                   for (unsigned flags = 0;
>>>> -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>> +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>>                                   flags++) {
>>>> -                       igt_subtest_f("set-priority%s%s%s%s",
>>>> +                       if (!(flags & NICE) && !(flags & ROOT))
>>>> +                               continue; /* Needs to be rot to set properties */
>>>
>>> Nope, just use igt_require as above.
>>
>> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
>> with igt_require they would just skip but still be there right?
> 
> subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
> Hence why igt_require/igt_skip result in a special subtest result and will
> not supress the subtest outright.

In this case the combination I am trying to suppress is actually invalid 
for all platforms/kernels that is why I wanted to avoid skipping as it 
would hint that the test can actually run if some conditions are met 
which is not the case here. That is why I was confused about the first 
suggestion (which I might be completely misinterpreting). I'll send the 
new revision so we can continue the discussion over the new code.

> 
> I know that vpg is struggling with heaps of skipped tests in their

(whispers) I actually agree with running everything and letting the test 
skip when not applicable ;).

Thanks,
Antonio

> reporting, but as soon as you do cross-platform/kernel/OS testing, it's
> the only reasonable way to handle things.
> 
> So yeah, this needs to be an igt_require, and probably moved into the
> subtest stanza (or wrapped with an igt_subtest_group depending upon the
> overall test logic).
> 
> 
> Aside: The igt intro docs should explain this, patch would be great if
> it's unclear/missing.
> -Daniel
>