Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Cc: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Taylor Blau <me@ttaylorr.com>, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run
Date: Fri, 16 Oct 2020 19:24:15 +0100
Message-ID: <2fd1231c-9df3-623d-9ae1-0d66526db618@gmail.com> (raw)
In-Reply-To: <CABPp-BHkykWh8L_FYhLR1BCCpPDmc_2q+Tccg_yZ7W8ZHZ4WsA@mail.gmail.com>

Hi Elijah

On 16/10/2020 18:27, Elijah Newren wrote:
> Hi Phillip,
> 
> On Fri, Oct 16, 2020 at 4:41 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Elijah
>>
>> On 14/10/2020 22:13, Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@gmail.com>
>>>
>>> Many of our test scripts have several "setup" tests.  It's a lot easier
>>> to say
>>>
>>>      ./t0050-filesystem.sh --run=setup,9
>>>
>>> in order to run all the setup tests as well as test #9, than it is to
>>> track down what all the setup tests are and enter all their numbers in
>>> the list.  Also, I often find myself wanting to run just one or a couple
>>> tests from the test file, but I don't know the numbering of any of the
>>> tests -- to get it I either have to first run the whole test file (or
>>> start counting by hand or figure out some other clever but non-obvious
>>> tricks).  It's really convenient to be able to just look at the test
>>> description(s) and then run
>>>
>>>      ./t6416-recursive-corner-cases.sh --run=symlink
>>>
>>> or
>>>
>>>      ./t6402-merge-rename.sh --run='setup,unnecessary update'
>>
>> The beginning of match_test_selector_list() looks like
>>
>> match_test_selector_list () {
>>          title="$1"
>>          shift
>>          arg="$1"
>>          shift
>>          test -z "$1" && return 0
>>
>>          # Both commas and whitespace are accepted as separators.
>>          OLDIFS=$IFS
>>          IFS='   ,'
>>          set -- $1
>>          IFS=$OLDIFS
>>
>>          # If the first selector is negative we include by default.
>>          include=
>>          case "$1" in
>>                  !*) include=t ;;
>>          esac
>>
>>          for selector
>>          do
>>
>> If I'm reading it correctly the selectors are split on commas and
>> whitespace which would mean we cannot match on "unnecessary update". I
>> think we definitely want the ability to include whitespace in the
>> selectors in order to be able to narrow down the tests that are run. I'm
>> not sure that there is much value in splitting numbers on whitespace as
>> it would mean the user has to quote them on the command line so we can
>> probably just do 'IFS=,'. We'd also need to keep IFS as ',' in the case
>> statement you add below as well rather than restoring it straight after
>> the 'set' statement above.
> 
> Given that t/README explicitly shows examples of space-separated lists
> of numbers,

That's a shame

> I'm worried we're breaking long-built expectations of
> other developers by changing IFS here. 

I agree

> Perhaps I could instead add
> the following paragraph to t/README:
> 
> Note: The argument to --run is split on commas and whitespace into
> separate strings, numbers, and ranges, and picks all tests that match
> any of individual selection criteria.  If the substring you want to
> match from the description text includes a comma or space, use the
> glob character '?' instead.  For example --run='unnecessary?update
> timing' would match on all tests that match either the glob
> *unnecessary?update* or the glob *timing*.
> 
> Does that address your concern?  The '?' will of course match on
> characters other than a space or comma, but the odds that it actually
> matches anything other than what you want is pretty slim, so I suspect
> that is good enough.

'unnecessary?update' is pretty ugly but I agree false matches are 
unlikely to be a problem it practice. Your suggested paragraph looks 
good to me

Thanks

