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

[David Weinehall <david.weinehall@linux.intel.com>]

On Fri, Oct 23, 2015 at 03:55:23PM +0100, Thomas Wood wrote:
> On 23 October 2015 at 12:42, David Weinehall
> <david.weinehall@linux.intel.com> 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?

> 
> > +                               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, doign classification
won't be feasible, since only "testname" or "testname (SKIP)" are
valid, TTBOMK.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx