From: David Weinehall <david.weinehall@linux.intel.com>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests
Date: Mon, 26 Oct 2015 19:30:31 +0200 [thread overview]
Message-ID: <20151026173031.GE2504@boom> (raw)
In-Reply-To: <CA+gsUGRjgzcfODdRzf7p8_aZwydLD=0Mj==Bx4N6J7SoAFyN3g@mail.gmail.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.
> >
> >> 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.
Regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-10-26 17:30 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-23 11:42 [PATCH i-g-t 0/3] Unify slow/combinatorial test handling David Weinehall
2015-10-23 11:42 ` [PATCH i-g-t 1/3] Rename gem_concurren_all over gem_concurrent_blit David Weinehall
2015-10-23 14:32 ` Thomas Wood
2015-10-26 15:03 ` David Weinehall
2015-10-23 11:42 ` [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests David Weinehall
2015-10-23 11:56 ` Chris Wilson
2015-10-23 13:50 ` Paulo Zanoni
2015-10-26 14:59 ` David Weinehall
2015-10-26 16:44 ` Paulo Zanoni
2015-10-26 17:30 ` David Weinehall [this message]
2015-10-26 17:59 ` Paulo Zanoni
2015-10-27 6:47 ` David Weinehall
2015-11-17 15:33 ` Daniel Vetter
2015-11-17 15:34 ` Daniel Vetter
2015-11-17 15:49 ` Paulo Zanoni
2015-11-18 10:19 ` David Weinehall
2015-10-23 14:55 ` Thomas Wood
2015-10-26 15:28 ` David Weinehall
2015-10-26 16:28 ` Thomas Wood
2015-10-26 17:34 ` David Weinehall
2015-10-26 18:15 ` Paulo Zanoni
2015-10-23 11:42 ` [PATCH i-g-t 3/3] Remove gem_concurrent_all, since it is now superfluous David Weinehall
2015-10-23 11:58 ` [PATCH i-g-t 0/3] Unify slow/combinatorial test handling Chris Wilson
2015-10-23 12:47 ` Daniel Vetter
2015-10-26 13:55 ` David Weinehall
2015-10-28 11:29 ` [PATCH i-g-t 0/3 v2] " David Weinehall
2015-10-28 11:29 ` [PATCH i-g-t 1/3] Copy gem_concurrent_all to gem_concurrent_blit David Weinehall
2015-10-28 11:29 ` [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests David Weinehall
2015-10-28 16:12 ` Paulo Zanoni
2015-10-30 7:56 ` David Weinehall
2015-10-30 11:55 ` Paulo Zanoni
2015-10-30 11:59 ` Chris Wilson
2015-10-28 17:14 ` Thomas Wood
2015-10-30 7:44 ` David Weinehall
2015-10-28 11:29 ` [PATCH i-g-t 3/3] Remove superfluous gem_concurrent_all.c David Weinehall
2015-10-30 13:18 ` [PATCH i-g-t 0/3 v3] Unify slow/combinatorial test handling David Weinehall
2015-10-30 13:18 ` [PATCH i-g-t 1/3 v3] Copy gem_concurrent_all to gem_concurrent_blit David Weinehall
2015-10-30 13:18 ` [PATCH i-g-t 2/3 v3] Unify handling of slow/combinatorial tests David Weinehall
2015-10-30 13:52 ` Chris Wilson
2015-11-12 11:00 ` David Weinehall
2015-10-30 13:18 ` [PATCH i-g-t 3/3 v3] Remove superfluous gem_concurrent_all.c David Weinehall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151026173031.GE2504@boom \
--to=david.weinehall@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=przanoni@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.