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

David Weinehall
2015-10-30T07:44:27Z

On Wed, Oct 28, 2015 at 05:14:28PM +0000, Thomas Wood wrote:
> If this is intended to be documented and used in tests, then it should
> be included in the public API (i.e. without the underscore prefix).

True. Will fix.

> > + *
> > + * This is used to skip subtests that should only be included
> > + * when the "--all" command line option has been specified. This version
> > + * is intended as a test.
> > + *
> > + * @slow_test: true if the subtest is part of the slow/combinatorial set
>
> If this is used to test if a slow subtest should be run, shouldn't
> slow_test always be true?

The test is written such that igt_subtest_slow_f() can always be used
both for fast and slow cases -- the slow flag will decide whether or not
it should bail out (combined with the --all flag, obviously).

So slow_test isn't always true.

> Documentation for igt_subtest_slow_f is needed here. If __slow is
> false, this macro just defines a normal subtest, which is
> contradictory to its name. Perhaps igt_subtest_with_flags_f (or
> similar) would be better and would also allow for future expansion
> with other categories.

Yeah, that could be a workable solution; in discussion with Daniel
earlier we agreed not to do a "flags" implementation, to keep things
simple for now, but it might indeed be better to do things right
from the start.

As we get to the point where we call igt_subtest with several different
flags things will probably get quite complex though; at that point
I suspect that the macro might start looking really hairy...

Kind regards, David