Re: [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests

Paulo Zanoni
2015-10-26T16:44:18Z

2015-10-26 12:59 GMT-02:00 David Weinehall <david.weinehall@linux.intel.com>:
> On Fri, Oct 23, 2015 at 11:50:46AM -0200, Paulo Zanoni wrote:
>
> [snip]
>
>> It's not clear to me, please clarify: now the tests that were
>> previously completely hidden will be listed in --list-subtests and
>> will be shown as skipped during normal runs?
>
> Yes. Daniel and I discussed this and he thought listing all test
> cases, even the slow ones, would not be an issue, since QA should
> be running the default set not the full list
> (and for that matter, shouldn't QA know what they are doing too? :P).

If that's the case, I really think your patch should not touch
kms_frontbuffer_tracking.c. The hidden subtests should not appear on
the list. People shouldn't even have to ask themselves why they are
getting 800 skips from a single testcase. Those are only for debugging
purposes.

>
>> For kms_frontbuffer_tracking, hidden tests are supposed to be just for
>> developers who know what they are doing. I hide them behind a special
>> command-line switch that's not used by QA because I don't want QA
>> wasting time running those tests. One third of the
>> kms_frontbuffer_tracking hidden tests only serve the purpose of
>> checking whether there's a bug in kms_frontbuffer_track itself or not.
>> For some other hidden tests, they are there just to help better debug
>> in case some other non-hidden tests fail. Some other hidden tests are
>> 100% useless and superfluous.
>
> Shouldn't 100% useless and superfluous tests be excised completely?

The change would be from "if (case && hidden) continue;" to "if (case)
continue;". But that's not the focus. There are still tests that are
useful for debugging but useless for QA.

>
>> QA should only run the non-hidden tests.
>
> Which is the default behaviour, AFAICT.

Then why do you want to expose those tests that you're not even
planning to run?? You're kinda implying that QA - or someone else -
will run those tests at some point, and I say that, for
kms_frontbuffer_tracking, that's a waste of time. Maybe this is the
case for the other tests you're touching, but not here.

>
>> So if some non-hidden test fails, the developers can use the hidden
>> tests to help debugging.
>>
>> Besides, the "if (t.slow)" could have been moved to
>> check_test_requirements(), making the code much simpler :)
>
> Thanks for the suggestion. Will modify the code accordingly.
> That change does indeed simplify things quite a bit!
>
>
> Kind regards, David

--
Paulo Zanoni