[PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Elijah Newren (3):
  test-lib: allow selecting tests by substring/regex with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 26 +++++++++++++++++------
 t/t0000-basic.sh             | 41 ++++++++++++++++++++++--------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 15 +++++++------
 5 files changed, 54 insertions(+), 32 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v1
Pull-Request: https://github.com/git/git/pull/878
-- 
gitgitgadget

[PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 26 +++++++++++++++++++-------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 13 +++++++------
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..23639c5bcf 100644
--- a/t/README
+++ b/t/README
@@ -258,13 +258,13 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
+The argument for --run, <test-selector>, is a list of description
+substrings or regexes or individual test numbers or ranges with an
+optional negation prefix that define what tests in a test suite to
+include in the run.  A range is two numbers separated with a dash and
+matches a range of tests with both ends been included.  You may omit
+the first or the second number to mean "from the first test" or "up to
+the very last test" respectively.
 
 Optional prefix of '!' means that the test or a range of tests
 should be excluded from the run.
@@ -317,6 +317,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/regex to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..07105b2078 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..2aca398e1e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,6 +769,8 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,8 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					echo "$title" | grep -q "$selector" && return
+					continue
 				fi
 		esac
 
@@ -1031,7 +1032,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

Re: [PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run

[Taylor Blau]

On Mon, Oct 12, 2020 at 11:26:02PM +0000, Elijah Newren via GitGitGadget wrote:
> diff --git a/t/README b/t/README
> index 2adaf7c2d2..23639c5bcf 100644
> --- a/t/README
> +++ b/t/README
> @@ -258,13 +258,13 @@ For an individual test suite --run could be used to specify that
>  only some tests should be run or that some tests should be
>  excluded from a run.
>
> -The argument for --run is a list of individual test numbers or
> -ranges with an optional negation prefix that define what tests in
> -a test suite to include in the run.  A range is two numbers
> -separated with a dash and matches a range of tests with both ends
> -been included.  You may omit the first or the second number to
> -mean "from the first test" or "up to the very last test"
> -respectively.
> +The argument for --run, <test-selector>, is a list of description
> +substrings or regexes or individual test numbers or ranges with an
> +optional negation prefix that define what tests in a test suite to
> +include in the run.  A range is two numbers separated with a dash and
> +matches a range of tests with both ends been included.  You may omit
> +the first or the second number to mean "from the first test" or "up to
> +the very last test" respectively.
>
>  Optional prefix of '!' means that the test or a range of tests
>  should be excluded from the run.

This piece of documentation looks to be now out of date; it mentions a
"test or range of tests", but that only covers a two of the now four
ways to describe a test.

The rest of the patch looks great to me; and I, too, am very excited
about it.

Thanks,
Taylor

Re: [PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Tue, Oct 13, 2020 at 8:39 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Mon, Oct 12, 2020 at 11:26:02PM +0000, Elijah Newren via GitGitGadget wrote:
> > diff --git a/t/README b/t/README
> > index 2adaf7c2d2..23639c5bcf 100644
> > --- a/t/README
> > +++ b/t/README
> > @@ -258,13 +258,13 @@ For an individual test suite --run could be used to specify that
> >  only some tests should be run or that some tests should be
> >  excluded from a run.
> >
> > -The argument for --run is a list of individual test numbers or
> > -ranges with an optional negation prefix that define what tests in
> > -a test suite to include in the run.  A range is two numbers
> > -separated with a dash and matches a range of tests with both ends
> > -been included.  You may omit the first or the second number to
> > -mean "from the first test" or "up to the very last test"
> > -respectively.
> > +The argument for --run, <test-selector>, is a list of description
> > +substrings or regexes or individual test numbers or ranges with an
> > +optional negation prefix that define what tests in a test suite to
> > +include in the run.  A range is two numbers separated with a dash and
> > +matches a range of tests with both ends been included.  You may omit
> > +the first or the second number to mean "from the first test" or "up to
> > +the very last test" respectively.
> >
> >  Optional prefix of '!' means that the test or a range of tests
> >  should be excluded from the run.
>
> This piece of documentation looks to be now out of date; it mentions a
> "test or range of tests", but that only covers a two of the now four
> ways to describe a test.

Personally, I had read "test or range of tests" as general enough to
cover all the cases (I would have agreed that "test *number* or range
of tests" would sound out of date), and just left it.  However, if
there's something clearer, I'm happy to adopt it.  Do you have an
alternate wording suggestion?

> The rest of the patch looks great to me; and I, too, am very excited
> about it.
>
> Thanks,
> Taylor

Re: [PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run

[Taylor Blau]

On Tue, Oct 13, 2020 at 10:23:32AM -0700, Elijah Newren wrote:
> On Tue, Oct 13, 2020 at 8:39 AM Taylor Blau <me@ttaylorr.com> wrote:
> > This piece of documentation looks to be now out of date; it mentions a
> > "test or range of tests", but that only covers a two of the now four
> > ways to describe a test.
>
> Personally, I had read "test or range of tests" as general enough to
> cover all the cases (I would have agreed that "test *number* or range
> of tests" would sound out of date), and just left it.  However, if
> there's something clearer, I'm happy to adopt it.  Do you have an
> alternate wording suggestion?

Maybe "test selectors"? You could even say something like "test
selectors (e.g., the test number, a range, a substring/regular
expression)". I don't know, though; I don't feel strongly about it.

Thanks,
Taylor

Re: [PATCH 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Tue, Oct 13, 2020 at 10:28 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Tue, Oct 13, 2020 at 10:23:32AM -0700, Elijah Newren wrote:
> > On Tue, Oct 13, 2020 at 8:39 AM Taylor Blau <me@ttaylorr.com> wrote:
> > > This piece of documentation looks to be now out of date; it mentions a
> > > "test or range of tests", but that only covers a two of the now four
> > > ways to describe a test.
> >
> > Personally, I had read "test or range of tests" as general enough to
> > cover all the cases (I would have agreed that "test *number* or range
> > of tests" would sound out of date), and just left it.  However, if
> > there's something clearer, I'm happy to adopt it.  Do you have an
> > alternate wording suggestion?
>
> Maybe "test selectors"? You could even say something like "test
> selectors (e.g., the test number, a range, a substring/regular
> expression)". I don't know, though; I don't feel strongly about it.

Earlier in the document, the --run option is defined as
   --run=<test-selector>::
which might suggest to some readers that "test-selector" is the
combination of all the selections and thus that you can only negate
the whole series rather than individual components of it.

Perhaps we can just nuke this sentence with a slight tweak of the
paragraph before it to include slightly more about the "optional
negation prefix".  I'll send an updated series that does that.

[PATCH 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
adds up to hundreds or thousands of lines of irrelevant output that make
it very hard to fish out the relevant results you were looking for.
Simplify the output for skipped tests down to just showing the one-line
descriptions.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2aca398e1e..7602bbe9e9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1059,7 +1059,7 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
+		say_color skip >&3 "skipping test: $1"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget

Re: [PATCH 3/3] test-lib: reduce verbosity of skipped tests

[Taylor Blau]

On Mon, Oct 12, 2020 at 11:26:04PM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> When using the --run flag to run just two or three tests from a test
> file which contains several dozen tests, having every skipped test print
> out dozens of lines of output for the test code for that skipped test
> adds up to hundreds or thousands of lines of irrelevant output that make
> it very hard to fish out the relevant results you were looking for.
> Simplify the output for skipped tests down to just showing the one-line
> descriptions.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  t/test-lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 2aca398e1e..7602bbe9e9 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1059,7 +1059,7 @@ test_skip () {
>  				"      <skipped message=\"$message\" />"
>  		fi
>
> -		say_color skip >&3 "skipping test: $@"
> +		say_color skip >&3 "skipping test: $1"

I would be comfortable going further than this and removing this line
entirely. We need the "ok $test_count # skip" below since it's part of
our TAP output, but now the output looks somewhat redundant.

With this patch running a test that I'm working on with `--run=...`, I
get output that looks like:

  skipping test: writing bitmaps via command-line can duplicate .keep objects
  ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run)

  skipping test: writing bitmaps via config can duplicate .keep objects
  ok 3 # skip writing bitmaps via config can duplicate .keep objects (--run)

Scanning over the same test description twice per skipped test makes the
output difficult (but still much easier than before) to scan. What do
you think about either of the following:

  skipping test: writing bitmaps via command-line can duplicate .keep objects
  ok 2 # skip (--run)

or:

  ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run)

I have a slight preference towards the latter, since it keeps more of
the information in the TAP line, and it cuts the total line count of
output from skipped tests in half.

For what it's worth, I'd also be fine with the patch as-is.

Thanks,
Taylor

Re: [PATCH 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren]

On Tue, Oct 13, 2020 at 8:44 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Mon, Oct 12, 2020 at 11:26:04PM +0000, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > When using the --run flag to run just two or three tests from a test
> > file which contains several dozen tests, having every skipped test print
> > out dozens of lines of output for the test code for that skipped test
> > adds up to hundreds or thousands of lines of irrelevant output that make
> > it very hard to fish out the relevant results you were looking for.
> > Simplify the output for skipped tests down to just showing the one-line
> > descriptions.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  t/test-lib.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 2aca398e1e..7602bbe9e9 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -1059,7 +1059,7 @@ test_skip () {
> >                               "      <skipped message=\"$message\" />"
> >               fi
> >
> > -             say_color skip >&3 "skipping test: $@"
> > +             say_color skip >&3 "skipping test: $1"
>
> I would be comfortable going further than this and removing this line
> entirely. We need the "ok $test_count # skip" below since it's part of
> our TAP output, but now the output looks somewhat redundant.
>
> With this patch running a test that I'm working on with `--run=...`, I
> get output that looks like:
>
>   skipping test: writing bitmaps via command-line can duplicate .keep objects
>   ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run)
>
>   skipping test: writing bitmaps via config can duplicate .keep objects
>   ok 3 # skip writing bitmaps via config can duplicate .keep objects (--run)
>
> Scanning over the same test description twice per skipped test makes the
> output difficult (but still much easier than before) to scan. What do
> you think about either of the following:
>
>   skipping test: writing bitmaps via command-line can duplicate .keep objects
>   ok 2 # skip (--run)
>
> or:
>
>   ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run)
>
> I have a slight preference towards the latter, since it keeps more of
> the information in the TAP line, and it cuts the total line count of
> output from skipped tests in half.
>
> For what it's worth, I'd also be fine with the patch as-is.

I was worried that shortening it as much as I did would run into
objections for some obscure reason.  But if no one objects, I think
I'd also prefer your latter suggestion for shortening it even more.
It looks like the dual output for skipped tests comes from commit
04ece59399 ("GIT_SKIP_TESTS: allow users to omit tests that are known
to break", 2006-12-28) by Junio, so it'd be nice to hear his opinion
on how much we shorten it.

Re: [PATCH 3/3] test-lib: reduce verbosity of skipped tests

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

>>   skipping test: writing bitmaps via command-line can duplicate .keep objects
>>   ok 2 # skip (--run)
>>
>> or:
>>
>>   ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run)
>>
>> I have a slight preference towards the latter, since it keeps more of
>> the information in the TAP line, and it cuts the total line count of
>> output from skipped tests in half.
>>
>> For what it's worth, I'd also be fine with the patch as-is.
>
> I was worried that shortening it as much as I did would run into
> objections for some obscure reason.  But if no one objects, I think
> I'd also prefer your latter suggestion for shortening it even more.
> It looks like the dual output for skipped tests comes from commit
> 04ece59399 ("GIT_SKIP_TESTS: allow users to omit tests that are known
> to break", 2006-12-28) by Junio, so it'd be nice to hear his opinion
> on how much we shorten it.

I am afraid that the surrounding code has changed so heavily that
expertise as the author of 04ece593 would not count that much in the
context of today's code.  Back in the late 2006 version of the code,
text_expect_success asked test_skip to see if a test should be
skipped, and when the latter says yes (which also gave "skipping
test" to the verbose output and "skip $test_count: $1" to normal
output), test_expect_success did not even call test_ok_ or
test_failure_.  So "ok 2 # skip ..." we see in the quoted part of
your exchange above is probably a much more recent invention.

As long as we can convince ourselves that writing similar things to
fd #1 (for tap in "ok $count # skip ($why)" format) and fd #3 is
redundant and not useful, I think removing the utterance to fd #3
in this codepath may be a reasonable thing to do.

    ... goes and scans for ">&3" in test-lib.sh and
    ... test-lib-functions.sh

OK, I think we can safely lose the one we send for the "verbose"
case here and that would turn our use of fd #3 more consistent with
how it is used by other parts of the test framework.

Thanks.

Re: [PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Phillip Wood]

Hi Elijah

I'm worryingly excited by this series

On 13/10/2020 00:26, Elijah Newren via GitGitGadget wrote:
> This patch series enables me to change
> 
>   ./t9999-my-test.sh --run=1-3,5,17,19
> 
> into
> 
>   ./t9999-my-test.sh --run=setup,rename,symlink
> 
> and have it pick out tests with "setup", "rename", or "symlink" in their
> description and run those. Saves me a lot of time, especially since numbers
> for tests aren't readily accessible. The easiest way for me to get the
> numbers corresponding to the tests I want to run, is to run all the tests
> and look at the output to match up the descriptions with their numbers --
> thus defeating the point of selecting just a subset of the tests to run in
> the first place.

Yes it's a real pain to have to run all the tests to find the numbers of 
the ones you actually wanted to run. I like patch 3 as well, without it 
if you're only running a couple of tests it can be hard to find then 
amongst all the skipped tests. I had a quick read of the patches and 
they looked fine to me.

Best Wishes

Phillip

> Elijah Newren (3):
>    test-lib: allow selecting tests by substring/regex with --run
>    t6006, t6012: adjust tests to use 'setup' instead of synonyms
>    test-lib: reduce verbosity of skipped tests
> 
>   t/README                     | 26 +++++++++++++++++------
>   t/t0000-basic.sh             | 41 ++++++++++++++++++++++--------------
>   t/t6006-rev-list-format.sh   |  2 +-
>   t/t6012-rev-list-simplify.sh |  2 +-
>   t/test-lib.sh                | 15 +++++++------
>   5 files changed, 54 insertions(+), 32 deletions(-)
> 
> 
> base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v1
> Pull-Request: https://github.com/git/git/pull/878
> 

Re: [PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Johannes Schindelin]

Hi,

On Tue, 13 Oct 2020, Phillip Wood wrote:

> I'm worryingly excited by this series

FWIW I am in the same boat.

> On 13/10/2020 00:26, Elijah Newren via GitGitGadget wrote:
> > This patch series enables me to change
> >
> >   ./t9999-my-test.sh --run=1-3,5,17,19
> >
> > into
> >
> >   ./t9999-my-test.sh --run=setup,rename,symlink
> >
> > and have it pick out tests with "setup", "rename", or "symlink" in their
> > description and run those. Saves me a lot of time, especially since numbers
> > for tests aren't readily accessible. The easiest way for me to get the
> > numbers corresponding to the tests I want to run, is to run all the tests
> > and look at the output to match up the descriptions with their numbers --
> > thus defeating the point of selecting just a subset of the tests to run in
> > the first place.
>
> Yes it's a real pain to have to run all the tests to find the numbers of the
> ones you actually wanted to run. I like patch 3 as well, without it if you're
> only running a couple of tests it can be hard to find then amongst all the
> skipped tests. I had a quick read of the patches and they looked fine to me.

I learned the trick to run with `--run=0` just to figure out the numbers
in a recent Outreachy project, I think it was Slavica who taught me.

But yes, without patch 3, the output looks awfully verbose and you have to
scroll back an insane amount of lines for long test scripts.

Giddy,
Dscho

Re: [PATCH 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Phillip Wood]

On 15/10/2020 12:48, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 13 Oct 2020, Phillip Wood wrote:
> 
>> I'm worryingly excited by this series
> 
> FWIW I am in the same boat.

I'm glad it's not just me!

>> On 13/10/2020 00:26, Elijah Newren via GitGitGadget wrote:
>>> This patch series enables me to change
>>>
>>>    ./t9999-my-test.sh --run=1-3,5,17,19
>>>
>>> into
>>>
>>>    ./t9999-my-test.sh --run=setup,rename,symlink
>>>
>>> and have it pick out tests with "setup", "rename", or "symlink" in their
>>> description and run those. Saves me a lot of time, especially since numbers
>>> for tests aren't readily accessible. The easiest way for me to get the
>>> numbers corresponding to the tests I want to run, is to run all the tests
>>> and look at the output to match up the descriptions with their numbers --
>>> thus defeating the point of selecting just a subset of the tests to run in
>>> the first place.
>>
>> Yes it's a real pain to have to run all the tests to find the numbers of the
>> ones you actually wanted to run. I like patch 3 as well, without it if you're
>> only running a couple of tests it can be hard to find then amongst all the
>> skipped tests. I had a quick read of the patches and they looked fine to me.
> 
> I learned the trick to run with `--run=0` just to figure out the numbers
> in a recent Outreachy project, I think it was Slavica who taught me.

That's a handy tip, thanks (though hopefully I soon wont need to use it)

Phillip

> But yes, without patch 3, the output looks awfully verbose and you have to
> scroll back an insane amount of lines for long test scripts.
> 
> Giddy,
> Dscho
> 

[PATCH v2 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Changes since v1:

 * Modified the documentation slightly to combine a separate sentence into
   the previous paragraph and avoid confusion around whether negation
   applies to substring selections (pointed out by Taylor)
 * Simplified the output for skipped tests even further, as suggested by
   Taylor.

Elijah Newren (3):
  test-lib: allow selecting tests by substring/regex with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 29 ++++++++++++++++---------
 t/t0000-basic.sh             | 41 ++++++++++++++++++++++--------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 14 ++++++------
 5 files changed, 53 insertions(+), 35 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v2
Pull-Request: https://github.com/git/git/pull/878

Range-diff vs v1:

 1:  46fce3a844 ! 1:  41e2528e83 test-lib: allow selecting tests by substring/regex with --run
     @@ t/README: For an individual test suite --run could be used to specify that
      -been included.  You may omit the first or the second number to
      -mean "from the first test" or "up to the very last test"
      -respectively.
     +-
     +-Optional prefix of '!' means that the test or a range of tests
     +-should be excluded from the run.
      +The argument for --run, <test-selector>, is a list of description
      +substrings or regexes or individual test numbers or ranges with an
     -+optional negation prefix that define what tests in a test suite to
     -+include in the run.  A range is two numbers separated with a dash and
     -+matches a range of tests with both ends been included.  You may omit
     -+the first or the second number to mean "from the first test" or "up to
     -+the very last test" respectively.
     ++optional negation prefix (of '!') that define what tests in a test
     ++suite to include (or exclude, if negated) in the run.  A range is two
     ++numbers separated with a dash and matches a range of tests with both
     ++ends been included.  You may omit the first or the second number to
     ++mean "from the first test" or "up to the very last test" respectively.
       
     - Optional prefix of '!' means that the test or a range of tests
     - should be excluded from the run.
     + If --run starts with an unprefixed number or range the initial
     + set of tests to run is empty. If the first item starts with '!'
      @@ t/README: test in the test suite except from 7 up to 11:
       
           $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 2:  d1d73400e8 = 2:  8f1a4420f4 t6006, t6012: adjust tests to use 'setup' instead of synonyms
 3:  bb2317972a ! 3:  85a4ca164a test-lib: reduce verbosity of skipped tests
     @@ t/test-lib.sh: test_skip () {
       		fi
       
      -		say_color skip >&3 "skipping test: $@"
     -+		say_color skip >&3 "skipping test: $1"
       		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
       		: true
       		;;

-- 
gitgitgadget

[PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 29 +++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 13 +++++++------
 3 files changed, 51 insertions(+), 32 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..5fd8eaf595 100644
--- a/t/README
+++ b/t/README
@@ -258,16 +258,13 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
-
-Optional prefix of '!' means that the test or a range of tests
-should be excluded from the run.
+The argument for --run, <test-selector>, is a list of description
+substrings or regexes or individual test numbers or ranges with an
+optional negation prefix (of '!') that define what tests in a test
+suite to include (or exclude, if negated) in the run.  A range is two
+numbers separated with a dash and matches a range of tests with both
+ends been included.  You may omit the first or the second number to
+mean "from the first test" or "up to the very last test" respectively.
 
 If --run starts with an unprefixed number or range the initial
 set of tests to run is empty. If the first item starts with '!'
@@ -317,6 +314,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/regex to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..07105b2078 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..2aca398e1e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,6 +769,8 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,8 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					echo "$title" | grep -q "$selector" && return
+					continue
 				fi
 		esac
 
@@ -1031,7 +1032,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote:

> Many of our test scripts have several "setup" tests.  It's a lot easier
> to say
> 
>    ./t0050-filesystem.sh --run=setup,9

I like this direction very well.

There was a small discussion recently that we might be better off
dropping test script numbers entirely, and allowing selection of script
names by word-hierarchy or regex, too. That's mostly orthogonal to what
you're doing here, but I think this is taking us in a good direction
there.

> @@ -819,9 +821,8 @@ match_test_selector_list () {
>  			*)
>  				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
>  				then
> -					echo "error: $title: invalid non-numeric in test" \
> -						"selector: '$orig_selector'" >&2
> -					exit 1
> +					echo "$title" | grep -q "$selector" && return
> +					continue
>  				fi

I like that you allow regexes. It's unfortunate that the skip-check
costs us a process in every test. It may not be that big a deal since we
only pay it if you use a non-numeric selector. But I wonder if there's
any reason not to use "expr" here, as well.

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

> > @@ -819,9 +821,8 @@ match_test_selector_list () {
> >                       *)
> >                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> >                               then
> > -                                     echo "error: $title: invalid non-numeric in test" \
> > -                                             "selector: '$orig_selector'" >&2
> > -                                     exit 1
> > +                                     echo "$title" | grep -q "$selector" && return
> > +                                     continue
> >                               fi
>
> I like that you allow regexes. It's unfortunate that the skip-check
> costs us a process in every test. It may not be that big a deal since we
> only pay it if you use a non-numeric selector. But I wonder if there's
> any reason not to use "expr" here, as well.

If you define the pattern is not regexp but is glob, you can use
case/esac to do this without any forking.

Your expr may well be built-in, though.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Wed, Oct 14, 2020 at 10:46:15AM -0700, Junio C Hamano wrote:

> > I like that you allow regexes. It's unfortunate that the skip-check
> > costs us a process in every test. It may not be that big a deal since we
> > only pay it if you use a non-numeric selector. But I wonder if there's
> > any reason not to use "expr" here, as well.
> 
> If you define the pattern is not regexp but is glob, you can use
> case/esac to do this without any forking.

Yes, that would probably be OK for most purposes, though I admit my real
love for regex support is the ability to use "." instead of space to
avoid quoting arguments. ;)

Globs may make some real patterns slightly simpler, though. I imagine
that the "setup" example may need to be "set.?up" or "set.*up" in
practice. Which is only "set*up" as a glob (I also don't have a problem
standardizing on one spelling as people find cases).

> Your expr may well be built-in, though.

Yeah, that was my assumption, though I didn't bother to test it. Having
done so, it looks like it's not a built-in either in dash or bash.  So
switching to it from grep may be buying less in practice than I thought.

We're also running a ton of exprs earlier in the function. Running:

  strace -f -e execve -o foo.out ./t0003-attributes.sh --run=10

appears to exec expr 65 times. There are only 103 execves total in the
whole run, so that's more than half of them! It might be worth seeing if
some of those could do globbing via case/esac.

Repeating without "--run" yields 39 exprs out of 492 execs. So that's
less abysmal. Most of those are from test_oid_cache.

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

> > If you define the pattern is not regexp but is glob, you can use
> > case/esac to do this without any forking.
>
> Yes, that would probably be OK for most purposes, though I admit my real
> love for regex support is the ability to use "." instead of space to
> avoid quoting arguments. ;)

I use "?" for the same purpose for globs. For things that are casual,
I find that
it tends to make the end-user (read: my) experience simpler to use globs than
to use regexp, largely for your ".*" vs "*" reasons.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Wed, Oct 14, 2020 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> > > If you define the pattern is not regexp but is glob, you can use
> > > case/esac to do this without any forking.
> >
> > Yes, that would probably be OK for most purposes, though I admit my real
> > love for regex support is the ability to use "." instead of space to
> > avoid quoting arguments. ;)
>
> I use "?" for the same purpose for globs. For things that are casual,
> I find that
> it tends to make the end-user (read: my) experience simpler to use globs than
> to use regexp, largely for your ".*" vs "*" reasons.

Oh, I thought you were arguing for globs over regexes here just due to
performance reasons.  Most of my uses of this feature so far are just
substring matches, so either globs or regexes would be fine for what
I've done so far.  Mostly, I was leaning towards regexes because I
figured they were a bit more flexible and perhaps there might arise a
usecase where that would be handy (plus Peff seemed to like them).
But if you find the normal usecase easier with globs, maybe I should
let you and Peff argue it out and then I'll resubmit with the
preferred solution?

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Wed, Oct 14, 2020 at 12:41:00PM -0700, Elijah Newren wrote:

> > I use "?" for the same purpose for globs. For things that are casual,
> > I find that
> > it tends to make the end-user (read: my) experience simpler to use globs than
> > to use regexp, largely for your ".*" vs "*" reasons.
> 
> Oh, I thought you were arguing for globs over regexes here just due to
> performance reasons.  Most of my uses of this feature so far are just
> substring matches, so either globs or regexes would be fine for what
> I've done so far.  Mostly, I was leaning towards regexes because I
> figured they were a bit more flexible and perhaps there might arise a
> usecase where that would be handy (plus Peff seemed to like them).
> But if you find the normal usecase easier with globs, maybe I should
> let you and Peff argue it out and then I'll resubmit with the
> preferred solution?

Thinking on it more, globs are probably a more natural fit here.
Especially if we do eventually allow selection of scripts by name, too
(e.g., for GIT_SKIP_TESTS), matching those filenames with a glob would
match what happens on the command-line.

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Thinking on it more, globs are probably a more natural fit here.
> Especially if we do eventually allow selection of scripts by name, too
> (e.g., for GIT_SKIP_TESTS), matching those filenames with a glob would
> match what happens on the command-line.

Ah, I didn't think of "match with filenames" but yes that is a good
complement to our "match with test titles" to be consistent with.

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Wed, Oct 14, 2020 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> > > If you define the pattern is not regexp but is glob, you can use
>> > > case/esac to do this without any forking.
>> >
>> > Yes, that would probably be OK for most purposes, though I admit my real
>> > love for regex support is the ability to use "." instead of space to
>> > avoid quoting arguments. ;)
>>
>> I use "?" for the same purpose for globs. For things that are casual,
>> I find that
>> it tends to make the end-user (read: my) experience simpler to use globs than
>> to use regexp, largely for your ".*" vs "*" reasons.
>
> Oh, I thought you were arguing for globs over regexes here just due to
> performance reasons.

Heh, but no, not me.

You and Peff were the ones who were talking about performance by
counting forks.  I more often than others come from usability's
point of view.  When we are not dealing with uncontrollable and
unbounded possiblity of end-user generated contents, I find it
easier to forego the power and flexibility of regexp and instead
settle on simpler globs---matching against the test titles is a
good example use case, I would imagine.

But if we are exposing regexp to those who run and debug test
scripts already, I am perfectly fine with using regexp with 'expr'.

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Wed, Oct 14, 2020 at 10:04 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote:
>
> > Many of our test scripts have several "setup" tests.  It's a lot easier
> > to say
> >
> >    ./t0050-filesystem.sh --run=setup,9
>
> I like this direction very well.
>
> There was a small discussion recently that we might be better off
> dropping test script numbers entirely, and allowing selection of script
> names by word-hierarchy or regex, too. That's mostly orthogonal to what
> you're doing here, but I think this is taking us in a good direction
> there.
>
> > @@ -819,9 +821,8 @@ match_test_selector_list () {
> >                       *)
> >                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> >                               then
> > -                                     echo "error: $title: invalid non-numeric in test" \
> > -                                             "selector: '$orig_selector'" >&2
> > -                                     exit 1
> > +                                     echo "$title" | grep -q "$selector" && return
> > +                                     continue
> >                               fi
>
> I like that you allow regexes. It's unfortunate that the skip-check
> costs us a process in every test. It may not be that big a deal since we
> only pay it if you use a non-numeric selector. But I wonder if there's
> any reason not to use "expr" here, as well.

I originally used [[ $title =~ "$selector" ]] in order to avoid the
sub-shell...but that was bash-specific.  I briefly looked to see if
there was a shell portable way to handle regexes, wasn't having much
luck, and decided that this is only likely to arise when people are
passing --run and thus only running a single script and they avoid all
the subprocesses that would have been invoked inside the test, so it's
still a big net win overall.  Does expr handle regexes, portably?  Or
are you suggesting dropping the regex handling and limit it to
substring matching?  In either case, does using expr save us anything
(isn't expr a shell command)?

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

> are you suggesting dropping the regex handling and limit it to
> substring matching?  In either case, does using expr save us anything
> (isn't expr a shell command)?

I had something along this line in mind, not to do a regex but a glob.

case "$title" in $selector) found=1 ;; esac

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Wed, Oct 14, 2020 at 10:57 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> > are you suggesting dropping the regex handling and limit it to
> > substring matching?  In either case, does using expr save us anything
> > (isn't expr a shell command)?
>
> I had something along this line in mind, not to do a regex but a glob.
>
> case "$title" in $selector) found=1 ;; esac

Interesting.  Since it needs to handle substring searching (e.g.
./test-script.sh --run=setup,rename), I think this would need to be
tweaked to be
   case "$title" in *${selector}*) include=$positive ;; esac

That'd probably be good enough for most cases, but I'm still inclined
to just pay the subprocess cost in order to allow regexes.  If someone
is specifying --run, they are going to be skipping many of the tests
(and thus a whole code block with dozens or even hundreds of execs per
test skipped), more than making up for the one extra exec per test.
And if someone doesn't specify --run, there is no extra cost.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Wed, Oct 14, 2020 at 10:57 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> > are you suggesting dropping the regex handling and limit it to
>> > substring matching?  In either case, does using expr save us anything
>> > (isn't expr a shell command)?
>>
>> I had something along this line in mind, not to do a regex but a glob.
>>
>> case "$title" in $selector) found=1 ;; esac
>
> Interesting.  Since it needs to handle substring searching (e.g.
> ./test-script.sh --run=setup,rename), I think this would need to be
> tweaked to be
>    case "$title" in *${selector}*) include=$positive ;; esac

Yup, of course we need to un-anchor with surrounding asterisks.
Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Wed, Oct 14, 2020 at 10:50:03AM -0700, Elijah Newren wrote:

> > > @@ -819,9 +821,8 @@ match_test_selector_list () {
> > >                       *)
> > >                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> > >                               then
> > > -                                     echo "error: $title: invalid non-numeric in test" \
> > > -                                             "selector: '$orig_selector'" >&2
> > > -                                     exit 1
> > > +                                     echo "$title" | grep -q "$selector" && return
> > > +                                     continue
> > >                               fi
> >
> > I like that you allow regexes. It's unfortunate that the skip-check
> > costs us a process in every test. It may not be that big a deal since we
> > only pay it if you use a non-numeric selector. But I wonder if there's
> > any reason not to use "expr" here, as well.
> 
> I originally used [[ $title =~ "$selector" ]] in order to avoid the
> sub-shell...but that was bash-specific.  I briefly looked to see if
> there was a shell portable way to handle regexes, wasn't having much
> luck, and decided that this is only likely to arise when people are
> passing --run and thus only running a single script and they avoid all
> the subprocesses that would have been invoked inside the test, so it's
> still a big net win overall.  Does expr handle regexes, portably?  Or
> are you suggesting dropping the regex handling and limit it to
> substring matching?  In either case, does using expr save us anything
> (isn't expr a shell command)?

There's an expr command doing a regex match in the diff context quoted
above. :)

I was wrong that it would save us a process, though. I thought it was a
builtin in modern shells, but it doesn't appear to be in either dash or
bash. So there's little reason to prefer it over grep here (switching to
globs could be done with case and would save a process).

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren]

On Wed, Oct 14, 2020 at 11:09 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Oct 14, 2020 at 10:50:03AM -0700, Elijah Newren wrote:
>
> > > > @@ -819,9 +821,8 @@ match_test_selector_list () {
> > > >                       *)
> > > >                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> > > >                               then
> > > > -                                     echo "error: $title: invalid non-numeric in test" \
> > > > -                                             "selector: '$orig_selector'" >&2
> > > > -                                     exit 1
> > > > +                                     echo "$title" | grep -q "$selector" && return
> > > > +                                     continue
> > > >                               fi
> > >
> > > I like that you allow regexes. It's unfortunate that the skip-check
> > > costs us a process in every test. It may not be that big a deal since we
> > > only pay it if you use a non-numeric selector. But I wonder if there's
> > > any reason not to use "expr" here, as well.
> >
> > I originally used [[ $title =~ "$selector" ]] in order to avoid the
> > sub-shell...but that was bash-specific.  I briefly looked to see if
> > there was a shell portable way to handle regexes, wasn't having much
> > luck, and decided that this is only likely to arise when people are
> > passing --run and thus only running a single script and they avoid all
> > the subprocesses that would have been invoked inside the test, so it's
> > still a big net win overall.  Does expr handle regexes, portably?  Or
> > are you suggesting dropping the regex handling and limit it to
> > substring matching?  In either case, does using expr save us anything
> > (isn't expr a shell command)?
>
> There's an expr command doing a regex match in the diff context quoted
> above. :)

Oh, indeed.  How'd I miss that?

> I was wrong that it would save us a process, though. I thought it was a
> builtin in modern shells, but it doesn't appear to be in either dash or
> bash. So there's little reason to prefer it over grep here (switching to
> globs could be done with case and would save a process).

I guess it's at least slightly more consistent with the surrounding code.

Oh, and I found a bug in that my code was not handling negated
substrings correctly, e.g.
   ./test-script.sh --run="!rename"
would run all the tests instead of excluding the ones that had
"rename" as a substring.  So I'll send a reroll fixing that.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote:
>
>> Many of our test scripts have several "setup" tests.  It's a lot easier
>> to say
>> 
>>    ./t0050-filesystem.sh --run=setup,9
>
> I like this direction very well.
>
> There was a small discussion recently that we might be better off
> dropping test script numbers entirely,...

I think I missed that one.  A pointer, if you have one handy?

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Thu, Oct 15, 2020 at 09:20:13AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote:
> >
> >> Many of our test scripts have several "setup" tests.  It's a lot easier
> >> to say
> >> 
> >>    ./t0050-filesystem.sh --run=setup,9
> >
> > I like this direction very well.
> >
> > There was a small discussion recently that we might be better off
> > dropping test script numbers entirely,...
> 
> I think I missed that one.  A pointer, if you have one handy?

The sub-thread between me and Jonathan starting here:

  https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/

but specifically:

  https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Oct 15, 2020 at 09:20:13AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote:
>> >
>> >> Many of our test scripts have several "setup" tests.  It's a lot easier
>> >> to say
>> >> 
>> >>    ./t0050-filesystem.sh --run=setup,9
>> >
>> > I like this direction very well.
>> >
>> > There was a small discussion recently that we might be better off
>> > dropping test script numbers entirely,...
>> 
>> I think I missed that one.  A pointer, if you have one handy?
>
> The sub-thread between me and Jonathan starting here:
>
>   https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/
>
> but specifically:
>
>   https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/

Ah, I see.  I actually do use "git ls-files t/ | grep ..." to look
for tests that are relevant to the issue I have at hand quite often,
so unlike what Jonathan said in the thread, having a good name does
matter to me.

As far as I can tell, the numbers in the test names serve only two
purposes.

One is as a unique key to avoid colliding in the test result
aggrevation database, and the other is as a unique key to use in
GIT_SKIP_TESTS (which in turn is used by the Meta/Make wrapper I
use, found on the todo branch, like 'Meta/Make --test=0050,1400
test').

I would be heavily inconvenienced if we decide to remove numbers
becuase it would rob the latter use case from me, but I'd survive if
we just are going to lift the requirement that numbers must be
unique.  I may end up running irrelevant 0050 and 1400 when the
tests I really want to run are the other 0050 and 1400 with
"--test=0050,1400", but when I am trying to run only 2 among 900+
scripts, running 2 extra ones I didn't have to run only because
their prefix collide is still much better and tolerable.

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Thu, Oct 15, 2020 at 01:08:22PM -0700, Junio C Hamano wrote:

> > The sub-thread between me and Jonathan starting here:
> >
> >   https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/
> >
> > but specifically:
> >
> >   https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/
> 
> Ah, I see.  I actually do use "git ls-files t/ | grep ..." to look
> for tests that are relevant to the issue I have at hand quite often,
> so unlike what Jonathan said in the thread, having a good name does
> matter to me.

I'm not sure where he suggests worse names. I'd think if anything we'd
have better names, because they'd be even more meaningful (if people
start using them for test selectors). FWIW, I also grep like that when
looking for scripts.

> As far as I can tell, the numbers in the test names serve only two
> purposes.
> 
> One is as a unique key to avoid colliding in the test result
> aggrevation database, and the other is as a unique key to use in
> GIT_SKIP_TESTS (which in turn is used by the Meta/Make wrapper I
> use, found on the todo branch, like 'Meta/Make --test=0050,1400
> test').

Yeah, that sounds right. We also frequently talk about "t5601" in
human-readable messages. I often use that to either tab-complete or run
"./t5601-*". So they do serve as short unique identifiers there. You
could tell me "t-foo-bar-baz" and that would similarly be unique, but
it's not as short. :)

They do serve as unique keys in places like test-results/, but really
the whole test name is the key. Most of them have unique text
descriptions. If I do:

  git ls-files t/ |
  perl -lne '/t[0-9]{4}-(.*).sh$/ and print "$1"' |
  sort | uniq -d

that only yields two duplicates (and we'd probably be better off giving
them unique names anyway).

> I would be heavily inconvenienced if we decide to remove numbers
> becuase it would rob the latter use case from me, but I'd survive if
> we just are going to lift the requirement that numbers must be
> unique.

My plan is that you'd be able to say:

  Meta/make --test=filesystem,update-ref

for t0050 and t1400 respectively. Or more interesting things like
"git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split
them by directories).

> I may end up running irrelevant 0050 and 1400 when the
> tests I really want to run are the other 0050 and 1400 with
> "--test=0050,1400", but when I am trying to run only 2 among 900+
> scripts, running 2 extra ones I didn't have to run only because
> their prefix collide is still much better and tolerable.

I think you could already do this in general. The only thing standing in
the way is test-lint-duplicates. I added that because I thought it would
be better to catch the duplicates early rather than get confused by them
later when you can't uniquely identify a test.  But in practice I do
feel like I spend more time de-duping tests when I rebase or merge
(because my branch raced with somebody else allocating the test number)
than I ever did dealing with unexpectedly skipped tests.

I definitely don't want to make anybody's life harder. But if the
numbers aren't unique and the text connected to them is, then it seems
like we should just use the text primarily.

-Peff

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Ah, I see.  I actually do use "git ls-files t/ | grep ..." to look
>> for tests that are relevant to the issue I have at hand quite often,
>> so unlike what Jonathan said in the thread, having a good name does
>> matter to me.
>
> I'm not sure where he suggests worse names. I'd think if anything we'd
> have better names, because they'd be even more meaningful (if people
> start using them for test selectors). FWIW, I also grep like that when
> looking for scripts.

I didn't mean Jonathan suggested worse names.  Unlike "I don't tend
to discover test scripts based on their filename", which was what
Jonathan said, I do look for tests based on their filename, so
having a good name matters (on the other hand, if you are the kind
of person who does not look for them by name, the naming may not
matter to you).


> My plan is that you'd be able to say:
>
>   Meta/make --test=filesystem,update-ref
>
> for t0050 and t1400 respectively. Or more interesting things like
> "git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split
> them by directories).

Most of the time, I am focused on running only the selected few, not
excluding a few known-broken ones.

For example, with an round of integration, I know only two tests
fail, and after I tried to fix the bug in the series, I want to
retry these two tests and nothing else to see if the attempted fix
patches the breakage up (of course I need to run the remainder of
the tests to ensure there is no regression, but that is just the
matter of running "make test" to run the whole thing again).

As long as it is known that "filesystem" and "update-ref" can serve
as tokens to uniquely identify these two tests, it would be fine for
my purpose.  But 0050 (under the rule that numbering must be unique)
would give me such an assurance much better without having to look
at any other test file.

The word "filesystem"?  Unless we have a rule that we can use each
unique word in test names only once (which of course is impractical)
I am not sure I can use it in place of 0050 without checking names
of other tests first.

> I definitely don't want to make anybody's life harder. But if the
> numbers aren't unique and the text connected to them is, then it seems
> like we should just use the text primarily.

True in principle, but it is harder to come up with unique substring
of text, knowing only that the whole string is unique.

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> My plan is that you'd be able to say:
>>
>>   Meta/make --test=filesystem,update-ref
>>
>> for t0050 and t1400 respectively. Or more interesting things like
>> "git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split
>> them by directories).
> ...
> The word "filesystem"?  Unless we have a rule that we can use each
> unique word in test names only once (which of course is impractical)
> I am not sure I can use it in place of 0050 without checking names
> of other tests first.
>
>> I definitely don't want to make anybody's life harder. But if the
>> numbers aren't unique and the text connected to them is, then it seems
>> like we should just use the text primarily.
>
> True in principle, but it is harder to come up with unique substring
> of text, knowing only that the whole string is unique.

I think there was a mismatch in assumptions and expectations.  I
think your "plan" was to use the whole name (what comes after \d+-
in the current naming scheme, except for the .sh at the end), and I
was assuming a glob with implicit * on both ends would be used, i.e.

    $ ls -d t/*update-ref*

would give me 1404 (update-ref-errors) in addition to 1400 (update-ref).

So if the rule is to always spell out the full name if I wanted
uniqueness, then it would work.  In the cited usecase of knowing
which two failed the first time, I should have the full filename
of them available.

Thanks.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Jeff King]

On Fri, Oct 16, 2020 at 09:16:57AM -0700, Junio C Hamano wrote:

> > I'm not sure where he suggests worse names. I'd think if anything we'd
> > have better names, because they'd be even more meaningful (if people
> > start using them for test selectors). FWIW, I also grep like that when
> > looking for scripts.
> 
> I didn't mean Jonathan suggested worse names.  Unlike "I don't tend
> to discover test scripts based on their filename", which was what
> Jonathan said, I do look for tests based on their filename, so
> having a good name matters (on the other hand, if you are the kind
> of person who does not look for them by name, the naming may not
> matter to you).

Ah, I misunderstood. Thanks for clarifying (and I am very much in your
camp that the names are useful).

> As long as it is known that "filesystem" and "update-ref" can serve
> as tokens to uniquely identify these two tests, it would be fine for
> my purpose.  But 0050 (under the rule that numbering must be unique)
> would give me such an assurance much better without having to look
> at any other test file.
> 
> The word "filesystem"?  Unless we have a rule that we can use each
> unique word in test names only once (which of course is impractical)
> I am not sure I can use it in place of 0050 without checking names
> of other tests first.

With your follow-up response:

> So if the rule is to always spell out the full name if I wanted
> uniqueness, then it would work.

I think we are on the same page, and my intent was to match full names.

So now you get "t0050" from some failed-test output (prove, or just the
output from make failing), and you copy it into the command-line to use
with "--test". And instead you'd just copy the full text name. It's a
little less convenient because t1234 is short enough that I'd type it,
and I'd probably cut-and-paste the text name. But other than that, I'd
expect the procedure to be the same.

The substring matches added by Elijah's series make sense for individual
test snippets within a script, I think. And I think we could even add
script-name matching now[1], without getting rid of the numbers. But if
we do so, we should be careful to introduce it as an anchored match and
not a substring match, to avoid having to switch it later.

-Peff

[1] And by "now" I don't mean we should hold up Elijah's patches for this
    feature, but that anybody who wishes to build it on top is free to
    do so without us having to make a decision on ditching the numbers
    entirely.

Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think we are on the same page, and my intent was to match full names.
>
> So now you get "t0050" from some failed-test output (prove, or just the
> output from make failing), and you copy it into the command-line to use
> with "--test". And instead you'd just copy the full text name. It's a
> little less convenient because t1234 is short enough that I'd type it,
> and I'd probably cut-and-paste the text name. But other than that, I'd
> expect the procedure to be the same.

Most often, they are no longer in my scroll buffer, though.  Two
4-digit integers I can easily hold in my head and type.  Full names
of two test scripts?  That's a bit taxing.

So, not really.

[PATCH v2 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH v2 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
adds up to hundreds or thousands of lines of irrelevant output that make
it very hard to fish out the relevant results you were looking for.
Simplify the output for skipped tests down to just showing the one-line
descriptions.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2aca398e1e..fe81d9c733 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1059,7 +1059,6 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget

Re: [PATCH v2 3/3] test-lib: reduce verbosity of skipped tests

[Jeff King]

On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
> 
> When using the --run flag to run just two or three tests from a test
> file which contains several dozen tests, having every skipped test print
> out dozens of lines of output for the test code for that skipped test
> adds up to hundreds or thousands of lines of irrelevant output that make
> it very hard to fish out the relevant results you were looking for.
> Simplify the output for skipped tests down to just showing the one-line
> descriptions.

This last sentence is inaccurate in this version, isn't it?

Other than that, I think this is a good change (I admit I never noticed
the irrelevant output because it is only shown with "-v", and that is
already full of irrelevant bits. But I have trouble imagining how it
would be useful).

Earlier discussion mentioned shortening the "ok 2 # skip" line, but I
think removing this fd-3 version is the only sane direction. The other
one gets parsed by TAP tools like prove, and may be shown. E.g., "prove
--directive t0003-attributes.sh" shows the description of the skipped
tests (likewise, "-v" shows all tests but highlights the skipped ones).

-Peff

Re: [PATCH v2 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren]

On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > When using the --run flag to run just two or three tests from a test
> > file which contains several dozen tests, having every skipped test print
> > out dozens of lines of output for the test code for that skipped test
> > adds up to hundreds or thousands of lines of irrelevant output that make
> > it very hard to fish out the relevant results you were looking for.
> > Simplify the output for skipped tests down to just showing the one-line
> > descriptions.
>
> This last sentence is inaccurate in this version, isn't it?

Maybe I could make it clearer, but I think that it is accurate[1].  If
this wording seems confusing, though, I could simplify the commit
message by reducing the sentence to "Simplify the output for skipped
tests."

[1] The original code showed the one-line description (with a
"skipping test:" prefix) AND the full test code AND then repeated the
one-line description on an "ok" line.  My v1 version of this patch
ripped out the long middle portion -- the full test code -- and thus
only showed the other bits, namely showing the one-line descriptions
twice (once with the "skipping test:" prefix, and the second time with
an "ok <number> # " prefix).  This v2 version of the patch shows the
one-line description only once with the "ok <number> #" prefix.  So,
I'd say the commit message is still accurate: the code only shows the
one-line description in both v1 and v2, though there is a question of
how many times it is shown.

> Other than that, I think this is a good change (I admit I never noticed
> the irrelevant output because it is only shown with "-v", and that is
> already full of irrelevant bits. But I have trouble imagining how it
> would be useful).
>
> Earlier discussion mentioned shortening the "ok 2 # skip" line, but I
> think removing this fd-3 version is the only sane direction. The other
> one gets parsed by TAP tools like prove, and may be shown. E.g., "prove
> --directive t0003-attributes.sh" shows the description of the skipped
> tests (likewise, "-v" shows all tests but highlights the skipped ones).

Make sense, thanks.

Re: [PATCH v2 3/3] test-lib: reduce verbosity of skipped tests

[Jeff King]

On Wed, Oct 14, 2020 at 10:39:01AM -0700, Elijah Newren wrote:

> On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote:
> >
> > On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote:
> >
> > > From: Elijah Newren <newren@gmail.com>
> > >
> > > When using the --run flag to run just two or three tests from a test
> > > file which contains several dozen tests, having every skipped test print
> > > out dozens of lines of output for the test code for that skipped test
> > > adds up to hundreds or thousands of lines of irrelevant output that make
> > > it very hard to fish out the relevant results you were looking for.
> > > Simplify the output for skipped tests down to just showing the one-line
> > > descriptions.
> >
> > This last sentence is inaccurate in this version, isn't it?
> 
> Maybe I could make it clearer, but I think that it is accurate[1].  If
> this wording seems confusing, though, I could simplify the commit
> message by reducing the sentence to "Simplify the output for skipped
> tests."

Yeah, I wondered if you might have been thinking that. It makes sense in
the context of the other discussion, but the single-line TAP output is
not even mentioned here. And it might be worth doing so, because the
real reason it is OK to delete this line entirely is that it is
redundant with that line.

-Peff

Re: [PATCH v2 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren]

On Wed, Oct 14, 2020 at 10:55 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Oct 14, 2020 at 10:39:01AM -0700, Elijah Newren wrote:
>
> > On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote:
> > >
> > > On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote:
> > >
> > > > From: Elijah Newren <newren@gmail.com>
> > > >
> > > > When using the --run flag to run just two or three tests from a test
> > > > file which contains several dozen tests, having every skipped test print
> > > > out dozens of lines of output for the test code for that skipped test
> > > > adds up to hundreds or thousands of lines of irrelevant output that make
> > > > it very hard to fish out the relevant results you were looking for.
> > > > Simplify the output for skipped tests down to just showing the one-line
> > > > descriptions.
> > >
> > > This last sentence is inaccurate in this version, isn't it?
> >
> > Maybe I could make it clearer, but I think that it is accurate[1].  If
> > this wording seems confusing, though, I could simplify the commit
> > message by reducing the sentence to "Simplify the output for skipped
> > tests."
>
> Yeah, I wondered if you might have been thinking that. It makes sense in
> the context of the other discussion, but the single-line TAP output is
> not even mentioned here. And it might be worth doing so, because the
> real reason it is OK to delete this line entirely is that it is
> redundant with that line.

Makes sense; I'll update it to mention that, once the discussion on
how we want to handle regexes/globs/subshells for patch 1/3 is
resolved.

Re: [PATCH v2 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Taylor Blau]

On Tue, Oct 13, 2020 at 07:19:43PM +0000, Elijah Newren via GitGitGadget wrote:
> Changes since v1:
>
>  * Modified the documentation slightly to combine a separate sentence into
>    the previous paragraph and avoid confusion around whether negation
>    applies to substring selections (pointed out by Taylor)
>  * Simplified the output for skipped tests even further, as suggested by
>    Taylor.

Thanks, this version looks very good to me. With Junio's ack on the
changes in test-lib, I feel much better about my own suggestion. So,
this all looks good to me.

Thanks,
Taylor

[PATCH v3 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Changes since v2:

 * Use a glob rather than a regex for selecting tests
 * Touched up the commit message in patch 3 to mention that the TAP output
   line is still included for skipped tests and already included the word
   "skip" to highlight the fact that the test was skipped.
 * Fixed a bug where -run="!rename" would run all tests instead of skipping
   the ones with 'rename' in the description

Elijah Newren (3):
  test-lib: allow selecting tests by substring/glob with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 29 ++++++++++++++++---------
 t/t0000-basic.sh             | 41 ++++++++++++++++++++++--------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 17 +++++++++------
 5 files changed, 56 insertions(+), 35 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v3
Pull-Request: https://github.com/git/git/pull/878

Range-diff vs v2:

 1:  41e2528e83 ! 1:  9c8b6a1a94 test-lib: allow selecting tests by substring/regex with --run
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    test-lib: allow selecting tests by substring/regex with --run
     +    test-lib: allow selecting tests by substring/glob with --run
      
          Many of our test scripts have several "setup" tests.  It's a lot easier
          to say
     @@ t/README: For an individual test suite --run could be used to specify that
      -Optional prefix of '!' means that the test or a range of tests
      -should be excluded from the run.
      +The argument for --run, <test-selector>, is a list of description
     -+substrings or regexes or individual test numbers or ranges with an
     ++substrings or globs or individual test numbers or ranges with an
      +optional negation prefix (of '!') that define what tests in a test
      +suite to include (or exclude, if negated) in the run.  A range is two
      +numbers separated with a dash and matches a range of tests with both
     @@ t/README: test in the test suite except from 7 up to 11:
       
      +Sometimes there may be multiple tests with e.g. "setup" in their name
      +that are needed and rather than figuring out the number for all of them
     -+we can just use "setup" as a substring/regex to match against the test
     ++we can just use "setup" as a substring/glob to match against the test
      +description:
      +
      +    $ sh ./t0050-filesystem.sh --run=setup,9-11
     @@ t/test-lib.sh: match_test_selector_list () {
      -					echo "error: $title: invalid non-numeric in test" \
      -						"selector: '$orig_selector'" >&2
      -					exit 1
     -+					echo "$title" | grep -q "$selector" && return
     ++					case "$title" in *${selector}*)
     ++						include=$positive
     ++						;;
     ++					esac
      +					continue
       				fi
       		esac
 2:  8f1a4420f4 = 2:  6479d73603 t6006, t6012: adjust tests to use 'setup' instead of synonyms
 3:  85a4ca164a ! 3:  24f6febde6 test-lib: reduce verbosity of skipped tests
     @@ Commit message
          When using the --run flag to run just two or three tests from a test
          file which contains several dozen tests, having every skipped test print
          out dozens of lines of output for the test code for that skipped test
     -    adds up to hundreds or thousands of lines of irrelevant output that make
     -    it very hard to fish out the relevant results you were looking for.
     -    Simplify the output for skipped tests down to just showing the one-line
     -    descriptions.
     +    (in addition to the TAP output line) adds up to hundreds or thousands of
     +    lines of irrelevant output that make it very hard to fish out the
     +    relevant results you were looking for.  Simplify the output for skipped
     +    tests to remove this extra output, leaving only the TAP output line
     +    (i.e. the line reading "ok <number> # skip <test-description>", which
     +    already mentions that the test was "skip"ped).
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      

-- 
gitgitgadget

[PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 29 +++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 16 ++++++++++------
 3 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..0a8b60b5c7 100644
--- a/t/README
+++ b/t/README
@@ -258,16 +258,13 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
-
-Optional prefix of '!' means that the test or a range of tests
-should be excluded from the run.
+The argument for --run, <test-selector>, is a list of description
+substrings or globs or individual test numbers or ranges with an
+optional negation prefix (of '!') that define what tests in a test
+suite to include (or exclude, if negated) in the run.  A range is two
+numbers separated with a dash and matches a range of tests with both
+ends been included.  You may omit the first or the second number to
+mean "from the first test" or "up to the very last test" respectively.
 
 If --run starts with an unprefixed number or range the initial
 set of tests to run is empty. If the first item starts with '!'
@@ -317,6 +314,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/glob to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..07105b2078 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..a040d54a76 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,6 +769,8 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,11 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					case "$title" in *${selector}*)
+						include=$positive
+						;;
+					esac
+					continue
 				fi
 		esac
 
@@ -1031,7 +1035,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Phillip Wood]

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.

Best Wishes

Phillip

> Add such an ability to test selection which relies on merely matching
> against the test description.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   t/README         | 29 +++++++++++++++++++----------
>   t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
>   t/test-lib.sh    | 16 ++++++++++------
>   3 files changed, 54 insertions(+), 32 deletions(-)
> 
> diff --git a/t/README b/t/README
> index 2adaf7c2d2..0a8b60b5c7 100644
> --- a/t/README
> +++ b/t/README
> @@ -258,16 +258,13 @@ For an individual test suite --run could be used to specify that
>   only some tests should be run or that some tests should be
>   excluded from a run.
>   
> -The argument for --run is a list of individual test numbers or
> -ranges with an optional negation prefix that define what tests in
> -a test suite to include in the run.  A range is two numbers
> -separated with a dash and matches a range of tests with both ends
> -been included.  You may omit the first or the second number to
> -mean "from the first test" or "up to the very last test"
> -respectively.
> -
> -Optional prefix of '!' means that the test or a range of tests
> -should be excluded from the run.
> +The argument for --run, <test-selector>, is a list of description
> +substrings or globs or individual test numbers or ranges with an
> +optional negation prefix (of '!') that define what tests in a test
> +suite to include (or exclude, if negated) in the run.  A range is two
> +numbers separated with a dash and matches a range of tests with both
> +ends been included.  You may omit the first or the second number to
> +mean "from the first test" or "up to the very last test" respectively.
>   
>   If --run starts with an unprefixed number or range the initial
>   set of tests to run is empty. If the first item starts with '!'
> @@ -317,6 +314,18 @@ test in the test suite except from 7 up to 11:
>   
>       $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
>   
> +Sometimes there may be multiple tests with e.g. "setup" in their name
> +that are needed and rather than figuring out the number for all of them
> +we can just use "setup" as a substring/glob to match against the test
> +description:
> +
> +    $ sh ./t0050-filesystem.sh --run=setup,9-11
> +
> +or one could select both the setup tests and the rename ones (assuming all
> +relevant tests had those words in their descriptions):
> +
> +    $ sh ./t0050-filesystem.sh --run=setup,rename
> +
>   Some tests in a test suite rely on the previous tests performing
>   certain actions, specifically some tests are designated as
>   "setup" test, so you cannot _arbitrarily_ disable one test and
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 923281af93..07105b2078 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
>   	EOF
>   "
>   
> -test_expect_success '--run invalid range start' "
> +test_expect_success '--run substring selector' "
> +	run_sub_test_lib_test run-substring-selector \
> +		'--run empty selectors' \
> +		--run='relevant' <<-\\EOF &&
> +	test_expect_success \"relevant test\" 'true'
> +	for i in 1 2 3 4 5 6
> +	do
> +		test_expect_success \"other test #\$i\" 'true'
> +	done
> +	test_done
> +	EOF
> +	check_sub_test_lib_test run-substring-selector <<-\\EOF
> +	> ok 1 - relevant test
> +	> ok 2 # skip other test #1 (--run)
> +	> ok 3 # skip other test #2 (--run)
> +	> ok 4 # skip other test #3 (--run)
> +	> ok 5 # skip other test #4 (--run)
> +	> ok 6 # skip other test #5 (--run)
> +	> ok 7 # skip other test #6 (--run)
> +	> # passed all 7 test(s)
> +	> 1..7
> +	EOF
> +"
> +
> +test_expect_success '--run keyword selection' "
>   	run_sub_test_lib_test_err run-inv-range-start \
>   		'--run invalid range start' \
>   		--run='a-5' <<-\\EOF &&
> @@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
>   	EOF_ERR
>   "
>   
> -test_expect_success '--run invalid selector' "
> -	run_sub_test_lib_test_err run-inv-selector \
> -		'--run invalid selector' \
> -		--run='1?' <<-\\EOF &&
> -	test_expect_success \"passing test #1\" 'true'
> -	test_done
> -	EOF
> -	check_sub_test_lib_test_err run-inv-selector \
> -		<<-\\EOF_OUT 3<<-\\EOF_ERR
> -	> FATAL: Unexpected exit with code 1
> -	EOF_OUT
> -	> error: --run: invalid non-numeric in test selector: '1?'
> -	EOF_ERR
> -"
> -
>   
>   test_set_prereq HAVEIT
>   haveit=no
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ef31f40037..a040d54a76 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -769,6 +769,8 @@ match_pattern_list () {
>   }
>   
>   match_test_selector_list () {
> +	operation="$1"
> +	shift
>   	title="$1"
>   	shift
>   	arg="$1"
> @@ -805,13 +807,13 @@ match_test_selector_list () {
>   			*-*)
>   				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
>   				then
> -					echo "error: $title: invalid non-numeric in range" \
> +					echo "error: $operation: invalid non-numeric in range" \
>   						"start: '$orig_selector'" >&2
>   					exit 1
>   				fi
>   				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
>   				then
> -					echo "error: $title: invalid non-numeric in range" \
> +					echo "error: $operation: invalid non-numeric in range" \
>   						"end: '$orig_selector'" >&2
>   					exit 1
>   				fi
> @@ -819,9 +821,11 @@ match_test_selector_list () {
>   			*)
>   				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
>   				then
> -					echo "error: $title: invalid non-numeric in test" \
> -						"selector: '$orig_selector'" >&2
> -					exit 1
> +					case "$title" in *${selector}*)
> +						include=$positive
> +						;;
> +					esac
> +					continue
>   				fi
>   		esac
>   
> @@ -1031,7 +1035,7 @@ test_skip () {
>   		skipped_reason="GIT_SKIP_TESTS"
>   	fi
>   	if test -z "$to_skip" && test -n "$run_list" &&
> -	   ! match_test_selector_list '--run' $test_count "$run_list"
> +	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
>   	then
>   		to_skip=t
>   		skipped_reason="--run"
> 

Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren]

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, I'm worried we're breaking long-built expectations of
other developers by changing IFS here.  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.

Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Phillip Wood]

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

Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> Given that t/README explicitly shows examples of space-separated lists
> of numbers, I'm worried we're breaking long-built expectations of
> other developers by changing IFS here.  Perhaps I could instead add
> the following paragraph to t/README:

Unlike something that would affect end-users, I think it is OK to
break backward compatibility one-way.  If you suddenly forbid spaces
and force our developers to use comma and nothing else, in muscle
memory of their fingers and/or in their scripts, in a version merged
to 'master', as long as their new habit and updated scripts use
comma consistently, they work fine on 'maint', right?

If there is no such "works on both sides of the flag day" choice, it
is a different story, of course, but comma should work fine for us
in this case.

Thanks.

Re: [PATCH v3 1/3] test-lib: allow selecting tests by substring/glob with --run

[Jeff King]

On Fri, Oct 16, 2020 at 01:02:16PM -0700, Junio C Hamano wrote:

> Elijah Newren <newren@gmail.com> writes:
> 
> > Given that t/README explicitly shows examples of space-separated lists
> > of numbers, I'm worried we're breaking long-built expectations of
> > other developers by changing IFS here.  Perhaps I could instead add
> > the following paragraph to t/README:
> 
> Unlike something that would affect end-users, I think it is OK to
> break backward compatibility one-way.  If you suddenly forbid spaces
> and force our developers to use comma and nothing else, in muscle
> memory of their fingers and/or in their scripts, in a version merged
> to 'master', as long as their new habit and updated scripts use
> comma consistently, they work fine on 'maint', right?
> 
> If there is no such "works on both sides of the flag day" choice, it
> is a different story, of course, but comma should work fine for us
> in this case.

I was just about to write the same argument. :)

-Peff

[PATCH v3 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH v3 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
(in addition to the TAP output line) adds up to hundreds or thousands of
lines of irrelevant output that make it very hard to fish out the
relevant results you were looking for.  Simplify the output for skipped
tests to remove this extra output, leaving only the TAP output line
(i.e. the line reading "ok <number> # skip <test-description>", which
already mentions that the test was "skip"ped).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a040d54a76..3f8c404335 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1062,7 +1062,6 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget

Re: [PATCH v3 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Jeff King]

On Wed, Oct 14, 2020 at 09:13:42PM +0000, Elijah Newren via GitGitGadget wrote:

> Changes since v2:
> 
>  * Use a glob rather than a regex for selecting tests
>  * Touched up the commit message in patch 3 to mention that the TAP output
>    line is still included for skipped tests and already included the word
>    "skip" to highlight the fact that the test was skipped.
>  * Fixed a bug where -run="!rename" would run all tests instead of skipping
>    the ones with 'rename' in the description

Thanks, this one addresses all my nits. I'm looking forward to using it
in practice. :)

-Peff

[PATCH v4 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Changes since v3:

 * Added text to the README explaining what to do if the substring you want
   to search on includes a comma or whitespace (namely, use the '?' glob
   character instead)

Elijah Newren (3):
  test-lib: allow selecting tests by substring/glob with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 37 +++++++++++++++++++++++---------
 t/t0000-basic.sh             | 41 ++++++++++++++++++++++--------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 17 +++++++++------
 5 files changed, 64 insertions(+), 35 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v4
Pull-Request: https://github.com/git/git/pull/878

Range-diff vs v3:

 1:  9c8b6a1a94 ! 1:  8f5d4ad318 test-lib: allow selecting tests by substring/glob with --run
     @@ t/README: For an individual test suite --run could be used to specify that
      +numbers separated with a dash and matches a range of tests with both
      +ends been included.  You may omit the first or the second number to
      +mean "from the first test" or "up to the very last test" respectively.
     ++
     ++The argument to --run is split on commas and whitespace into separate
     ++strings, numbers, and ranges, and picks all tests that match any of
     ++the individual selection criteria.  If the substring of the
     ++description text that you want to match 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*.
       
       If --run starts with an unprefixed number or range the initial
       set of tests to run is empty. If the first item starts with '!'
 2:  6479d73603 = 2:  bbae029cc8 t6006, t6012: adjust tests to use 'setup' instead of synonyms
 3:  24f6febde6 = 3:  0e1a1125f7 test-lib: reduce verbosity of skipped tests

-- 
gitgitgadget

[PATCH v4 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 37 +++++++++++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 16 ++++++++++------
 3 files changed, 62 insertions(+), 32 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..4594f3ab7b 100644
--- a/t/README
+++ b/t/README
@@ -258,16 +258,21 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
-
-Optional prefix of '!' means that the test or a range of tests
-should be excluded from the run.
+The argument for --run, <test-selector>, is a list of description
+substrings or globs or individual test numbers or ranges with an
+optional negation prefix (of '!') that define what tests in a test
+suite to include (or exclude, if negated) in the run.  A range is two
+numbers separated with a dash and matches a range of tests with both
+ends been included.  You may omit the first or the second number to
+mean "from the first test" or "up to the very last test" respectively.
+
+The argument to --run is split on commas and whitespace into separate
+strings, numbers, and ranges, and picks all tests that match any of
+the individual selection criteria.  If the substring of the
+description text that you want to match 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*.
 
 If --run starts with an unprefixed number or range the initial
 set of tests to run is empty. If the first item starts with '!'
@@ -317,6 +322,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/glob to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..07105b2078 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..a040d54a76 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,6 +769,8 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,11 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					case "$title" in *${selector}*)
+						include=$positive
+						;;
+					esac
+					continue
 				fi
 		esac
 
@@ -1031,7 +1035,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

[PATCH v4 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH v4 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
(in addition to the TAP output line) adds up to hundreds or thousands of
lines of irrelevant output that make it very hard to fish out the
relevant results you were looking for.  Simplify the output for skipped
tests to remove this extra output, leaving only the TAP output line
(i.e. the line reading "ok <number> # skip <test-description>", which
already mentions that the test was "skip"ped).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a040d54a76..3f8c404335 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1062,7 +1062,6 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget

Re: [PATCH v4 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Junio C Hamano]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Changes since v3:
>
>  * Added text to the README explaining what to do if the substring you want
>    to search on includes a comma or whitespace (namely, use the '?' glob
>    character instead)

That would work just as well with or without forbidding spaces.

>      ++The argument to --run is split on commas and whitespace into separate
>      ++strings, numbers, and ranges, and picks all tests that match any of
>      ++the individual selection criteria.  If the substring of the
>      ++description text that you want to match 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*.

OK.

Will queue.

Re: [PATCH v4 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren]

On Fri, Oct 16, 2020 at 1:25 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Changes since v3:
> >
> >  * Added text to the README explaining what to do if the substring you want
> >    to search on includes a comma or whitespace (namely, use the '?' glob
> >    character instead)
>
> That would work just as well with or without forbidding spaces.
>
> >      ++The argument to --run is split on commas and whitespace into separate
> >      ++strings, numbers, and ranges, and picks all tests that match any of
> >      ++the individual selection criteria.  If the substring of the
> >      ++description text that you want to match 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*.
>
> OK.
>
> Will queue.

Since both you and Peff argued for just changing IFS, I'll do that,
and update this text and elsewhere in t/README to reflect that we only
split on commas and send that in as v4.

[PATCH v5 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Changes since v4:

 * Changed to only split tests on commas

Elijah Newren (3):
  test-lib: allow selecting tests by substring/glob with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 48 ++++++++++++++++++++------------
 t/t0000-basic.sh             | 53 +++++++++++++++++++++---------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 19 +++++++------
 5 files changed, 75 insertions(+), 49 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v5
Pull-Request: https://github.com/git/git/pull/878

Range-diff vs v4:

 1:  8f5d4ad318 ! 1:  389c2c171a test-lib: allow selecting tests by substring/glob with --run
     @@ t/README: For an individual test suite --run could be used to specify that
      +ends been included.  You may omit the first or the second number to
      +mean "from the first test" or "up to the very last test" respectively.
      +
     -+The argument to --run is split on commas and whitespace into separate
     -+strings, numbers, and ranges, and picks all tests that match any of
     -+the individual selection criteria.  If the substring of the
     -+description text that you want to match 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*.
     ++The argument to --run is split on commas into separate strings,
     ++numbers, and ranges, and picks all tests that match any of the
     ++individual selection criteria.  If the substring of the description
     ++text that you want to match includes a comma, use the glob character
     ++'?' instead.  For example --run='rebase,merge?cherry-pick' would match
     ++on all tests that match either the glob *rebase* or the glob
     ++*merge?cherry-pick*.
       
       If --run starts with an unprefixed number or range the initial
       set of tests to run is empty. If the first item starts with '!'
     +@@ t/README: all the tests are added to the initial set.  After initial set is
     + determined every test number or range is added or excluded from
     + the set one by one, from left to right.
     + 
     +-Individual numbers or ranges could be separated either by a space
     +-or a comma.
     +-
     + For example, to run only tests up to a specific test (21), one
     + could do this:
     + 
     +@@ t/README: or this:
     + Common case is to run several setup tests (1, 2, 3) and then a
     + specific test (21) that relies on that setup:
     + 
     +-    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
     ++    $ sh ./t9200-git-cvsexport-commit.sh --run='1,2,3,21'
     + 
     + or:
     + 
     +@@ t/README: or:
     + 
     + or:
     + 
     +-    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
     ++    $ sh ./t9200-git-cvsexport-commit.sh --run='-3,21'
     + 
     + As noted above, the test set is built by going through the items
     + from left to right, so this:
     + 
     +-    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
     ++    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4,!3'
     + 
     + will run tests 1, 2, and 4.  Items that come later have higher
     + precedence.  It means that this:
     + 
     +-    $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
     ++    $ sh ./t9200-git-cvsexport-commit.sh --run='!3,1-4'
     + 
     + would just run tests from 1 to 4, including 3.
     + 
      @@ t/README: test in the test suite except from 7 up to 11:
       
           $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
     @@ t/README: test in the test suite except from 7 up to 11:
       "setup" test, so you cannot _arbitrarily_ disable one test and
      
       ## t/t0000-basic.sh ##
     +@@ t/t0000-basic.sh: test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' "
     + 
     + test_expect_success '--run basic' "
     + 	run_sub_test_lib_test run-basic \
     +-		'--run basic' --run='1 3 5' <<-\\EOF &&
     ++		'--run basic' --run='1,3,5' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
     +@@ t/t0000-basic.sh: test_expect_success '--run with a range' "
     + 
     + test_expect_success '--run with two ranges' "
     + 	run_sub_test_lib_test run-two-ranges \
     +-		'--run with two ranges' --run='1-2 5-6' <<-\\EOF &&
     ++		'--run with two ranges' --run='1-2,5-6' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
     +@@ t/t0000-basic.sh: test_expect_success '--run with basic negation' "
     + 
     + test_expect_success '--run with two negations' "
     + 	run_sub_test_lib_test run-two-neg \
     +-		'--run with two negations' --run='"'!3 !6'"' <<-\\EOF &&
     ++		'--run with two negations' --run='"'!3,!6'"' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
     +@@ t/t0000-basic.sh: test_expect_success '--run with two negations' "
     + 
     + test_expect_success '--run a range and negation' "
     + 	run_sub_test_lib_test run-range-and-neg \
     +-		'--run a range and negation' --run='"'-4 !2'"' <<-\\EOF &&
     ++		'--run a range and negation' --run='"'-4,!2'"' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
     +@@ t/t0000-basic.sh: test_expect_success '--run range negation' "
     + test_expect_success '--run include, exclude and include' "
     + 	run_sub_test_lib_test run-inc-neg-inc \
     + 		'--run include, exclude and include' \
     +-		--run='"'1-5 !1-3 2'"' <<-\\EOF &&
     ++		--run='"'1-5,!1-3,2'"' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
     +@@ t/t0000-basic.sh: test_expect_success '--run include, exclude and include, comma separated' "
     + test_expect_success '--run exclude and include' "
     + 	run_sub_test_lib_test run-neg-inc \
     + 		'--run exclude and include' \
     +-		--run='"'!3- 5'"' <<-\\EOF &&
     ++		--run='"'!3-,5'"' <<-\\EOF &&
     + 	for i in 1 2 3 4 5 6
     + 	do
     + 		test_expect_success \"passing test #\$i\" 'true'
      @@ t/t0000-basic.sh: test_expect_success '--run empty selectors' "
       	EOF
       "
     @@ t/test-lib.sh: match_pattern_list () {
       	title="$1"
       	shift
       	arg="$1"
     +@@ t/test-lib.sh: match_test_selector_list () {
     + 
     + 	# Both commas and whitespace are accepted as separators.
     + 	OLDIFS=$IFS
     +-	IFS=' 	,'
     ++	IFS=','
     + 	set -- $1
     + 	IFS=$OLDIFS
     + 
      @@ t/test-lib.sh: match_test_selector_list () {
       			*-*)
       				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 2:  bbae029cc8 = 2:  7c7a3d9a34 t6006, t6012: adjust tests to use 'setup' instead of synonyms
 3:  0e1a1125f7 = 3:  01494bc0ba test-lib: reduce verbosity of skipped tests

-- 
gitgitgadget

[PATCH v5 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 48 +++++++++++++++++++++++++++----------------
 t/t0000-basic.sh | 53 ++++++++++++++++++++++++++++--------------------
 t/test-lib.sh    | 18 +++++++++-------
 3 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..c730a70770 100644
--- a/t/README
+++ b/t/README
@@ -258,16 +258,21 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
-
-Optional prefix of '!' means that the test or a range of tests
-should be excluded from the run.
+The argument for --run, <test-selector>, is a list of description
+substrings or globs or individual test numbers or ranges with an
+optional negation prefix (of '!') that define what tests in a test
+suite to include (or exclude, if negated) in the run.  A range is two
+numbers separated with a dash and matches a range of tests with both
+ends been included.  You may omit the first or the second number to
+mean "from the first test" or "up to the very last test" respectively.
+
+The argument to --run is split on commas into separate strings,
+numbers, and ranges, and picks all tests that match any of the
+individual selection criteria.  If the substring of the description
+text that you want to match includes a comma, use the glob character
+'?' instead.  For example --run='rebase,merge?cherry-pick' would match
+on all tests that match either the glob *rebase* or the glob
+*merge?cherry-pick*.
 
 If --run starts with an unprefixed number or range the initial
 set of tests to run is empty. If the first item starts with '!'
@@ -275,9 +280,6 @@ all the tests are added to the initial set.  After initial set is
 determined every test number or range is added or excluded from
 the set one by one, from left to right.
 
-Individual numbers or ranges could be separated either by a space
-or a comma.
-
 For example, to run only tests up to a specific test (21), one
 could do this:
 
@@ -290,7 +292,7 @@ or this:
 Common case is to run several setup tests (1, 2, 3) and then a
 specific test (21) that relies on that setup:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1,2,3,21'
 
 or:
 
@@ -298,17 +300,17 @@ or:
 
 or:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-3,21'
 
 As noted above, the test set is built by going through the items
 from left to right, so this:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4,!3'
 
 will run tests 1, 2, and 4.  Items that come later have higher
 precedence.  It means that this:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!3,1-4'
 
 would just run tests from 1 to 4, including 3.
 
@@ -317,6 +319,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/glob to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..62a16ed4c8 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -430,7 +430,7 @@ test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' "
 
 test_expect_success '--run basic' "
 	run_sub_test_lib_test run-basic \
-		'--run basic' --run='1 3 5' <<-\\EOF &&
+		'--run basic' --run='1,3,5' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -472,7 +472,7 @@ test_expect_success '--run with a range' "
 
 test_expect_success '--run with two ranges' "
 	run_sub_test_lib_test run-two-ranges \
-		'--run with two ranges' --run='1-2 5-6' <<-\\EOF &&
+		'--run with two ranges' --run='1-2,5-6' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -556,7 +556,7 @@ test_expect_success '--run with basic negation' "
 
 test_expect_success '--run with two negations' "
 	run_sub_test_lib_test run-two-neg \
-		'--run with two negations' --run='"'!3 !6'"' <<-\\EOF &&
+		'--run with two negations' --run='"'!3,!6'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -577,7 +577,7 @@ test_expect_success '--run with two negations' "
 
 test_expect_success '--run a range and negation' "
 	run_sub_test_lib_test run-range-and-neg \
-		'--run a range and negation' --run='"'-4 !2'"' <<-\\EOF &&
+		'--run a range and negation' --run='"'-4,!2'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -620,7 +620,7 @@ test_expect_success '--run range negation' "
 test_expect_success '--run include, exclude and include' "
 	run_sub_test_lib_test run-inc-neg-inc \
 		'--run include, exclude and include' \
-		--run='"'1-5 !1-3 2'"' <<-\\EOF &&
+		--run='"'1-5,!1-3,2'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -664,7 +664,7 @@ test_expect_success '--run include, exclude and include, comma separated' "
 test_expect_success '--run exclude and include' "
 	run_sub_test_lib_test run-neg-inc \
 		'--run exclude and include' \
-		--run='"'!3- 5'"' <<-\\EOF &&
+		--run='"'!3-,5'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..debfd73fd6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,6 +769,8 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
@@ -777,7 +779,7 @@ match_test_selector_list () {
 
 	# Both commas and whitespace are accepted as separators.
 	OLDIFS=$IFS
-	IFS=' 	,'
+	IFS=','
 	set -- $1
 	IFS=$OLDIFS
 
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,11 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					case "$title" in *${selector}*)
+						include=$positive
+						;;
+					esac
+					continue
 				fi
 		esac
 
@@ -1031,7 +1035,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

Re: [PATCH v5 1/3] test-lib: allow selecting tests by substring/glob with --run

[Andrei Rybak]

On 2020-10-17 00:50, Elijah Newren via GitGitGadget wrote:

[trim]

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ef31f40037..debfd73fd6 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -769,6 +769,8 @@ match_pattern_list () {
>  }
>  
>  match_test_selector_list () {
> +	operation="$1"
> +	shift
>  	title="$1"
>  	shift
>  	arg="$1"
> @@ -777,7 +779,7 @@ match_test_selector_list () {
>  
>  	# Both commas and whitespace are accepted as separators.

This comment becomes outdated with the change below.

>  	OLDIFS=$IFS
> -	IFS=' 	,'
> +	IFS=','
>  	set -- $1
>  	IFS=$OLDIFS
>  
> @@ -805,13 +807,13 @@ match_test_selector_list () {

Re: [PATCH v5 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren]

On Fri, Oct 16, 2020 at 4:25 PM Andrei Rybak <rybak.a.v@gmail.com> wrote:
>
> On 2020-10-17 00:50, Elijah Newren via GitGitGadget wrote:
>
> [trim]
>
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index ef31f40037..debfd73fd6 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -769,6 +769,8 @@ match_pattern_list () {
> >  }
> >
> >  match_test_selector_list () {
> > +     operation="$1"
> > +     shift
> >       title="$1"
> >       shift
> >       arg="$1"
> > @@ -777,7 +779,7 @@ match_test_selector_list () {
> >
> >       # Both commas and whitespace are accepted as separators.
>
> This comment becomes outdated with the change below.

Sigh...indeed.  Thanks for pointing it out; I'll send out v6 soon with the fix.

[PATCH v5 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH v5 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
(in addition to the TAP output line) adds up to hundreds or thousands of
lines of irrelevant output that make it very hard to fish out the
relevant results you were looking for.  Simplify the output for skipped
tests to remove this extra output, leaving only the TAP output line
(i.e. the line reading "ok <number> # skip <test-description>", which
already mentions that the test was "skip"ped).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index debfd73fd6..e020bcc5e7 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1062,7 +1062,6 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget

[PATCH v6 0/3] Make test selection easier by specifying description substrings instead of just numeric counters

[Elijah Newren via GitGitGadget]

This patch series enables me to change

 ./t9999-my-test.sh --run=1-3,5,17,19

into

 ./t9999-my-test.sh --run=setup,rename,symlink

and have it pick out tests with "setup", "rename", or "symlink" in their
description and run those. Saves me a lot of time, especially since numbers
for tests aren't readily accessible. The easiest way for me to get the
numbers corresponding to the tests I want to run, is to run all the tests
and look at the output to match up the descriptions with their numbers --
thus defeating the point of selecting just a subset of the tests to run in
the first place.

Changes since v5:

 * Update comment to match the code (selectors are now only split on commas,
   not on whitespace)

Elijah Newren (3):
  test-lib: allow selecting tests by substring/glob with --run
  t6006, t6012: adjust tests to use 'setup' instead of synonyms
  test-lib: reduce verbosity of skipped tests

 t/README                     | 48 ++++++++++++++++++++------------
 t/t0000-basic.sh             | 53 +++++++++++++++++++++---------------
 t/t6006-rev-list-format.sh   |  2 +-
 t/t6012-rev-list-simplify.sh |  2 +-
 t/test-lib.sh                | 21 ++++++++------
 5 files changed, 76 insertions(+), 50 deletions(-)


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-878%2Fnewren%2Ftest-selection-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-878/newren/test-selection-v6
Pull-Request: https://github.com/git/git/pull/878

Range-diff vs v5:

 1:  389c2c171a ! 1:  0355c88882 test-lib: allow selecting tests by substring/glob with --run
     @@ t/test-lib.sh: match_pattern_list () {
       	title="$1"
       	shift
       	arg="$1"
     -@@ t/test-lib.sh: match_test_selector_list () {
     + 	shift
     + 	test -z "$1" && return 0
       
     - 	# Both commas and whitespace are accepted as separators.
     +-	# Both commas and whitespace are accepted as separators.
     ++	# Commas are accepted as separators.
       	OLDIFS=$IFS
      -	IFS=' 	,'
      +	IFS=','
 2:  7c7a3d9a34 = 2:  3cc9abf461 t6006, t6012: adjust tests to use 'setup' instead of synonyms
 3:  01494bc0ba = 3:  0af7835487 test-lib: reduce verbosity of skipped tests

-- 
gitgitgadget

[PATCH v6 1/3] test-lib: allow selecting tests by substring/glob with --run

[Elijah Newren via GitGitGadget]

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'

Add such an ability to test selection which relies on merely matching
against the test description.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/README         | 48 +++++++++++++++++++++++++++----------------
 t/t0000-basic.sh | 53 ++++++++++++++++++++++++++++--------------------
 t/test-lib.sh    | 20 ++++++++++--------
 3 files changed, 74 insertions(+), 47 deletions(-)

diff --git a/t/README b/t/README
index 2adaf7c2d2..c730a70770 100644
--- a/t/README
+++ b/t/README
@@ -258,16 +258,21 @@ For an individual test suite --run could be used to specify that
 only some tests should be run or that some tests should be
 excluded from a run.
 
-The argument for --run is a list of individual test numbers or
-ranges with an optional negation prefix that define what tests in
-a test suite to include in the run.  A range is two numbers
-separated with a dash and matches a range of tests with both ends
-been included.  You may omit the first or the second number to
-mean "from the first test" or "up to the very last test"
-respectively.
-
-Optional prefix of '!' means that the test or a range of tests
-should be excluded from the run.
+The argument for --run, <test-selector>, is a list of description
+substrings or globs or individual test numbers or ranges with an
+optional negation prefix (of '!') that define what tests in a test
+suite to include (or exclude, if negated) in the run.  A range is two
+numbers separated with a dash and matches a range of tests with both
+ends been included.  You may omit the first or the second number to
+mean "from the first test" or "up to the very last test" respectively.
+
+The argument to --run is split on commas into separate strings,
+numbers, and ranges, and picks all tests that match any of the
+individual selection criteria.  If the substring of the description
+text that you want to match includes a comma, use the glob character
+'?' instead.  For example --run='rebase,merge?cherry-pick' would match
+on all tests that match either the glob *rebase* or the glob
+*merge?cherry-pick*.
 
 If --run starts with an unprefixed number or range the initial
 set of tests to run is empty. If the first item starts with '!'
@@ -275,9 +280,6 @@ all the tests are added to the initial set.  After initial set is
 determined every test number or range is added or excluded from
 the set one by one, from left to right.
 
-Individual numbers or ranges could be separated either by a space
-or a comma.
-
 For example, to run only tests up to a specific test (21), one
 could do this:
 
@@ -290,7 +292,7 @@ or this:
 Common case is to run several setup tests (1, 2, 3) and then a
 specific test (21) that relies on that setup:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1,2,3,21'
 
 or:
 
@@ -298,17 +300,17 @@ or:
 
 or:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-3,21'
 
 As noted above, the test set is built by going through the items
 from left to right, so this:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4,!3'
 
 will run tests 1, 2, and 4.  Items that come later have higher
 precedence.  It means that this:
 
-    $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!3,1-4'
 
 would just run tests from 1 to 4, including 3.
 
@@ -317,6 +319,18 @@ test in the test suite except from 7 up to 11:
 
     $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 
+Sometimes there may be multiple tests with e.g. "setup" in their name
+that are needed and rather than figuring out the number for all of them
+we can just use "setup" as a substring/glob to match against the test
+description:
+
+    $ sh ./t0050-filesystem.sh --run=setup,9-11
+
+or one could select both the setup tests and the rename ones (assuming all
+relevant tests had those words in their descriptions):
+
+    $ sh ./t0050-filesystem.sh --run=setup,rename
+
 Some tests in a test suite rely on the previous tests performing
 certain actions, specifically some tests are designated as
 "setup" test, so you cannot _arbitrarily_ disable one test and
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 923281af93..62a16ed4c8 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -430,7 +430,7 @@ test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' "
 
 test_expect_success '--run basic' "
 	run_sub_test_lib_test run-basic \
-		'--run basic' --run='1 3 5' <<-\\EOF &&
+		'--run basic' --run='1,3,5' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -472,7 +472,7 @@ test_expect_success '--run with a range' "
 
 test_expect_success '--run with two ranges' "
 	run_sub_test_lib_test run-two-ranges \
-		'--run with two ranges' --run='1-2 5-6' <<-\\EOF &&
+		'--run with two ranges' --run='1-2,5-6' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -556,7 +556,7 @@ test_expect_success '--run with basic negation' "
 
 test_expect_success '--run with two negations' "
 	run_sub_test_lib_test run-two-neg \
-		'--run with two negations' --run='"'!3 !6'"' <<-\\EOF &&
+		'--run with two negations' --run='"'!3,!6'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -577,7 +577,7 @@ test_expect_success '--run with two negations' "
 
 test_expect_success '--run a range and negation' "
 	run_sub_test_lib_test run-range-and-neg \
-		'--run a range and negation' --run='"'-4 !2'"' <<-\\EOF &&
+		'--run a range and negation' --run='"'-4,!2'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -620,7 +620,7 @@ test_expect_success '--run range negation' "
 test_expect_success '--run include, exclude and include' "
 	run_sub_test_lib_test run-inc-neg-inc \
 		'--run include, exclude and include' \
-		--run='"'1-5 !1-3 2'"' <<-\\EOF &&
+		--run='"'1-5,!1-3,2'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -664,7 +664,7 @@ test_expect_success '--run include, exclude and include, comma separated' "
 test_expect_success '--run exclude and include' "
 	run_sub_test_lib_test run-neg-inc \
 		'--run exclude and include' \
-		--run='"'!3- 5'"' <<-\\EOF &&
+		--run='"'!3-,5'"' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
 		test_expect_success \"passing test #\$i\" 'true'
@@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' "
 	EOF
 "
 
-test_expect_success '--run invalid range start' "
+test_expect_success '--run substring selector' "
+	run_sub_test_lib_test run-substring-selector \
+		'--run empty selectors' \
+		--run='relevant' <<-\\EOF &&
+	test_expect_success \"relevant test\" 'true'
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"other test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-substring-selector <<-\\EOF
+	> ok 1 - relevant test
+	> ok 2 # skip other test #1 (--run)
+	> ok 3 # skip other test #2 (--run)
+	> ok 4 # skip other test #3 (--run)
+	> ok 5 # skip other test #4 (--run)
+	> ok 6 # skip other test #5 (--run)
+	> ok 7 # skip other test #6 (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
+test_expect_success '--run keyword selection' "
 	run_sub_test_lib_test_err run-inv-range-start \
 		'--run invalid range start' \
 		--run='a-5' <<-\\EOF &&
@@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-test_expect_success '--run invalid selector' "
-	run_sub_test_lib_test_err run-inv-selector \
-		'--run invalid selector' \
-		--run='1?' <<-\\EOF &&
-	test_expect_success \"passing test #1\" 'true'
-	test_done
-	EOF
-	check_sub_test_lib_test_err run-inv-selector \
-		<<-\\EOF_OUT 3<<-\\EOF_ERR
-	> FATAL: Unexpected exit with code 1
-	EOF_OUT
-	> error: --run: invalid non-numeric in test selector: '1?'
-	EOF_ERR
-"
-
 
 test_set_prereq HAVEIT
 haveit=no
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..72b88dffa1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -769,15 +769,17 @@ match_pattern_list () {
 }
 
 match_test_selector_list () {
+	operation="$1"
+	shift
 	title="$1"
 	shift
 	arg="$1"
 	shift
 	test -z "$1" && return 0
 
-	# Both commas and whitespace are accepted as separators.
+	# Commas are accepted as separators.
 	OLDIFS=$IFS
-	IFS=' 	,'
+	IFS=','
 	set -- $1
 	IFS=$OLDIFS
 
@@ -805,13 +807,13 @@ match_test_selector_list () {
 			*-*)
 				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"start: '$orig_selector'" >&2
 					exit 1
 				fi
 				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in range" \
+					echo "error: $operation: invalid non-numeric in range" \
 						"end: '$orig_selector'" >&2
 					exit 1
 				fi
@@ -819,9 +821,11 @@ match_test_selector_list () {
 			*)
 				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
 				then
-					echo "error: $title: invalid non-numeric in test" \
-						"selector: '$orig_selector'" >&2
-					exit 1
+					case "$title" in *${selector}*)
+						include=$positive
+						;;
+					esac
+					continue
 				fi
 		esac
 
@@ -1031,7 +1035,7 @@ test_skip () {
 		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$run_list" &&
-	   ! match_test_selector_list '--run' $test_count "$run_list"
+	   ! match_test_selector_list '--run' "$1" $test_count "$run_list"
 	then
 		to_skip=t
 		skipped_reason="--run"
-- 
gitgitgadget

[PATCH v6 2/3] t6006, t6012: adjust tests to use 'setup' instead of synonyms

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

With the new ability to pass --run=setup to select which tests to run,
it is more convenient if tests use the term "setup" instead of synonyms
like 'prepare' or 'rebuild'.  There are undoubtedly many other tests in
our testsuite that could be changed over too, these are just a couple
that I ran into.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index bc95da8a5f..99a1eaf332 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -339,7 +339,7 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_expect_success 'prepare expected messages (for test %b)' '
+test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
 	This commit message is much longer than the others,
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index b6fa43ace0..7254060240 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -168,7 +168,7 @@ test_expect_success '--full-diff is not affected by --parents' '
 #
 # This example is explained in Documentation/rev-list-options.txt
 
-test_expect_success 'rebuild repo' '
+test_expect_success 'setup rebuild repo' '
 	rm -rf .git * &&
 	git init &&
 	git switch -c main &&
-- 
gitgitgadget

[PATCH v6 3/3] test-lib: reduce verbosity of skipped tests

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

When using the --run flag to run just two or three tests from a test
file which contains several dozen tests, having every skipped test print
out dozens of lines of output for the test code for that skipped test
(in addition to the TAP output line) adds up to hundreds or thousands of
lines of irrelevant output that make it very hard to fish out the
relevant results you were looking for.  Simplify the output for skipped
tests to remove this extra output, leaving only the TAP output line
(i.e. the line reading "ok <number> # skip <test-description>", which
already mentions that the test was "skip"ped).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 72b88dffa1..f68bca745a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1062,7 +1062,6 @@ test_skip () {
 				"      <skipped message=\"$message\" />"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
-- 
gitgitgadget