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

[Paulo Zanoni]

2015-11-17 13:34 GMT-02:00 Daniel Vetter <daniel at ffwll.ch>:
> On Mon, Oct 26, 2015 at 03:59:24PM -0200, Paulo Zanoni wrote:
>> 2015-10-26 15:30 GMT-02:00 David Weinehall <david.weinehall at linux.intel.com>:
>> > On Mon, Oct 26, 2015 at 02:44:18PM -0200, Paulo Zanoni wrote:
>> >> 2015-10-26 12:59 GMT-02:00 David Weinehall <david.weinehall at 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.
>> >
>> > Fair enough.  I'll try to come up with a resonable way to exclude them
>> > from the list in a generic manner.  Because that's the whole point of
>> > this exercise -- to standardise this rather than have every test case
>> > implement its own method of choosing whether or not to run all tests.
>>
>> Maybe instead of marking these tests as SKIP we could use some other
>> flag. That would avoid the confusion between "skipped because some
>> condition was not match but the test is useful" vs "skipped because
>> the test is unnecessary".
>>
>> >
>> >> >
>> >> >> 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.
>> >
>> > It's not the focus of my change, no. But if there are tests that are
>> > useless and/or superfluous, then they should be dropped.
>> > Note that
>> > I'm not suggesting that all non-default tests be dropped, just that
>> > if there indeed are tests that don't make sense, they shouldn't be
>> > in the test case in the first place.
>> >
>> >> >
>> >> >> 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??
>> >
>> > To allow developers to see the options they have?
>> >
>> >> 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.
>> >
>> > No, I'm not implying that -- you're putting those words in my mouth.
>> >
>> > Anyway, the choice to expose all cases, not just those run without
>> > specifying --all, was a suggestion by Daniel -- you'll have to prod him
>> > to hear what his reasoning was.
>>
>> CC'ing Daniel.
>
> I thought the hidden tests in kms_frontbuffer_tracking would be useful,
> just really slow, but seems I'm mistaken. In general we have a bunch of
> stress tests which we want to run, but at a lower priority.

So it doesn't sound good to put both the kms_frontbuffer_trackign and
the slow-but-useful behind the same knob. Anyway, I think the "flags"
idea can solve the problem.


> The idea is to
> eventually use this knob to resurface them, but right now with only BAT
> igt running in our CI we can't do that and still need to do a lot of other
> things first.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Paulo Zanoni