Phillip


  reply index

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-12 23:26 [PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Elijah Newren via GitGitGadget
2020-10-12 23:26 ` [PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run Elijah Newren via GitGitGadget
2020-10-13 15:39   ` Taylor Blau
2020-10-13 17:23     ` Elijah Newren
2020-10-13 17:28       ` Taylor Blau
2020-10-13 18:21         ` Elijah Newren
2020-10-12 23:26 ` [PATCH 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-12 23:26 ` [PATCH 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget
2020-10-13 15:44   ` Taylor Blau
2020-10-13 17:56     ` Elijah Newren
2020-10-13 19:27       ` Junio C Hamano
2020-10-13 13:08 ` [PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Phillip Wood
2020-10-15 11:48   ` Johannes Schindelin
2020-10-15 13:42     ` Phillip Wood
2020-10-13 19:19 ` [PATCH v2 " Elijah Newren via GitGitGadget
2020-10-13 19:19   ` [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run Elijah Newren via GitGitGadget
2020-10-14 17:04     ` Jeff King
2020-10-14 17:46       ` Junio C Hamano
2020-10-14 18:07         ` Jeff King
2020-10-14 19:24           ` Junio C Hamano
2020-10-14 19:41             ` Elijah Newren
2020-10-14 19:49               ` Jeff King
2020-10-15 16:09                 ` Junio C Hamano
2020-10-15 15:59               ` Junio C Hamano
2020-10-14 17:50       ` Elijah Newren
2020-10-14 17:57         ` Junio C Hamano
2020-10-14 19:12           ` Elijah Newren
2020-10-15 16:10             ` Junio C Hamano
2020-10-14 18:09         ` Jeff King
2020-10-14 19:02           ` Elijah Newren
2020-10-15 16:20       ` Junio C Hamano
2020-10-15 19:46         ` Jeff King
2020-10-15 20:08           ` Junio C Hamano
2020-10-16  0:38             ` Jeff King
2020-10-16 16:16               ` Junio C Hamano
2020-10-16 16:30                 ` Junio C Hamano
2020-10-16 20:06                 ` Jeff King
2020-10-16 20:22                   ` Junio C Hamano
2020-10-13 19:19   ` [PATCH v2 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-13 19:19   ` [PATCH v2 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget
2020-10-14 16:53     ` Jeff King
2020-10-14 17:39       ` Elijah Newren
2020-10-14 17:55         ` Jeff King
2020-10-14 17:57           ` Elijah Newren
2020-10-13 19:49   ` [PATCH v2 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Taylor Blau
2020-10-14 21:13   ` [PATCH v3 " Elijah Newren via GitGitGadget
2020-10-14 21:13     ` [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run Elijah Newren via GitGitGadget
2020-10-16 11:40       ` Phillip Wood
2020-10-16 17:27         ` Elijah Newren
2020-10-16 18:24           ` Phillip Wood [this message]
2020-10-16 20:02           ` Junio C Hamano
2020-10-16 20:07             ` Jeff King
2020-10-14 21:13     ` [PATCH v3 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-14 21:13     ` [PATCH v3 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget
2020-10-15 19:48     ` [PATCH v3 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Jeff King
2020-10-16 19:28     ` [PATCH v4 " Elijah Newren via GitGitGadget
2020-10-16 19:28       ` [PATCH v4 1/3] test-lib: allow selecting tests by substring/glob with --run Elijah Newren via GitGitGadget
2020-10-16 19:28       ` [PATCH v4 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-16 19:28       ` [PATCH v4 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget
2020-10-16 20:25       ` [PATCH v4 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Junio C Hamano
2020-10-16 20:56         ` Elijah Newren
2020-10-16 22:50       ` [PATCH v5 " Elijah Newren via GitGitGadget
2020-10-16 22:50         ` [PATCH v5 1/3] test-lib: allow selecting tests by substring/glob with --run Elijah Newren via GitGitGadget
2020-10-16 23:25           ` Andrei Rybak
2020-10-17 23:43             ` Elijah Newren
2020-10-16 22:50         ` [PATCH v5 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-16 22:50         ` [PATCH v5 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget
2020-10-18  0:23         ` [PATCH v6 0/3] Make test selection easier by specifying description substrings instead of just numeric counters Elijah Newren via GitGitGadget
2020-10-18  0:23           ` [PATCH v6 1/3] test-lib: allow selecting tests by substring/glob with --run Elijah Newren via GitGitGadget
2020-10-18  0:23           ` [PATCH v6 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms Elijah Newren via GitGitGadget
2020-10-18  0:23           ` [PATCH v6 3/3] test-lib: reduce verbosity of skipped tests Elijah Newren via GitGitGadget

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=2fd1231c-9df3-623d-9ae1-0d66526db618@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    /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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git