All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: intel-gfx@lists.freedesktop.org, derek.j.morton@intel.com,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality.
Date: Thu, 28 Jan 2016 08:35:11 +0000	[thread overview]
Message-ID: <56A9D2BF.1040208@intel.com> (raw)
In-Reply-To: <20160127153609.GE23290@intel.com>

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-01-28  8:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-27 10:05 [PATCH i-g-t] lib/igt_core.c: Expand --run-subtest functionality Derek Morton
2016-01-27 12:32 ` Ville Syrjälä
2016-01-27 13:30   ` Morton, Derek J
2016-01-27 14:30     ` Ville Syrjälä
2016-01-27 15:02       ` Morton, Derek J
2016-01-27 15:36         ` Ville Syrjälä
2016-01-28  8:35           ` Dave Gordon [this message]
2016-01-28 10:47             ` Morton, Derek J
2016-01-27 13:39   ` Daniel Vetter
2016-01-27 14:01     ` Morton, Derek J
2016-01-27 15:42       ` Daniel Vetter
2016-01-27 16:45         ` Morton, Derek J
2016-01-27 17:58           ` Daniel Vetter
2016-02-04 11:41 ` David Weinehall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A9D2BF.1040208@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=derek.j.morton@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.