On Fri, Oct 30, 2015 at 09:55:03AM -0200, Paulo Zanoni wrote: > 2015-10-30 5:56 GMT-02:00 David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>: > > On Wed, Oct 28, 2015 at 02:12:15PM -0200, Paulo Zanoni wrote: > >> 2015-10-28 9:29 GMT-02:00 David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>: > >> > 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 in some cases. > >> > Until now there's been no unified way of handling this. Remedy > >> > this by introducing the --all option to igt_core, > >> > and use it in gem_concurrent_blit & kms_frontbuffer_tracking. > >> > >> I really think you should explain both your plan and its > >> implementation in more details here. > > > > Well, I don't see how much more there is to explain; the idea is simply > > that different tests shouldn't implement similar behaviour in different > > manners (current kms_frontbuffer_tracking uses a command line option, > > gem_concurrent_blit changes behaviour depending on the file name it's > > called with). > > What made me write that is that I noticed that now --all is required > during --list-subtests (thanks for doing this!) but it was not easy to > notice this in the commit message or in the code. So I was thinking > something simple, such as a description of how to use the new option > both when running IGT and when writing subtests: > > "These tests will only appear in --list-subtests if you also specify > --all. Same for --run-subtest calls. There's this new macro > igt_subtest_slow_f (maybe igt_subtest_flags_f now?) which should be > used in case the subtest can be slow/combinatorial." > > > > >> > > >> > Signed-off-by: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx> > >> > --- > >> > lib/igt_core.c | 24 +++++ > >> > lib/igt_core.h | 7 ++ > >> > tests/gem_concurrent_blit.c | 44 ++++----- > >> > tests/kms_frontbuffer_tracking.c | 208 ++++++++++++++++++++++----------------- > >> > 4 files changed, 165 insertions(+), 118 deletions(-) > >> > > >> > diff --git a/lib/igt_core.c b/lib/igt_core.c > >> > index 59127cafe606..6575b9d6bf0d 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; > >> > >> The option is called --all, the new subtest macro is _slow and the > >> variables and enums are called with_slow_combinatorial. Is this > >> intentional? > > > > The option is called --all because "--with-slow-combinatorial" was > > considered to be too much of a mouthful. The variables & enums are > > still retaining these names because they're much more descriptive. > > Ok, let's keep is like this then (in case they don't change with > _flags suggestion). > > > > > The macro is called _slow because I wanted to keep it a bit shorter, > > but I can rename it to _slow_combinatorial if that's preferred. > > I agree _slow_combinatorial is too big, so let's keep it like this. > Maybe the new version is going to be _flags or something? igt_subtest_cond_f(). The subtest is included in the test lists if and only if the conditional experession is true. And run with igt_subtest_flags. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
