On Mon, Oct 26, 2015 at 04:28:15PM +0000, Thomas Wood wrote:
> On 26 October 2015 at 15:28, David Weinehall
> <david.weinehall@xxxxxxxxxxxxxxx> wrote:
> > On Fri, Oct 23, 2015 at 03:55:23PM +0100, Thomas Wood wrote:
> >> On 23 October 2015 at 12:42, David Weinehall
> >> <david.weinehall@xxxxxxxxxxxxxxx> wrote:
> >> > Some tests should not be run by default, due to their slow,
> >> > and sometimes superfluous, nature.
> >> >
> >> > We still want to be able to run these tests though in some cases.
> >> > Until now there's been no unified way of handling this. Remedy
> >> > this by introducing the --with-slow-combinatorial option to
> >> > igt_core, and use it in gem_concurrent_blit & kms_frontbuffer_tracking.
> >> > ---
> >> > lib/igt_core.c | 19 ++++++
> >> > lib/igt_core.h | 1 +
> >> > tests/gem_concurrent_blit.c | 40 ++++++++----
> >> > tests/kms_frontbuffer_tracking.c | 135 +++++++++++++++++++++++++++------------
> >> > 4 files changed, 142 insertions(+), 53 deletions(-)
> >> >
> >> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> >> > index 59127cafe606..ba40ce0e0ead 100644
> >> > --- a/lib/igt_core.c
> >> > +++ b/lib/igt_core.c
> >> > @@ -216,6 +216,7 @@ const char *igt_interactive_debug;
> >> >
> >> > /* subtests helpers */
> >> > static bool list_subtests = false;
> >> > +static bool with_slow_combinatorial = false;
> >> > static char *run_single_subtest = NULL;
> >> > static bool run_single_subtest_found = false;
> >> > static const char *in_subtest = NULL;
> >> > @@ -235,6 +236,7 @@ bool test_child;
> >> >
> >> > enum {
> >> > OPT_LIST_SUBTESTS,
> >> > + OPT_WITH_SLOW_COMBINATORIAL,
> >> > OPT_RUN_SUBTEST,
> >> > OPT_DESCRIPTION,
> >> > OPT_DEBUG,
> >> > @@ -478,6 +480,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
> >> >
> >> > fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
> >> > fprintf(f, " --list-subtests\n"
> >> > + " --with-slow-combinatorial\n"
> >> > " --run-subtest <pattern>\n"
> >> > " --debug[=log-domain]\n"
> >> > " --interactive-debug[=domain]\n"
> >> > @@ -510,6 +513,7 @@ static int common_init(int *argc, char **argv,
> >> > int c, option_index = 0, i, x;
> >> > static struct option long_options[] = {
> >> > {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> >> > + {"with-slow-combinatorial", 0, 0, OPT_WITH_SLOW_COMBINATORIAL},
> >> > {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> >> > {"help-description", 0, 0, OPT_DESCRIPTION},
> >> > {"debug", optional_argument, 0, OPT_DEBUG},
> >> > @@ -617,6 +621,10 @@ static int common_init(int *argc, char **argv,
> >> > if (!run_single_subtest)
> >> > list_subtests = true;
> >> > break;
> >> > + case OPT_WITH_SLOW_COMBINATORIAL:
> >> > + if (!run_single_subtest)
> >>
> >> This will cause piglit (and therefore QA) to unconditionally run all
> >> tests marked as slow, since it runs subtests individually.
> >
> > Why doesn't piglit run the default set of tests instead?
>
> What is the default set of tests? Each subtest is executed by piglit
> using --run-subtest to ensure information can be collected per-subtest
> (return code, error messages, dmesg logs, timings, etc.).
The default set would be the tests that are run when running
gem_concurrent_blit or kms_frontbuffer_tracking without specifying
--all.
> >
> >>
> >> > + with_slow_combinatorial = true;
> >> > + break;
> >> > case OPT_RUN_SUBTEST:
> >> > if (!list_subtests)
> >> > run_single_subtest = strdup(optarg);
> >> > @@ -1629,6 +1637,17 @@ void igt_skip_on_simulation(void)
> >> > igt_require(!igt_run_in_simulation());
> >> > }
> >> >
> >> > +/**
> >> > + * igt_slow_combinatorial:
> >> > + *
> >> > + * This is used to define subtests that should only be listed/run
> >> > + * when the "--with-slow-combinatorial" has been specified
> >>
> >> This isn't quite correct, as the subtests that use
> >> igt_slow_combinatorial will still always be listed.
> >
> > Yeah, I agree that the comment is incorrect; it should say "be run",
> > or alternatively the code altered to not list them unless "--all"
> > is passed.
> >
> >> > + */
> >> > +void igt_slow_combinatorial(void)
> >> > +{
> >> > + igt_skip_on(!with_slow_combinatorial);
> >>
> >> Although it is convenient to just skip the tests when the
> >> --with-slow-combinatorial flag is passed, it may be useful to be able
> >> to classify the subtests before they are run, so that they are
> >> filtered out from the test list entirely. An approach that can do this
> >> might also be used to mark tests as being part of the basic acceptance
> >> tests, so that they can be marked as such without relying on the
> >> naming convention.
> >
> > If the list is how piglit gets its list of tests, doing classification
> > won't be feasible, since only "testname" or "testname (SKIP)" are
> > valid, TTBOMK.
>
> Test and subtest names for i-g-t are collected and parsed in
> piglit/tests/igt.py, which could always be updated include
> classification parsing.
It would probably make sense to do this, but considering that I neither
have proper python-fu nor know enough about the various test cases to
classify them properly, I think that's orthogonal to this changeset.
Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
