Make test selection easier by specifying description substrings instead of just numeric counters

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

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 26 +++++++++++++++++++-------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 13 +++++++------
 3 files changed, 51 insertions(+), 29 deletions(-)

index 2adaf7c2d2..23639c5bcf 100644
@@ -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

index ef31f40037..2aca398e1e 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 2aca398e1e..7602bbe9e9 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

Hi Elijah

I'm worryingly excited by this series

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

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

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.

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.

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 29 +++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 13 +++++++------
 3 files changed, 51 insertions(+), 32 deletions(-)

index 2adaf7c2d2..5fd8eaf595 100644
@@ -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

index ef31f40037..2aca398e1e 100644
@@ -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

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

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

index 2aca398e1e..fe81d9c733 100644
@@ -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

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.

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

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

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.

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.

Make sense, thanks.

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.

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)?

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

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

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

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.

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).

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

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

Oh, indeed. How'd I miss that?

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.

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.

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

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 <[email protected]>

## 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).

gitgitgadget

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

From: Elijah Newren <[email protected]>

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).

---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

index a040d54a76..3f8c404335 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 29 +++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 16 ++++++++++------
 3 files changed, 54 insertions(+), 32 deletions(-)

index 2adaf7c2d2..0a8b60b5c7 100644
@@ -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

index ef31f40037..a040d54a76 100644
@@ -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

Hi,

FWIW I am in the same boat.

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

I'm glad it's not just me!

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

Phillip

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.

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.

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

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

Thanks.

The sub-thread between me and Jonathan starting here:

https://lore.kernel.org/git/[email protected]/

but specifically:

https://lore.kernel.org/git/[email protected]/

-Peff

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

-Peff

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.

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.

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).

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 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

Hi Elijah

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

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).

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.

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

Thanks.

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.

Hi Phillip,

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.

Hi Elijah

That's a shame

I agree

'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

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 37 +++++++++++++++++++++++++++----------
 t/t0000-basic.sh | 41 +++++++++++++++++++++++++----------------
 t/test-lib.sh    | 16 ++++++++++------
 3 files changed, 62 insertions(+), 32 deletions(-)

index 2adaf7c2d2..4594f3ab7b 100644
@@ -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

index ef31f40037..a040d54a76 100644
@@ -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

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

From: Elijah Newren <[email protected]>

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).

---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

index a040d54a76..3f8c404335 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

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.

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

With your follow-up response:

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.

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

-Peff

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.

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

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.

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

From: Elijah Newren <[email protected]>

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).

---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

index debfd73fd6..e020bcc5e7 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 48 +++++++++++++++++++++++++++----------------
 t/t0000-basic.sh | 53 ++++++++++++++++++++++++++++--------------------
 t/test-lib.sh    | 18 +++++++++-------
 3 files changed, 73 insertions(+), 46 deletions(-)

index 2adaf7c2d2..c730a70770 100644
@@ -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

index ef31f40037..debfd73fd6 100644
@@ -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

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

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

[trim]

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

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

From: Elijah Newren <[email protected]>

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.

---
 t/README         | 48 +++++++++++++++++++++++++++----------------
 t/t0000-basic.sh | 53 ++++++++++++++++++++++++++++--------------------
 t/test-lib.sh    | 20 ++++++++++--------
 3 files changed, 74 insertions(+), 47 deletions(-)

index 2adaf7c2d2..c730a70770 100644
@@ -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

index ef31f40037..72b88dffa1 100644
@@ -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

From: Elijah Newren <[email protected]>

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.

---
 t/t6006-rev-list-format.sh   | 2 +-
 t/t6012-rev-list-simplify.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

index bc95da8a5f..99a1eaf332 100755
@@ -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,

index b6fa43ace0..7254060240 100755
@@ -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

From: Elijah Newren <[email protected]>

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).

---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

index 72b88dffa1..f68bca745a 100644
@@ -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