| Message ID | 1453889156-24489-1-git-send-email-derek.j.morton@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: > Added support for specifying arbitary lists of subtests to run or > to exclude from being run by using : or ^ as a seperator. > > :subtest1:subtest2: Will run subtest1 and subtest2 > ^subtest1^subtest2^ will run all subtests except subtest1 and subtest2 Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? > > Any subtest string not starting : or ^ is treated as a normal wildcard > expression. > > This is required mainly on android to exclude subtests that test > features that do not exist in the android driver while still being able > to run other subtests in the binary when a wildcard expression is > insufficient. > > Signed-off-by: Derek Morton <derek.j.morton@intel.com> > --- > lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 6b69bb7..b9e7470 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -207,7 +207,15 @@ > * To do that obtain the lists of subtests with "--list-subtests", which can be > * run as non-root and doesn't require the i915 driver to be loaded (or any > * intel gpu to be present). Then individual subtests can be run with > - * "--run-subtest". Usage help for tests with subtests can be obtained with the > + * "--run-subtest". --run-subtest accepts wildcard characters. A list of > + * subtests to run may be specified by using : as a seperator. A list of > + * subtests to exclude may be specified using ^ as a seperator. > + * > + * - --run-subtest basic* will run all subtests starting basic. > + * - --run-subtest :subtest1:subtest2: will run only subtest1 and subtest2 > + * - --run-subtest ^subtest1^subtest2^ will run all except subtest1 and subtest2 > + * > + * Usage help for tests with subtests can be obtained with the > * "--help" command line option. > */ > > @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, > extra_opt_handler, handler_data); > } > > +static bool check_testlist(const char *subtest_name) > +{ > + char *p; > + > + /* Run subtests in list > + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: > + * return true if found. > + */ > + if (run_single_subtest[0] == ':') { > + p = strstr(run_single_subtest, subtest_name); > + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) > + return true; > + } > + /* Run subtests not in list > + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ > + * return true if not found. > + */ > + else if (run_single_subtest[0] == '^') { > + p = strstr(run_single_subtest, subtest_name); > + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) > + return true; > + } > + /* Run subtests that match shell wildcard */ > + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) > + return true; > + > + return false; > +} > + > /* > * Note: Testcases which use these helpers MUST NOT output anything to stdout > * outside of places protected by igt_run_subtest checks - the piglit > @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) > } > > if (run_single_subtest) { > - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) > + > + if (check_testlist(subtest_name) == false) > return false; > else > run_single_subtest_found = true; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, January 27, 2016 12:33 PM >To: Morton, Derek J >Cc: intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >> Added support for specifying arbitary lists of subtests to run or to >> exclude from being run by using : or ^ as a seperator. >> >> :subtest1:subtest2: Will run subtest1 and subtest2 ^subtest1^subtest2^ >> will run all subtests except subtest1 and subtest2 > >Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. Allowing it to be added multiple times could be a way of building up a list of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? I think that will just end up more complicated than the simple separated list solution this patch introduces. //Derek > >> >> Any subtest string not starting : or ^ is treated as a normal wildcard >> expression. >> >> This is required mainly on android to exclude subtests that test >> features that do not exist in the android driver while still being >> able to run other subtests in the binary when a wildcard expression is >> insufficient. >> >> Signed-off-by: Derek Morton <derek.j.morton@intel.com> >> --- >> lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb7..b9e7470 >> 100644 >> --- a/lib/igt_core.c >> +++ b/lib/igt_core.c >> @@ -207,7 +207,15 @@ >> * To do that obtain the lists of subtests with "--list-subtests", which can be >> * run as non-root and doesn't require the i915 driver to be loaded (or any >> * intel gpu to be present). Then individual subtests can be run with >> - * "--run-subtest". Usage help for tests with subtests can be >> obtained with the >> + * "--run-subtest". --run-subtest accepts wildcard characters. A list >> + of >> + * subtests to run may be specified by using : as a seperator. A list >> + of >> + * subtests to exclude may be specified using ^ as a seperator. >> + * >> + * - --run-subtest basic* will run all subtests starting basic. >> + * - --run-subtest :subtest1:subtest2: will run only subtest1 and >> + subtest2 >> + * - --run-subtest ^subtest1^subtest2^ will run all except subtest1 >> + and subtest2 >> + * >> + * Usage help for tests with subtests can be obtained with the >> * "--help" command line option. >> */ >> >> @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, >> extra_opt_handler, handler_data); } >> >> +static bool check_testlist(const char *subtest_name) { >> + char *p; >> + >> + /* Run subtests in list >> + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: >> + * return true if found. >> + */ >> + if (run_single_subtest[0] == ':') { >> + p = strstr(run_single_subtest, subtest_name); >> + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) >> + return true; >> + } >> + /* Run subtests not in list >> + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ >> + * return true if not found. >> + */ >> + else if (run_single_subtest[0] == '^') { >> + p = strstr(run_single_subtest, subtest_name); >> + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) >> + return true; >> + } >> + /* Run subtests that match shell wildcard */ >> + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) >> + return true; >> + >> + return false; >> +} >> + >> /* >> * Note: Testcases which use these helpers MUST NOT output anything to stdout >> * outside of places protected by igt_run_subtest checks - the piglit >> @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) >> } >> >> if (run_single_subtest) { >> - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) >> + >> + if (check_testlist(subtest_name) == false) >> return false; >> else >> run_single_subtest_found = true; >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel OTC >
On Wed, Jan 27, 2016 at 02:32:47PM +0200, Ville Syrjälä wrote: > On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: > > Added support for specifying arbitary lists of subtests to run or > > to exclude from being run by using : or ^ as a seperator. > > > > :subtest1:subtest2: Will run subtest1 and subtest2 > > ^subtest1^subtest2^ will run all subtests except subtest1 and subtest2 > > Hmm. Getting a bit complicated perhaps. Would it be simpler to just > allow specifying the --r option multiple times? So we'd start with the > full list of subtests, and each --r option would filter the list in > some way? Also why not use piglit ... or what is this for? -Daniel
> >-----Original Message----- >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Wednesday, January 27, 2016 1:39 PM >To: Ville Syrjälä >Cc: Morton, Derek J; intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >On Wed, Jan 27, 2016 at 02:32:47PM +0200, Ville Syrjälä wrote: >> On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >> > Added support for specifying arbitary lists of subtests to run or to >> > exclude from being run by using : or ^ as a seperator. >> > >> > :subtest1:subtest2: Will run subtest1 and subtest2 >> > ^subtest1^subtest2^ will run all subtests except subtest1 and >> > subtest2 >> >> Hmm. Getting a bit complicated perhaps. Would it be simpler to just >> allow specifying the --r option multiple times? So we'd start with the >> full list of subtests, and each --r option would filter the list in >> some way? > >Also why not use piglit ... or what is this for? We don't use piglet on android. Piglet does not know about adb. Piglet expects to be running on the system under test not on a separate host. The main aim of this is because on android we are not testing a driver which is drm-nightly. The kernel / display driver used on android will not have all the features that are in the latest linux kernel. We keep hitting problems where new subtests get added to IGT to test features that are not yet in the android kernel. We run and report tests at a binary level as that is what the project managers expect. We wish to be able to run the latest versions of IGT to pick up bug fixes and useful test changes, but want a way of being able to exclude subtests that are not currently appropriate on android without having to exclude complete test binaries. The specific subtests which need to be excluded will differ depending on the HW (CHV vs BXT for example) and specific driver version in the build under test so we need a simple mechanism to specify the subtests to run or exclude (depending on which is more appropriate) for each test. //Derek >-Daniel >-- >Daniel Vetter >Software Engineer, Intel Corporation >http://blog.ffwll.ch >
On Wed, Jan 27, 2016 at 01:30:01PM +0000, Morton, Derek J wrote: > > > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Wednesday, January 27, 2016 12:33 PM > >To: Morton, Derek J > >Cc: intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > > > >On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: > >> Added support for specifying arbitary lists of subtests to run or to > >> exclude from being run by using : or ^ as a seperator. > >> > >> :subtest1:subtest2: Will run subtest1 and subtest2 ^subtest1^subtest2^ > >> will run all subtests except subtest1 and subtest2 > > > >Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? > > By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. --r not -r ;) > Allowing it to be added multiple times could be a way of building up a list of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. Or could be something like --r !subtest-name > With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. > Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? Hmm. I suppoose my original idea of start with full list and filter stuff out doesn't work entirely well. Eg. --r foo --r bar would run nothing. So I guess the option would have to be able to add to the list as well. So I guess we could make it so that '!' removes the specified test(s), no-'!' adds them, and if this is the first --r option and there's no '!' we'd clear the list entirely before adding the specified test(s) to it. > > I think that will just end up more complicated than the simple separated list solution this patch introduces. I suppose. But it would avoid adding a new language which can look a bit like a weird regexp but isn't. Maybe if you just use ',' to separate the subtests specifications, and '!' to specify the not condition things would look a bit more standardish. > > //Derek > > > > >> > >> Any subtest string not starting : or ^ is treated as a normal wildcard > >> expression. > >> > >> This is required mainly on android to exclude subtests that test > >> features that do not exist in the android driver while still being > >> able to run other subtests in the binary when a wildcard expression is > >> insufficient. > >> > >> Signed-off-by: Derek Morton <derek.j.morton@intel.com> > >> --- > >> lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > >> 1 file changed, 40 insertions(+), 2 deletions(-) > >> > >> diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb7..b9e7470 > >> 100644 > >> --- a/lib/igt_core.c > >> +++ b/lib/igt_core.c > >> @@ -207,7 +207,15 @@ > >> * To do that obtain the lists of subtests with "--list-subtests", which can be > >> * run as non-root and doesn't require the i915 driver to be loaded (or any > >> * intel gpu to be present). Then individual subtests can be run with > >> - * "--run-subtest". Usage help for tests with subtests can be > >> obtained with the > >> + * "--run-subtest". --run-subtest accepts wildcard characters. A list > >> + of > >> + * subtests to run may be specified by using : as a seperator. A list > >> + of > >> + * subtests to exclude may be specified using ^ as a seperator. > >> + * > >> + * - --run-subtest basic* will run all subtests starting basic. > >> + * - --run-subtest :subtest1:subtest2: will run only subtest1 and > >> + subtest2 > >> + * - --run-subtest ^subtest1^subtest2^ will run all except subtest1 > >> + and subtest2 > >> + * > >> + * Usage help for tests with subtests can be obtained with the > >> * "--help" command line option. > >> */ > >> > >> @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, > >> extra_opt_handler, handler_data); } > >> > >> +static bool check_testlist(const char *subtest_name) { > >> + char *p; > >> + > >> + /* Run subtests in list > >> + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: > >> + * return true if found. > >> + */ > >> + if (run_single_subtest[0] == ':') { > >> + p = strstr(run_single_subtest, subtest_name); > >> + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) > >> + return true; > >> + } > >> + /* Run subtests not in list > >> + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ > >> + * return true if not found. > >> + */ > >> + else if (run_single_subtest[0] == '^') { > >> + p = strstr(run_single_subtest, subtest_name); > >> + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) > >> + return true; > >> + } > >> + /* Run subtests that match shell wildcard */ > >> + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) > >> + return true; > >> + > >> + return false; > >> +} > >> + > >> /* > >> * Note: Testcases which use these helpers MUST NOT output anything to stdout > >> * outside of places protected by igt_run_subtest checks - the piglit > >> @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) > >> } > >> > >> if (run_single_subtest) { > >> - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) > >> + > >> + if (check_testlist(subtest_name) == false) > >> return false; > >> else > >> run_single_subtest_found = true; > >> -- > >> 1.9.1 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >-- > >Ville Syrjälä > >Intel OTC > >
> > >-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, January 27, 2016 2:31 PM >To: Morton, Derek J >Cc: intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >On Wed, Jan 27, 2016 at 01:30:01PM +0000, Morton, Derek J wrote: >> > >> > >> >-----Original Message----- >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >> >Sent: Wednesday, January 27, 2016 12:33 PM >> >To: Morton, Derek J >> >Cc: intel-gfx@lists.freedesktop.org >> >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >> > >> >On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >> >> Added support for specifying arbitary lists of subtests to run or >> >> to exclude from being run by using : or ^ as a seperator. >> >> >> >> :subtest1:subtest2: Will run subtest1 and subtest2 >> >> ^subtest1^subtest2^ will run all subtests except subtest1 and >> >> subtest2 >> > >> >Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? >> >> By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. > >--r not -r ;) > >> Allowing it to be added multiple times could be a way of building up a list of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. > >Or could be something like --r !subtest-name > >> With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. >> Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? > >Hmm. I suppoose my original idea of start with full list and filter stuff out doesn't work entirely well. Eg. --r foo --r bar would run nothing. So I guess the option would have to be able to add to the list as well. > >So I guess we could make it so that '!' removes the specified test(s), no-'!' adds them, and if this is the first --r option and there's no '!' we'd clear the list entirely before adding the specified test(s) to it. > >> >> I think that will just end up more complicated than the simple separated list solution this patch introduces. > >I suppose. But it would avoid adding a new language which can look a bit like a weird regexp but isn't. > >Maybe if you just use ',' to separate the subtests specifications, and '!' to specify the not condition things would look a bit more standardish. I can't use ! as the shell uses it for some command history substitution. That is why I chose ^. There are very few special character the shell does not mess up :-(. How about the following: Comma separated list of subtests to specify a list of subtests to run. Prepend ^ to the list to specify a list of subtests to exclude. I would then have to use strstr to look for a comma in what is specified to --run-subtests. Is there any risk of a comma in a wildcard expression? I found some code for fnmatch with google and it looks like it is simply ?*[]- which is treated as special characters. //Derek > >> >> //Derek >> >> > >> >> >> >> Any subtest string not starting : or ^ is treated as a normal >> >> wildcard expression. >> >> >> >> This is required mainly on android to exclude subtests that test >> >> features that do not exist in the android driver while still being >> >> able to run other subtests in the binary when a wildcard expression >> >> is insufficient. >> >> >> >> Signed-off-by: Derek Morton <derek.j.morton@intel.com> >> >> --- >> >> lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb7..b9e7470 >> >> 100644 >> >> --- a/lib/igt_core.c >> >> +++ b/lib/igt_core.c >> >> @@ -207,7 +207,15 @@ >> >> * To do that obtain the lists of subtests with "--list-subtests", which can be >> >> * run as non-root and doesn't require the i915 driver to be loaded (or any >> >> * intel gpu to be present). Then individual subtests can be run >> >> with >> >> - * "--run-subtest". Usage help for tests with subtests can be >> >> obtained with the >> >> + * "--run-subtest". --run-subtest accepts wildcard characters. A >> >> + list of >> >> + * subtests to run may be specified by using : as a seperator. A >> >> + list of >> >> + * subtests to exclude may be specified using ^ as a seperator. >> >> + * >> >> + * - --run-subtest basic* will run all subtests starting basic. >> >> + * - --run-subtest :subtest1:subtest2: will run only subtest1 and >> >> + subtest2 >> >> + * - --run-subtest ^subtest1^subtest2^ will run all except >> >> + subtest1 and subtest2 >> >> + * >> >> + * Usage help for tests with subtests can be obtained with the >> >> * "--help" command line option. >> >> */ >> >> >> >> @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, >> >> extra_opt_handler, handler_data); } >> >> >> >> +static bool check_testlist(const char *subtest_name) { >> >> + char *p; >> >> + >> >> + /* Run subtests in list >> >> + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: >> >> + * return true if found. >> >> + */ >> >> + if (run_single_subtest[0] == ':') { >> >> + p = strstr(run_single_subtest, subtest_name); >> >> + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) >> >> + return true; >> >> + } >> >> + /* Run subtests not in list >> >> + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ >> >> + * return true if not found. >> >> + */ >> >> + else if (run_single_subtest[0] == '^') { >> >> + p = strstr(run_single_subtest, subtest_name); >> >> + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) >> >> + return true; >> >> + } >> >> + /* Run subtests that match shell wildcard */ >> >> + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) >> >> + return true; >> >> + >> >> + return false; >> >> +} >> >> + >> >> /* >> >> * Note: Testcases which use these helpers MUST NOT output anything to stdout >> >> * outside of places protected by igt_run_subtest checks - the >> >> piglit @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) >> >> } >> >> >> >> if (run_single_subtest) { >> >> - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) >> >> + >> >> + if (check_testlist(subtest_name) == false) >> >> return false; >> >> else >> >> run_single_subtest_found = true; >> >> -- >> >> 1.9.1 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> >-- >> >Ville Syrjälä >> >Intel OTC >> > > >-- >Ville Syrjälä >Intel OTC >
On Wed, Jan 27, 2016 at 03:02:15PM +0000, Morton, Derek J wrote: > > > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Wednesday, January 27, 2016 2:31 PM > >To: Morton, Derek J > >Cc: intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > > > >On Wed, Jan 27, 2016 at 01:30:01PM +0000, Morton, Derek J wrote: > >> > > >> > > >> >-----Original Message----- > >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >> >Sent: Wednesday, January 27, 2016 12:33 PM > >> >To: Morton, Derek J > >> >Cc: intel-gfx@lists.freedesktop.org > >> >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >> > > >> >On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: > >> >> Added support for specifying arbitary lists of subtests to run or > >> >> to exclude from being run by using : or ^ as a seperator. > >> >> > >> >> :subtest1:subtest2: Will run subtest1 and subtest2 > >> >> ^subtest1^subtest2^ will run all subtests except subtest1 and > >> >> subtest2 > >> > > >> >Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? > >> > >> By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. > > > >--r not -r ;) > > > >> Allowing it to be added multiple times could be a way of building up a list of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. > > > >Or could be something like --r !subtest-name > > > >> With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. > >> Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? > > > >Hmm. I suppoose my original idea of start with full list and filter stuff out doesn't work entirely well. Eg. --r foo --r bar would run nothing. So I guess the option would have to be able to add to the list as well. > > > >So I guess we could make it so that '!' removes the specified test(s), no-'!' adds them, and if this is the first --r option and there's no '!' we'd clear the list entirely before adding the specified test(s) to it. > > > >> > >> I think that will just end up more complicated than the simple separated list solution this patch introduces. > > > >I suppose. But it would avoid adding a new language which can look a bit like a weird regexp but isn't. > > > >Maybe if you just use ',' to separate the subtests specifications, and '!' to specify the not condition things would look a bit more standardish. > > I can't use ! as the shell uses it for some command history substitution. That is why I chose ^. There are very few special character the shell does not mess up :-(. A bunch of stuff uses ! (eg. test, find, etc.). The shell won't eat it if there's a space after it, or you could always escape it. getopt doesn't support it natively however so it could a pain to use the space trick. > > How about the following: > > Comma separated list of subtests to specify a list of subtests to run. > Prepend ^ to the list to specify a list of subtests to exclude. > > I would then have to use strstr to look for a comma in what is specified to --run-subtests. Is there any risk of a comma in a wildcard expression? I found some code for fnmatch with google and it looks like it is simply ?*[]- which is treated as special characters. > > //Derek > > > > >> > >> //Derek > >> > >> > > >> >> > >> >> Any subtest string not starting : or ^ is treated as a normal > >> >> wildcard expression. > >> >> > >> >> This is required mainly on android to exclude subtests that test > >> >> features that do not exist in the android driver while still being > >> >> able to run other subtests in the binary when a wildcard expression > >> >> is insufficient. > >> >> > >> >> Signed-off-by: Derek Morton <derek.j.morton@intel.com> > >> >> --- > >> >> lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > >> >> 1 file changed, 40 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb7..b9e7470 > >> >> 100644 > >> >> --- a/lib/igt_core.c > >> >> +++ b/lib/igt_core.c > >> >> @@ -207,7 +207,15 @@ > >> >> * To do that obtain the lists of subtests with "--list-subtests", which can be > >> >> * run as non-root and doesn't require the i915 driver to be loaded (or any > >> >> * intel gpu to be present). Then individual subtests can be run > >> >> with > >> >> - * "--run-subtest". Usage help for tests with subtests can be > >> >> obtained with the > >> >> + * "--run-subtest". --run-subtest accepts wildcard characters. A > >> >> + list of > >> >> + * subtests to run may be specified by using : as a seperator. A > >> >> + list of > >> >> + * subtests to exclude may be specified using ^ as a seperator. > >> >> + * > >> >> + * - --run-subtest basic* will run all subtests starting basic. > >> >> + * - --run-subtest :subtest1:subtest2: will run only subtest1 and > >> >> + subtest2 > >> >> + * - --run-subtest ^subtest1^subtest2^ will run all except > >> >> + subtest1 and subtest2 > >> >> + * > >> >> + * Usage help for tests with subtests can be obtained with the > >> >> * "--help" command line option. > >> >> */ > >> >> > >> >> @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, > >> >> extra_opt_handler, handler_data); } > >> >> > >> >> +static bool check_testlist(const char *subtest_name) { > >> >> + char *p; > >> >> + > >> >> + /* Run subtests in list > >> >> + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: > >> >> + * return true if found. > >> >> + */ > >> >> + if (run_single_subtest[0] == ':') { > >> >> + p = strstr(run_single_subtest, subtest_name); > >> >> + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) > >> >> + return true; > >> >> + } > >> >> + /* Run subtests not in list > >> >> + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ > >> >> + * return true if not found. > >> >> + */ > >> >> + else if (run_single_subtest[0] == '^') { > >> >> + p = strstr(run_single_subtest, subtest_name); > >> >> + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) > >> >> + return true; > >> >> + } > >> >> + /* Run subtests that match shell wildcard */ > >> >> + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) > >> >> + return true; > >> >> + > >> >> + return false; > >> >> +} > >> >> + > >> >> /* > >> >> * Note: Testcases which use these helpers MUST NOT output anything to stdout > >> >> * outside of places protected by igt_run_subtest checks - the > >> >> piglit @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) > >> >> } > >> >> > >> >> if (run_single_subtest) { > >> >> - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) > >> >> + > >> >> + if (check_testlist(subtest_name) == false) > >> >> return false; > >> >> else > >> >> run_single_subtest_found = true; > >> >> -- > >> >> 1.9.1 > >> >> > >> >> _______________________________________________ > >> >> Intel-gfx mailing list > >> >> Intel-gfx@lists.freedesktop.org > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > > >> >-- > >> >Ville Syrjälä > >> >Intel OTC > >> > > > > >-- > >Ville Syrjälä > >Intel OTC > >
On Wed, Jan 27, 2016 at 02:01:36PM +0000, Morton, Derek J wrote: > > > >-----Original Message----- > >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > >Sent: Wednesday, January 27, 2016 1:39 PM > >To: Ville Syrjälä > >Cc: Morton, Derek J; intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > > > >On Wed, Jan 27, 2016 at 02:32:47PM +0200, Ville Syrjälä wrote: > >> On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: > >> > Added support for specifying arbitary lists of subtests to run or to > >> > exclude from being run by using : or ^ as a seperator. > >> > > >> > :subtest1:subtest2: Will run subtest1 and subtest2 > >> > ^subtest1^subtest2^ will run all subtests except subtest1 and > >> > subtest2 > >> > >> Hmm. Getting a bit complicated perhaps. Would it be simpler to just > >> allow specifying the --r option multiple times? So we'd start with the > >> full list of subtests, and each --r option would filter the list in > >> some way? > > > >Also why not use piglit ... or what is this for? > > We don't use piglet on android. Piglet does not know about adb. Piglet > expects to be running on the system under test not on a separate host. This can be fixed, and iirc there's even been patches floating around to run piglits remotely via adb. > The main aim of this is because on android we are not testing a driver > which is drm-nightly. The kernel / display driver used on android will > not have all the features that are in the latest linux kernel. We keep > hitting problems where new subtests get added to IGT to test features > that are not yet in the android kernel. We run and report tests at a > binary level as that is what the project managers expect. We wish to be > able to run the latest versions of IGT to pick up bug fixes and useful > test changes, but want a way of being able to exclude subtests that are > not currently appropriate on android without having to exclude complete > test binaries. The specific subtests which need to be excluded will > differ depending on the HW (CHV vs BXT for example) and specific driver > version in the build under test so we need a simple mechanism to specify > the subtests to run or exclude (depending on which is more appropriate) > for each test. So a bunch of things: - Reporting at the per-binary level. Still doesn't make sense, and really imo not a technical issue. Worst case write shell scripts (or autogenerate those) with the testcase groups. - igts falling over when the kernel doesn't support a feature. This shouldn't ever happen, igt testcases are suppose to skip when the requirements aren't met. Please report any such cases so that we can fix them up in upstream igt. - Android folks breaking the libdrm abi isn't in your list, but comes up fairly often, too. Cheers, Daniel
> > >-----Original Message----- >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Wednesday, January 27, 2016 3:43 PM >To: Morton, Derek J >Cc: Daniel Vetter; Ville Syrjälä; intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >On Wed, Jan 27, 2016 at 02:01:36PM +0000, Morton, Derek J wrote: >> > >> >-----Original Message----- >> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of >> >Daniel Vetter >> >Sent: Wednesday, January 27, 2016 1:39 PM >> >To: Ville Syrjälä >> >Cc: Morton, Derek J; intel-gfx@lists.freedesktop.org >> >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >> > >> >On Wed, Jan 27, 2016 at 02:32:47PM +0200, Ville Syrjälä wrote: >> >> On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >> >> > Added support for specifying arbitary lists of subtests to run or >> >> > to exclude from being run by using : or ^ as a seperator. >> >> > >> >> > :subtest1:subtest2: Will run subtest1 and subtest2 >> >> > ^subtest1^subtest2^ will run all subtests except subtest1 and >> >> > subtest2 >> >> >> >> Hmm. Getting a bit complicated perhaps. Would it be simpler to just >> >> allow specifying the --r option multiple times? So we'd start with >> >> the full list of subtests, and each --r option would filter the >> >> list in some way? >> > >> >Also why not use piglit ... or what is this for? >> >> We don't use piglet on android. Piglet does not know about adb. Piglet >> expects to be running on the system under test not on a separate host. > >This can be fixed, and iirc there's even been patches floating around to run piglits remotely via adb. > >> The main aim of this is because on android we are not testing a driver >> which is drm-nightly. The kernel / display driver used on android will >> not have all the features that are in the latest linux kernel. We keep >> hitting problems where new subtests get added to IGT to test features >> that are not yet in the android kernel. We run and report tests at a >> binary level as that is what the project managers expect. We wish to >> be able to run the latest versions of IGT to pick up bug fixes and >> useful test changes, but want a way of being able to exclude subtests >> that are not currently appropriate on android without having to >> exclude complete test binaries. The specific subtests which need to be >> excluded will differ depending on the HW (CHV vs BXT for example) and >> specific driver version in the build under test so we need a simple >> mechanism to specify the subtests to run or exclude (depending on >> which is more appropriate) for each test. > >So a bunch of things: >- Reporting at the per-binary level. Still doesn't make sense, and really > imo not a technical issue. Worst case write shell scripts (or > autogenerate those) with the testcase groups. Not a technical issue, but easier to add additional functionality to igt than write scripts to execute the binary multiple times. Memory leaks between subtests will be hidden if the binary is executed separately for each subtest. Anything in fixtures are executed once per binary and not once per subtest so it is possible at least to get some strange behaviour difference. In either case if there is a difference between running subtests separately or together it would be a bug in the test code. There would be some overhead in scripting by the need to call with --list-subtests then remove the undesired subtests before running the rest of the subtests. Quite a lot of work compared to the size of this patch putting the functionality directly in IGT. > >- igts falling over when the kernel doesn't support a feature. This > shouldn't ever happen, igt testcases are suppose to skip when the > requirements aren't met. Please report any such cases so that we can fix > them up in upstream igt. I do not think everything is fixable upstream. We have had cases where there is an ioctl missing on android (or worse does something different, though that gets fixed eventually), or where an ioctl has been extended. If the ioctl fails the test fails. Generally if we find an issue we think is fixable in upstream IGT we will do so. //Derek > >- Android folks breaking the libdrm abi isn't in your list, but comes up > fairly often, too. > >Cheers, Daniel >-- >Daniel Vetter >Software Engineer, Intel Corporation >http://blog.ffwll.ch >
On Wed, Jan 27, 2016 at 04:45:57PM +0000, Morton, Derek J wrote: > > > > > >-----Original Message----- > >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > >Sent: Wednesday, January 27, 2016 3:43 PM > >To: Morton, Derek J > >Cc: Daniel Vetter; Ville Syrjälä; intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >- igts falling over when the kernel doesn't support a feature. This > > shouldn't ever happen, igt testcases are suppose to skip when the > > requirements aren't met. Please report any such cases so that we can fix > > them up in upstream igt. > > I do not think everything is fixable upstream. > > We have had cases where there is an ioctl missing on android (or worse > does something different, though that gets fixed eventually), or where > an ioctl has been extended. If the ioctl fails the test fails. All tests should make sure they can handle missing ioctl. There's indeed some trouble when android extends the interface in incompatible ways compared to upstream. The solution to that would be to start adding android ioctls at the very end of the range (same with flags and everything else really). Or you need a bunch of patches on top of upstream igt to adjust testcases to the Android abi. But I guess the problem with that is that Android still uses its own testsuites for both GT and display, and until that's unified it'll be just painful. And I think that pain should be beared by vpg, not upstream, since it's just part of the price to be paid for essentially forking/diverging. -Daniel
On 27/01/16 15:36, Ville Syrjälä wrote: > On Wed, Jan 27, 2016 at 03:02:15PM +0000, Morton, Derek J wrote: >>> >>> >>> -----Original Message----- >>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >>> Sent: Wednesday, January 27, 2016 2:31 PM >>> To: Morton, Derek J >>> Cc: intel-gfx@lists.freedesktop.org >>> Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >>> >>> On Wed, Jan 27, 2016 at 01:30:01PM +0000, Morton, Derek J wrote: >>>>> >>>>> >>>>> -----Original Message----- >>>>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >>>>> Sent: Wednesday, January 27, 2016 12:33 PM >>>>> To: Morton, Derek J >>>>> Cc: intel-gfx@lists.freedesktop.org >>>>> Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >>>>> >>>>> On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >>>>>> Added support for specifying arbitary lists of subtests to run or >>>>>> to exclude from being run by using : or ^ as a seperator. >>>>>> >>>>>> :subtest1:subtest2: Will run subtest1 and subtest2 >>>>>> ^subtest1^subtest2^ will run all subtests except subtest1 and >>>>>> subtest2 >>>>> >>>>> Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? >>>> >>>> By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. >>> >>> --r not -r ;) >>> >>>> Allowing it to be added multiple times could be a way of building up alist of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. >>> >>> Or could be something like --r !subtest-name >>> >>>> With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. >>>> Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? >>> >>> Hmm. I suppoose my original idea of start with full list and filter stuff out doesn't work entirely well. Eg. --r foo --r bar would run nothing. So I guess the option would have to be able to add to the list as well. >>> >>> So I guess we could make it so that '!' removes the specified test(s), no-'!' adds them, and if this is the first --r option and there's no '!' we'd clear the list entirely before adding the specified test(s) to it. >>> >>>> >>>> I think that will just end up more complicated than the simple separated list solution this patch introduces. >>> >>> I suppose. But it would avoid adding a new language which can look a bit like a weird regexp but isn't. >>> >>> Maybe if you just use ',' to separate the subtests specifications, and '!' to specify the not condition things would look a bit more standardish. >> >> I can't use ! as the shell uses it for some command history substitution. That is why I chose ^. There are very few special character the shell does not mess up :-(. > > A bunch of stuff uses ! (eg. test, find, etc.). The shell won't eat it if > there's a space after it, or you could always escape it. getopt doesn't > support it natively however so it could a pain to use the space trick. In bash(!), $ set +H prevents the abomination that is csh-derived history expansion. It defaults to off anyway in non-interactive shells, so that it doesn't mess up scripts, only interactive users, who can see the results of their foolishness and retype the failed command (in it's entirety, because failed substitutions don't go into the history list for recall and editing!). History expansion uses ^ as well as !, so just switching to that might not eliminate all surprises, although by default it only triggers substitution at the start of a command. In patterns, bash recognises both ^ and ! as negating the pattern. I suggest you do the same; and for suboptions, how about allowing either comma or colon equally as separators? So, how about: 0. start with a full list 1. each new --run-subtest option filters the current list (AND) 2. each subargument to a --run-subtest expands the next filter (OR) 3. an argument beginning with ! or ^ negates the match 4. no exceptions Then: t --run-subtest pat1 # run matching tests only t --run-subtest ^pat1 # exclude matching tests t --run-subtest pat1,pat2 # run tests matching EITHER t --run-subtest ^pat1,pat2 # excludes matching EITHER t --run-subtest pat1 --run-subtest pat2 # run tests matching BOTH t --run-subtest pat1 --run-subtest !pat2 # pat1 except pat2 t --run-subtest ^pat1,^pat2 # not allowed (boolean priority is a bit odd here, it's OR highest, then NOT, then AND, which isn't the usual (NOT>AND>OR) but it's not too bad). I don't think you can get exclude-matching-both, but is that a likely usecase? Or alternatively: 0. start with an empty list 1. each new --run-subtest option adds to the current list (OR) 2. each subargument to a --run-subtest filters the next addition (AND) 3. a subargument beginning with ! or ^ negates the match 4. iff no --run-subtests then assume --run-subtests '*' Then: t --run-subtest pat1 # run matching tests only t --run-subtest ^pat1 # exclude matching tests t --run-subtest pat1 --run-subtest pat2 # run tests matching EITHER t --run-subtest ^pat1:^pat2 # excludes matching EITHER t --run-subtest pat1:pat2 # run tests matching BOTH t --run-subtest pat1:^pat2 # pat1 except pat2 t --run-subtest !pat1 --run-subtest !pat2 # excludes matching BOTH This is slightly more complex, but also slightly more powerful, and the boolean priority is now as expected (NOT first, then AND, then OR). We get exclude-matching-both using de Morgan to rewrite NOT(a AND b) as (NOT a) OR (NOT b). .Dave.
> > >-----Original Message----- >From: Gordon, David S >Sent: Thursday, January 28, 2016 8:35 AM >To: intel-gfx@lists.freedesktop.org; Morton, Derek J; Ville Syrjälä >Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. > >On 27/01/16 15:36, Ville Syrjälä wrote: >> On Wed, Jan 27, 2016 at 03:02:15PM +0000, Morton, Derek J wrote: >>>> >>>> >>>> -----Original Message----- >>>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >>>> Sent: Wednesday, January 27, 2016 2:31 PM >>>> To: Morton, Derek J >>>> Cc: intel-gfx@lists.freedesktop.org >>>> Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >>>> >>>> On Wed, Jan 27, 2016 at 01:30:01PM +0000, Morton, Derek J wrote: >>>>>> >>>>>> >>>>>> -----Original Message----- >>>>>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >>>>>> Sent: Wednesday, January 27, 2016 12:33 PM >>>>>> To: Morton, Derek J >>>>>> Cc: intel-gfx@lists.freedesktop.org >>>>>> Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality. >>>>>> >>>>>> On Wed, Jan 27, 2016 at 10:05:56AM +0000, Derek Morton wrote: >>>>>>> Added support for specifying arbitary lists of subtests to run or >>>>>>> to exclude from being run by using : or ^ as a seperator. >>>>>>> >>>>>>> :subtest1:subtest2: Will run subtest1 and subtest2 >>>>>>> ^subtest1^subtest2^ will run all subtests except subtest1 and >>>>>>> subtest2 >>>>>> >>>>>> Hmm. Getting a bit complicated perhaps. Would it be simpler to just allow specifying the --r option multiple times? So we'd start with the full list of subtests, and each --r option would filter the list in some way? >>>>> >>>>> By -r I assume you mean --run-subtest? Running a test with -r <subtest> just prints the usage options at the moment. >>>> >>>> --r not -r ;) >>>> >>>>> Allowing it to be added multiple times could be a way of building up alist of subtests to run, but a completely new command line option would need to then be added (--skip-subtest?) to allow building up a list of subtests to be skipped. >>>> >>>> Or could be something like --r !subtest-name >>>> >>>>> With my change as is, it allows a string to be passed to the test which details which subtests should be run. If a new parameter such as --skip-subtest is added then it would require knowledge of whether the string contains subtests to run or not to run. That would complicate any scripts that use this to automate testing. >>>>> Allowing --run-subtest (and --skip-subtest) to be specified multiple times will also complicate the code in igt_core.c. Currently there is a simple call to strdup(). If --run-subtest can be specified multiple times the code will have to deal with concatenating strings and any memory reallocation that needs. Also how to deal with the possibility of multiple wildcard expressions? >>>> >>>> Hmm. I suppoose my original idea of start with full list and filter stuff out doesn't work entirely well. Eg. --r foo --r bar would run nothing. So I guess the option would have to be able to add to the list as well. >>>> >>>> So I guess we could make it so that '!' removes the specified test(s), no-'!' adds them, and if this is the first --r option and there's no '!' we'd clear the list entirely before adding the specified test(s) to it. >>>> >>>>> >>>>> I think that will just end up more complicated than the simple separated list solution this patch introduces. >>>> >>>> I suppose. But it would avoid adding a new language which can look a bit like a weird regexp but isn't. >>>> >>>> Maybe if you just use ',' to separate the subtests specifications, and '!' to specify the not condition things would look a bit more standardish. >>> >>> I can't use ! as the shell uses it for some command history substitution. That is why I chose ^. There are very few special character the shell does not mess up :-(. >> >> A bunch of stuff uses ! (eg. test, find, etc.). The shell won't eat it >> if there's a space after it, or you could always escape it. getopt >> doesn't support it natively however so it could a pain to use the space trick. > >In bash(!), > $ set +H >prevents the abomination that is csh-derived history expansion. It defaults to off anyway in non-interactive shells, so that it doesn't mess up scripts, only interactive users, who can see the results of their foolishness and retype the failed command (in it's entirety, because failed substitutions don't go into the history list for recall and editing!). > >History expansion uses ^ as well as !, so just switching to that might not eliminate all surprises, although by default it only triggers substitution at the start of a command. > >In patterns, bash recognises both ^ and ! as negating the pattern. I suggest you do the same; and for suboptions, how about allowing either comma or colon equally as separators? > >So, how about: > 0. start with a full list > 1. each new --run-subtest option filters the current list (AND) > 2. each subargument to a --run-subtest expands the next filter (OR) > 3. an argument beginning with ! or ^ negates the match > 4. no exceptions > >Then: > t --run-subtest pat1 # run matching tests only > t --run-subtest ^pat1 # exclude matching tests > t --run-subtest pat1,pat2 # run tests matching EITHER > t --run-subtest ^pat1,pat2 # excludes matching EITHER > t --run-subtest pat1 --run-subtest pat2 # run tests matching BOTH > t --run-subtest pat1 --run-subtest !pat2 # pat1 except pat2 > t --run-subtest ^pat1,^pat2 # not allowed > >(boolean priority is a bit odd here, it's OR highest, then NOT, then AND, which isn't the usual (NOT>AND>OR) but it's not too bad). I don't think you can get exclude-matching-both, but is that a likely usecase? > >Or alternatively: > 0. start with an empty list > 1. each new --run-subtest option adds to the current list (OR) > 2. each subargument to a --run-subtest filters the next addition (AND) > 3. a subargument beginning with ! or ^ negates the match > 4. iff no --run-subtests then assume --run-subtests '*' > >Then: > t --run-subtest pat1 # run matching tests only > t --run-subtest ^pat1 # exclude matching tests > t --run-subtest pat1 --run-subtest pat2 # run tests matching EITHER > t --run-subtest ^pat1:^pat2 # excludes matching EITHER > t --run-subtest pat1:pat2 # run tests matching BOTH > t --run-subtest pat1:^pat2 # pat1 except pat2 > t --run-subtest !pat1 --run-subtest !pat2 # excludes matching BOTH > >This is slightly more complex, but also slightly more powerful, and the boolean priority is now as expected (NOT first, then AND, then OR). We get exclude-matching-both using de Morgan to rewrite NOT(a AND b) as (NOT a) OR (NOT b). I don't want to add support for specifying --run-subtest multiple times. That will excessively complicate the code with the existing wildcard support for no real improvement in functionality. Also it is useful to be able to either run specified tests or run all except specified tests depending on what you want to do. If --run-subtest is allowed to be specified multiple times then you need some rule to specify whether starting with a full or empty list which could be confusing. Trying to document (and implement) all this behaviour is going to lead to a lot of confusion compared to simply allowing a comma separated list. I can add support for both ! and ^ as the not operator. > >.Dave. >
I really like the thought of having this functionality in i-g-t, especially if combined with my patches that allows for categorising subtests (I'll submit a new version of those patches soon, since they never got merged last time around). I'll refrain from bikeshedding on the syntax. In the longer run I think a rewrite of the subtest logic to allow not only specifying what tests to run and/or exclude, but also to specify what *order* to run the tests in (if reordering is possible) and to randomise the order would be useful. Since some testcases contain so many subtests that having those testcases be a part of the CI/Piglit testruns isn't possible, we need some way to have those tests run anyway. My current train of thought is to either traverse through the tests (1..20, 21..40, 41..60, etc.; which would require piglit to keep state, which it currently doesn't) or to randomise a set of subtests that would take something like 30s. This might mean that things could slip under the radar for a long time, but compared to today's situation where these tests aren't run at all it'd still be an improvement. Just my 2¢. Kind regards, David Weinehall
diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb7..b9e7470 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -207,7 +207,15 @@ * To do that obtain the lists of subtests with "--list-subtests", which can be * run as non-root and doesn't require the i915 driver to be loaded (or any * intel gpu to be present). Then individual subtests can be run with - * "--run-subtest". Usage help for tests with subtests can be obtained with the + * "--run-subtest". --run-subtest accepts wildcard characters. A list of + * subtests to run may be specified by using : as a seperator. A list of + * subtests to exclude may be specified using ^ as a seperator. + * + * - --run-subtest basic* will run all subtests starting basic. + * - --run-subtest :subtest1:subtest2: will run only subtest1 and subtest2 + * - --run-subtest ^subtest1^subtest2^ will run all except subtest1 and subtest2 + * + * Usage help for tests with subtests can be obtained with the * "--help" command line option. */ @@ -786,6 +794,35 @@ void igt_simple_init_parse_opts(int *argc, char **argv, extra_opt_handler, handler_data); } +static bool check_testlist(const char *subtest_name) +{ + char *p; + + /* Run subtests in list + * Look for subtest_name in list of form :subtest1:subtest2:subtest3: + * return true if found. + */ + if (run_single_subtest[0] == ':') { + p = strstr(run_single_subtest, subtest_name); + if ((p) && (*(p-1) == ':') && (*(p+strlen(subtest_name)) == ':' )) + return true; + } + /* Run subtests not in list + * Look for subtest_name in list of form ^test1^subtest2^subtest3^ + * return true if not found. + */ + else if (run_single_subtest[0] == '^') { + p = strstr(run_single_subtest, subtest_name); + if (!((p) && (*(p-1) == '^') && (*(p+strlen(subtest_name)) == '^' ))) + return true; + } + /* Run subtests that match shell wildcard */ + else if (fnmatch(run_single_subtest, subtest_name, 0) == 0) + return true; + + return false; +} + /* * Note: Testcases which use these helpers MUST NOT output anything to stdout * outside of places protected by igt_run_subtest checks - the piglit @@ -814,7 +851,8 @@ bool __igt_run_subtest(const char *subtest_name) } if (run_single_subtest) { - if (fnmatch(run_single_subtest, subtest_name, 0) != 0) + + if (check_testlist(subtest_name) == false) return false; else run_single_subtest_found = true;
Added support for specifying arbitary lists of subtests to run or to exclude from being run by using : or ^ as a seperator. :subtest1:subtest2: Will run subtest1 and subtest2 ^subtest1^subtest2^ will run all subtests except subtest1 and subtest2 Any subtest string not starting : or ^ is treated as a normal wildcard expression. This is required mainly on android to exclude subtests that test features that do not exist in the android driver while still being able to run other subtests in the binary when a wildcard expression is insufficient. Signed-off-by: Derek Morton <derek.j.morton@intel.com> --- lib/igt_core.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-)