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

From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests
Date: Tue, 17 Nov 2015 16:34:45 +0100	[thread overview]
Message-ID: <20151117153445.GH16848@phenom.ffwll.local> (raw)
In-Reply-To: <CA+gsUGSBiCiNYBEWyNbNOh-Dke6=a=eUJG9JG8uVBtE5FrymeQ@mail.gmail.com>

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@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@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. 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx