All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] Better control of the tests run by a test suite
@ 2014-03-24  8:49 Ilya Bobyr
  2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-24  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine

Hello,

This is a second attempt on a functionality I proposed in

    [PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
    http://www.mail-archive.com/git%40vger.kernel.org/msg44828.html

except that the implementation is quite different now.

I hope that I have accounted for the comments that were voiced so
far.  Let's see :)

The idea behind the change is that sometimes it is convenient to
run only certain tests from a test suite.  Specifically I am
thinking about the following two use cases:

 1. You are developing new functionality.  You add a test that
    fails and then you add and modify code to make it pass.
    
 2. You have a failed test and you need to understand what is
    wrong.
    
In the first case you when you run the test suite, you probably
want to run some setup tests and then only one test that you are
focused on.

For code written in C time between you make a change and see a
test result is considerably increased by the compilation.  But
for script code turn around time is mostly due to the run time of
the test suite itself. [1]

For the second case you actually want the test suite to stop
after the failing test, so that you can examine the trash
directory without any modifications from the subsequent tests.
You probably do not care about them.

In the previous patch I've added an environment variable to
control tests to be run in a test suite.  I thought that it would
be similar to an already existing GIT_SKIP_TESTS.  As I did not
provide a cover letter that caused some misunderstanding I think.

This patch adds new command line argument '--run' that accepts a
list of patterns and restrictions on the test numbers that would
be included or excluded from this run of the test suite.

During discussion of the previous patch there were some talks
about extending GIT_SKIP_TESTS syntax.  In particular here:

> That is
> 
>         GIT_SKIP_TESTS='t9??? !t91??'
> 
> would skip nine-thousand series, but would run 91xx series, and all
> the others are not excluded.
> 
> Simple rules to consider:
> 
>  - If the list consists of _only_ negated patterns, pretend that
>    there is "unless otherwise specified with negatives, skip all
>    tests", i.e. treat GIT_SKIP_TESTS='!t91??' just the same way you
>    would treat GIT_SKIP_TESTS='* !t91??'.
> 
>  - The orders should not matter for simplicity of the semantics;
>    before running each test, check if it matches any negative (and
>    run it if it matches, without looking at any positives), and
>    otherwise check if it matches any positive (and skip it if it
>    does not).
> 
> Hmm?

    http://www.mail-archive.com/git%40vger.kernel.org/msg44922.html


I've used that as a basis, but the end result is somewhat
different.  Plus I've added boundary checks as in "<123".

Here are some examples of how functionality added by the patch
could be used.  In order to run setup tests and then only a
specific test (use case 1) one can do:

    $ ./t0000-init.sh --run='1 2 25'

or:

    $ ./t0000-init.sh --run='<3 25'

('<=' is also supported, as well as '>' and '>=').

In order to run up to a specific test (use case 2) one can do:

    $ ./t0000-init.sh --run='<=34'

or:

    $ ./t0000-init.sh --run='!>34'

Simple semantics described above is easy to implement, but at the
same time is probably not that convenient.  Rules implemented by
the patch are as follows:

 - Order does matter.  Whatever is specified on the right has
   higher precedence.

 - When the first pattern is positive the initial set of the
   tests to be run is empty.  You are adding to an empty set as
   in '1 2 25'.

   When the first pattern is negative the initial set of the
   tests to run contains all the tests.  You are subtracting
   from that set as in '!>34'.

It seems that for simple cases that gives simple syntax and is
almost unbiased if you think about preferring inclusion over
exclusion.  While it is unlikely it also allows for complicated
expressions.  And the implementation is quite simple as well.

Main functionality is in the third patch.  First two are just
minor fixes in related parts of the code.

P.S. I did not reply to the previous patch thread as this one is
quite different.


[1] Here are some times I see on Cygin:

    $ touch builtin/rev-parse.c
    
    $ time make
    ...
    
    real    0m10.382s
    user    0m3.829s
    sys     0m5.269s

Running the t0000-init.sh test suite is like this:

    $ time ./t0001-init.sh
    [...]
    1..36

    real    0m6.693s
    user    0m1.505s
    sys     0m3.937s

If I run only the 1, 2, 4 and 5th tests, it only half the time to
run the tests:

    $ time GIT_SKIP_TESTS='t0001.[36789] t0001.??' ./t0001-init.sh
    [...]
    1..36

    real    0m3.313s
    user    0m0.769s
    sys     0m1.844s 

Overall the change is from 17 to 14 seconds it is not that big.

If you only consider the test suite, as you do while you develop
an sh based tool, for example, the change is from 6.6 to 3.3
seconds.  That is quite noticeable.


 t/README         |   75 ++++++++++++--
 t/t0000-basic.sh |  296 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   96 +++++++++++++++++-
 3 files changed, 454 insertions(+), 13 deletions(-)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
@ 2014-03-24  8:49 ` Ilya Bobyr
  2014-03-24 11:39   ` Ramsay Jones
  2014-03-25  5:52   ` Eric Sunshine
  2014-03-24  8:49 ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-24  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ilya Bobyr

Most arguments that could be provided to a test have short forms.
Unless documented the only way to learn then is to read the code.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 t/README |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/README b/t/README
index caeeb9d..ccb5989 100644
--- a/t/README
+++ b/t/README
@@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
 (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
 appropriately before running "make".
 
---verbose::
+-v,--verbose::
 	This makes the test more verbose.  Specifically, the
 	command being run and their output if any are also
 	output.
@@ -81,7 +81,7 @@ appropriately before running "make".
 	numbers matching <pattern>.  The number matched against is
 	simply the running count of the test within the file.
 
---debug::
+-d,--debug::
 	This may help the person who is developing a new test.
 	It causes the command defined with test_debug to run.
 	The "trash" directory (used to store all temporary data
@@ -89,18 +89,18 @@ appropriately before running "make".
 	failed tests so that you can inspect its contents after
 	the test finished.
 
---immediate::
+-i,--immediate::
 	This causes the test to immediately exit upon the first
 	failed test. Cleanup commands requested with
 	test_when_finished are not executed if the test failed,
 	in order to keep the state for inspection by the tester
 	to diagnose the bug.
 
---long-tests::
+-l,--long-tests::
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
---valgrind=<tool>::
+-v,--valgrind=<tool>::
 	Execute all Git binaries under valgrind tool <tool> and exit
 	with status 126 on errors (just like regular tests, this will
 	only stop the test script when running under -i).
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so
  2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
  2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
@ 2014-03-24  8:49 ` Ilya Bobyr
  2014-03-24  8:49 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-24  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ilya Bobyr

We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS.  Use "(GIT_SKIP_TESTS)" instead.

Plus tests that check basic GIT_SKIP_TESTS functions.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 t/t0000-basic.sh |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   13 ++++++----
 2 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index a2bb63c..ae8874e 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -270,6 +270,69 @@ test_expect_success 'test --verbose-only' '
 	EOF
 '
 
+test_expect_success 'GIT_SKIP_TESTS' "
+	GIT_SKIP_TESTS='git.2' \
+		run_sub_test_lib_test git-skip-tests-basic \
+		'GIT_SKIP_TESTS' <<-\\EOF &&
+	for i in 1 2 3
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 - passing test #3
+	> # passed all 3 test(s)
+	> 1..3
+	EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS several tests' "
+	GIT_SKIP_TESTS='git.2 git.5' \
+		run_sub_test_lib_test git-skip-tests-several \
+		'GIT_SKIP_TESTS several tests' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-several <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS sh pattern' "
+	GIT_SKIP_TESTS='git.[2-5]' \
+		run_sub_test_lib_test git-skip-tests-sh-pattern \
+		'GIT_SKIP_TESTS sh pattern' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-sh-pattern <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 # skip passing test #3 (GIT_SKIP_TESTS)
+	> ok 4 # skip passing test #4 (GIT_SKIP_TESTS)
+	> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 569b52d..e035f36 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -452,25 +452,28 @@ test_finish_ () {
 
 test_skip () {
 	to_skip=
+	skipped_reason=
 	if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
 	then
 		to_skip=t
+		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$test_prereq" &&
 	   ! test_have_prereq "$test_prereq"
 	then
 		to_skip=t
-	fi
-	case "$to_skip" in
-	t)
+
 		of_prereq=
 		if test "$missing_prereq" != "$test_prereq"
 		then
 			of_prereq=" of $test_prereq"
 		fi
-
+		skipped_reason="missing $missing_prereq${of_prereq}"
+	fi
+	case "$to_skip" in
+	t)
 		say_color skip >&3 "skipping test: $@"
-		say_color skip "ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
+		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
 	*)
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
  2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
  2014-03-24  8:49 ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
@ 2014-03-24  8:49 ` Ilya Bobyr
  2014-03-24 23:03 ` [RFC/PATCH] Better control of the tests run by a test suite Jeff King
  2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
  4 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-24  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ilya Bobyr

Allow better control of the set of tests that will be executed for a
single test suite.  Mostly useful while debugging or developing as it
allows to focus on a specific test.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 t/README         |   65 ++++++++++++++-
 t/t0000-basic.sh |  233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   85 ++++++++++++++++++++
 3 files changed, 379 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index ccb5989..519f0dc 100644
--- a/t/README
+++ b/t/README
@@ -100,6 +100,10 @@ appropriately before running "make".
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+-r,--run=<test numbers>::
+	This causes only specific tests to be included or excluded.  See
+	section "Skipping Tests" below for "<test numbers>" syntax.
+
 -v,--valgrind=<tool>::
 	Execute all Git binaries under valgrind tool <tool> and exit
 	with status 126 on errors (just like regular tests, this will
@@ -187,10 +191,63 @@ and either can match the "t[0-9]{4}" part to skip the whole
 test, or t[0-9]{4} followed by ".$number" to say which
 particular test to skip.
 
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+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.
+
+--run argument is a list of patterns with optional prefixes that
+are matched against test numbers within the current test suite.
+Supported pattern:
+
+ - A number matches a test with that number.
+
+ - sh metacharacters such as '*', '?' and '[]' match as usual in
+   shell.
+
+ - A number prefixed with '<', '<=', '>', or '>=' matches all
+   tests 'before', 'before or including', 'after', or 'after or
+   including' the specified one.
+
+Optional prefixes are:
+
+ - '+' or no prefix: test(s) matching the pattern are included in
+   the run.
+
+ - '-' or '!': test(s) matching the pattern are exluded from the
+   run.
+
+If --run starts with '+' or unprefixed pattern the initial set of
+tests to run is empty. If the first pattern starts with '-' or
+'!' all the tests are added to the initial set.  After initial
+set is determined every pattern, test number or range is added or
+excluded from the set one by one, from left to right.
+
+For example, common case is to run several setup tests and then a
+specific test that relies on that setup:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='<4 21'
+
+To run only tests up to a specific test one could do this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!>=21'
+
+As noted above test set is build going though patterns left to
+right, so this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='<5 !3'
+
+will run tests 1, 2, and 4.
+
+Some tests in the existing test suite rely on previous test item,
+so you cannot arbitrarily disable one and expect the remainder of
+test to check what the test originally was intended to check.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing.  Or when you want to run up to a
+certain test.
 
 
 Naming Tests
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae8874e..4da527f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -333,6 +333,239 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
 	EOF
 "
 
+test_expect_success '--run basic' "
+	run_sub_test_lib_test run-basic \
+		'--run basic' --run='1 3 5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with <' "
+	run_sub_test_lib_test run-lt \
+		'--run with <' --run='<4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-lt <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with <=' "
+	run_sub_test_lib_test run-le \
+		'--run with <=' --run='<=4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-le <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with >' "
+	run_sub_test_lib_test run-gt \
+		'--run with >' --run='>4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-gt <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with >=' "
+	run_sub_test_lib_test run-ge \
+		'--run with >=' --run='>=4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-ge <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with basic negation' "
+	run_sub_test_lib_test run-basic-neg \
+		'--run with basic negation' --run='-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with two negations' "
+	run_sub_test_lib_test run-two-neg \
+		'--run with two negation' --run='"'!3 !6'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run < and negation' "
+	run_sub_test_lib_test run-lt-neg \
+		'--run < and negation' --run='"'<5 !2'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-lt-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run range negation' "
+	run_sub_test_lib_test run-range-neg \
+		'--run range negation' --run='-<3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-neg <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run include, exclude and include' "
+	run_sub_test_lib_test run-inc-neg-inc \
+		'--run include, exclude and include' \
+		--run='"'<=5 !<3 1'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+test_expect_success '--run exclude and include' "
+	run_sub_test_lib_test run-neg-inc \
+		'--run exclude and include' \
+		--run='"'!>2 5'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e035f36..63e481a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -191,6 +191,14 @@ do
 		immediate=t; shift ;;
 	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
 		GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
+	-r)
+		shift; test "$#" -ne 0 || {
+			echo 'error: -r requires an argument' >&2;
+			exit 1;
+		}
+		run_list=$1; shift ;;
+	--run=*)
+		run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
 	-h|--h|--he|--hel|--help)
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
@@ -366,6 +374,76 @@ match_pattern_list () {
 	return 1
 }
 
+match_run_pattern_list () {
+	arg="$1"
+	shift
+	test -z "$*" && return 0
+
+	# If the first patern is negative we include by default.
+	include=
+	case "$1" in
+		[-!]*) include=t ;;
+	esac
+
+	for pattern_
+	do
+		orig_pattern=$pattern_
+
+		positive=t
+		case "$pattern_" in
+			[-!]*)
+				positive=
+				pattern_=${pattern_##?}
+				;;
+		esac
+
+		# Short cut for "obvious" cases
+		[ "x$include" = "x" -a "x$positive" = "x" ] && continue
+		[ "x$include" = "xt" -a "x$positive" = "xt" ] && continue
+
+		pattern_op=
+		case "$pattern_" in
+			\<=*)
+				pattern_op='-le'
+				pattern_=${pattern_##??}
+				;;
+			\<*)
+				pattern_op='-lt'
+				pattern_=${pattern_##?}
+				;;
+			\>=*)
+				pattern_op='-ge'
+				pattern_=${pattern_##??}
+				;;
+			\>*)
+				pattern_op='-gt'
+				pattern_=${pattern_##?}
+				;;
+		esac
+
+		if test -n "$pattern_op"
+		then
+			if expr "z$pattern_" : "z[0-9]*[^0-9]" >/dev/null
+			then
+				echo "error: --run: test number contains" \
+					"non-digits: '$orig_pattern'" >&2
+				exit 1
+			fi
+			if test $arg $pattern_op $pattern_
+			then
+				include=$positive
+			fi
+		else
+			case "$arg" in
+				$pattern_)
+					include=$positive
+			esac
+		fi
+	done
+
+	test -n "$include"
+}
+
 maybe_teardown_verbose () {
 	test -z "$verbose_only" && return
 	exec 4>/dev/null 3>/dev/null
@@ -470,6 +548,13 @@ test_skip () {
 		fi
 		skipped_reason="missing $missing_prereq${of_prereq}"
 	fi
+	if test -z "$to_skip" && test -n "$run_list" &&
+		! match_run_pattern_list $test_count $run_list
+	then
+		to_skip=t
+		skipped_reason="--run"
+	fi
+
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
@ 2014-03-24 11:39   ` Ramsay Jones
  2014-03-24 17:19     ` Ilya Bobyr
  2014-03-25  5:52   ` Eric Sunshine
  1 sibling, 1 reply; 32+ messages in thread
From: Ramsay Jones @ 2014-03-24 11:39 UTC (permalink / raw)
  To: Ilya Bobyr, git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine

On 24/03/14 08:49, Ilya Bobyr wrote:
> Most arguments that could be provided to a test have short forms.
> Unless documented the only way to learn then is to read the code.
> 
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  t/README |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/t/README b/t/README
> index caeeb9d..ccb5989 100644
> --- a/t/README
> +++ b/t/README
> @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
>  (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
>  appropriately before running "make".
>  
> ---verbose::
> +-v,--verbose::

OK

>  	This makes the test more verbose.  Specifically, the
>  	command being run and their output if any are also
>  	output.
> @@ -81,7 +81,7 @@ appropriately before running "make".
>  	numbers matching <pattern>.  The number matched against is
>  	simply the running count of the test within the file.
>  
> ---debug::
> +-d,--debug::
>  	This may help the person who is developing a new test.
>  	It causes the command defined with test_debug to run.
>  	The "trash" directory (used to store all temporary data
> @@ -89,18 +89,18 @@ appropriately before running "make".
>  	failed tests so that you can inspect its contents after
>  	the test finished.
>  
> ---immediate::
> +-i,--immediate::
>  	This causes the test to immediately exit upon the first
>  	failed test. Cleanup commands requested with
>  	test_when_finished are not executed if the test failed,
>  	in order to keep the state for inspection by the tester
>  	to diagnose the bug.
>  
> ---long-tests::
> +-l,--long-tests::
>  	This causes additional long-running tests to be run (where
>  	available), for more exhaustive testing.
>  
> ---valgrind=<tool>::
> +-v,--valgrind=<tool>::

The -v short option is taken, above ... :-P

>  	Execute all Git binaries under valgrind tool <tool> and exit
>  	with status 126 on errors (just like regular tests, this will
>  	only stop the test script when running under -i).
> 

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-24 11:39   ` Ramsay Jones
@ 2014-03-24 17:19     ` Ilya Bobyr
  2014-03-25 17:23       ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-24 17:19 UTC (permalink / raw)
  To: Ramsay Jones, git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine

On 3/24/2014 4:39 AM, Ramsay Jones wrote:
> On 24/03/14 08:49, Ilya Bobyr wrote:
>> Most arguments that could be provided to a test have short forms.
>> Unless documented the only way to learn then is to read the code.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>>  t/README |   10 +++++-----
>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/t/README b/t/README
>> index caeeb9d..ccb5989 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
>>  (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
>>  appropriately before running "make".
>>  
>> ---verbose::
>> +-v,--verbose::
> OK
>
>> [...]
>>  
>> ---valgrind=<tool>::
>> +-v,--valgrind=<tool>::
> The -v short option is taken, above ... :-P

Right %)
Thanks :)
This one starts only with "--va", will fix it.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH] Better control of the tests run by a test suite
  2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
                   ` (2 preceding siblings ...)
  2014-03-24  8:49 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
@ 2014-03-24 23:03 ` Jeff King
  2014-03-25  4:58   ` Junio C Hamano
  2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
  4 siblings, 1 reply; 32+ messages in thread
From: Jeff King @ 2014-03-24 23:03 UTC (permalink / raw)
  To: Ilya Bobyr
  Cc: git, Junio C Hamano, Thomas Rast, Eric Sunshine, Jonathan Nieder

On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:

> Here are some examples of how functionality added by the patch
> could be used.  In order to run setup tests and then only a
> specific test (use case 1) one can do:
> 
>     $ ./t0000-init.sh --run='1 2 25'
> 
> or:
> 
>     $ ./t0000-init.sh --run='<3 25'
> 
> ('<=' is also supported, as well as '>' and '>=').

I don't have anything against this in principle, but I suspect it will
end up being a big pain to figure out which of the early tests are
required to set up the state, and which are not. Having "<" makes
specifying it easier, but you still have to read the test script to
figure out which tests need to be run.

I wonder if it would make sense to "auto-select" tests that match a
regex like "set.?up|create"? A while ago, Jonathan made a claim that
this would cover most tests that are dependencies of other tests. I did
not believe him, but looking into it, I recall that we did seem to have
quite a few matching that pattern. If there were a good feature like
this that gave us a reason to follow that pattern, I think people might
fix the remainder

-Peff

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH] Better control of the tests run by a test suite
  2014-03-24 23:03 ` [RFC/PATCH] Better control of the tests run by a test suite Jeff King
@ 2014-03-25  4:58   ` Junio C Hamano
  2014-03-27 10:15     ` Ilya Bobyr
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-03-25  4:58 UTC (permalink / raw)
  To: Jeff King; +Cc: Ilya Bobyr, git, Thomas Rast, Eric Sunshine, Jonathan Nieder

Jeff King <peff@peff.net> writes:

> On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:
>
>> Here are some examples of how functionality added by the patch
>> could be used.  In order to run setup tests and then only a
>> specific test (use case 1) one can do:
>> 
>>     $ ./t0000-init.sh --run='1 2 25'
>> 
>> or:
>> 
>>     $ ./t0000-init.sh --run='<3 25'
>> 
>> ('<=' is also supported, as well as '>' and '>=').
>
> I don't have anything against this in principle, but I suspect it will
> end up being a big pain to figure out which of the early tests are
> required to set up the state, and which are not. Having "<" makes
> specifying it easier, but you still have to read the test script to
> figure out which tests need to be run.

Likewise.

> I wonder if it would make sense to "auto-select" tests that match a
> regex like "set.?up|create"? A while ago, Jonathan made a claim that
> this would cover most tests that are dependencies of other tests. I did
> not believe him, but looking into it, I recall that we did seem to have
> quite a few matching that pattern. If there were a good feature like
> this that gave us a reason to follow that pattern, I think people might
> fix the remainder

This may be worth experimenting with, I would think.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
  2014-03-24 11:39   ` Ramsay Jones
@ 2014-03-25  5:52   ` Eric Sunshine
  1 sibling, 0 replies; 32+ messages in thread
From: Eric Sunshine @ 2014-03-25  5:52 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Git List, Junio C Hamano, Thomas Rast

On Mon, Mar 24, 2014 at 4:49 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
> Most arguments that could be provided to a test have short forms.
> Unless documented the only way to learn then is to read the code.

s/then/them/

(Also, add a comma after "documented".)

> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  t/README |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/t/README b/t/README
> index caeeb9d..ccb5989 100644
> --- a/t/README
> +++ b/t/README
> @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
>  (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
>  appropriately before running "make".
>
> ---verbose::
> +-v,--verbose::
>         This makes the test more verbose.  Specifically, the
>         command being run and their output if any are also
>         output.
> @@ -81,7 +81,7 @@ appropriately before running "make".
>         numbers matching <pattern>.  The number matched against is
>         simply the running count of the test within the file.
>
> ---debug::
> +-d,--debug::
>         This may help the person who is developing a new test.
>         It causes the command defined with test_debug to run.
>         The "trash" directory (used to store all temporary data
> @@ -89,18 +89,18 @@ appropriately before running "make".
>         failed tests so that you can inspect its contents after
>         the test finished.
>
> ---immediate::
> +-i,--immediate::
>         This causes the test to immediately exit upon the first
>         failed test. Cleanup commands requested with
>         test_when_finished are not executed if the test failed,
>         in order to keep the state for inspection by the tester
>         to diagnose the bug.
>
> ---long-tests::
> +-l,--long-tests::
>         This causes additional long-running tests to be run (where
>         available), for more exhaustive testing.
>
> ---valgrind=<tool>::
> +-v,--valgrind=<tool>::
>         Execute all Git binaries under valgrind tool <tool> and exit
>         with status 126 on errors (just like regular tests, this will
>         only stop the test script when running under -i).
> --
> 1.7.9
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-24 17:19     ` Ilya Bobyr
@ 2014-03-25 17:23       ` Junio C Hamano
  2014-03-27  9:39         ` Ilya Bobyr
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-03-25 17:23 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Ramsay Jones, git, Thomas Rast, Eric Sunshine

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> On 3/24/2014 4:39 AM, Ramsay Jones wrote:
>> On 24/03/14 08:49, Ilya Bobyr wrote:
>>> Most arguments that could be provided to a test have short forms.
>>> Unless documented the only way to learn then is to read the code.
>>>
>>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>>> ---
>>>  t/README |   10 +++++-----
>>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/t/README b/t/README
>>> index caeeb9d..ccb5989 100644
>>> --- a/t/README
>>> +++ b/t/README
>>> @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
>>>  (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
>>>  appropriately before running "make".
>>>  
>>> ---verbose::
>>> +-v,--verbose::
>> OK
>>
>>> [...]
>>>  
>>> ---valgrind=<tool>::
>>> +-v,--valgrind=<tool>::
>> The -v short option is taken, above ... :-P
>
> Right %)
> Thanks :)
> This one starts only with "--va", will fix it.

Please don't.

In general, when option names can be shortened by taking a unique
prefix, it is better not to give short form in the documentation at
all.  There is no guarantee that the short form you happen to pick
when you document it will continue to be unique forever.  When we
add another --vasomething option, --va will become ambiguous and one
of these two things must happen:

 (1) --valgrind and --vasomething are equally useful and often used.
     Neither will get --va and either --val or --vas needs to be
     given.

 (2) Because we documented --va as --valgrind, people feel that they
     are entitled to expect --va will stay forever to be a shorthand
     for --valgrind and nothing else.  The shortened forms will be
     between --va (or longer prefix of --valgrind) and --vas (or
     longer prefix of --vasomething).

We would rather want to see (1), as people new to the system do not
have to learn that --valgrind can be spelled --va merely by being
the first to appear, and --vasomething must be spelled --vas because
it happened to come later.  Longer term, nobody should care how the
system evolved into the current shape, but (2) will require that to
understand and remember why one is --va and the other has to be --vas.

We already have this suboptimal (2) situation between "--valgrind"
and "--verbose" options, but a shorter form "v" that is used for
"verbose" is so widely understood and used that I think it is an
acceptable exception.  So

         --verbose::
        +-v::
                Give verbose output from the test

is OK, but "--valgrind can be shortened to --va" is not.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-25 17:23       ` Junio C Hamano
@ 2014-03-27  9:39         ` Ilya Bobyr
  2014-03-27 16:35           ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27  9:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ramsay Jones, git, Thomas Rast, Eric Sunshine

On 3/25/2014 10:23 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> On 3/24/2014 4:39 AM, Ramsay Jones wrote:
>>> On 24/03/14 08:49, Ilya Bobyr wrote:
>>> [...]
>>>> [...]
>>>>  
>>>> ---valgrind=<tool>::
>>>> +-v,--valgrind=<tool>::
>>> The -v short option is taken, above ... :-P
>> Right %)
>> Thanks :)
>> This one starts only with "--va", will fix it.
> Please don't.
>
> In general, when option names can be shortened by taking a unique
> prefix, it is better not to give short form in the documentation at
> all.  There is no guarantee that the short form you happen to pick
> when you document it will continue to be unique forever.  When we
> add another --vasomething option, --va will become ambiguous and one
> of these two things must happen:
>
>  (1) --valgrind and --vasomething are equally useful and often used.
>      Neither will get --va and either --val or --vas needs to be
>      given.
>
>  (2) Because we documented --va as --valgrind, people feel that they
>      are entitled to expect --va will stay forever to be a shorthand
>      for --valgrind and nothing else.  The shortened forms will be
>      between --va (or longer prefix of --valgrind) and --vas (or
>      longer prefix of --vasomething).
>
> We would rather want to see (1), as people new to the system do not
> have to learn that --valgrind can be spelled --va merely by being
> the first to appear, and --vasomething must be spelled --vas because
> it happened to come later.  Longer term, nobody should care how the
> system evolved into the current shape, but (2) will require that to
> understand and remember why one is --va and the other has to be --vas.
>
> We already have this suboptimal (2) situation between "--valgrind"
> and "--verbose" options, but a shorter form "v" that is used for
> "verbose" is so widely understood and used that I think it is an
> acceptable exception.  So
>
>          --verbose::
>         +-v::
>                 Give verbose output from the test
>
> is OK, but "--valgrind can be shortened to --va" is not.

Sure, this is exactly what I meant, but I guess, I was too short
so it created ambiguity =)
I was going to just remove the '-v' from '--valgrind'.

Shortening is a separate issue.  I did not look at it.
I can see that it is also not documented.   At the same time
shortening is entirely consistent at the moment, and does not
work for options that take arguments.

My main intent was to document '-r' :)
As no other short form were documented, I had to fix that issue
first.

If there is decision on how shortening should work for all the
options, maybe I could add a paragraph on that and make existing
options more consistent.

I guess the questions would be, should it possible to use short
forms for options that take arguments?

If so, '--valgrind' becomes impossible to shorten because there
is '--valgrind-only' that is a separate option.  Same for
'--verbose'  and '--verbose-only'.

For convenience here is the relevant switch in the way it is
right now:

    case "$1" in
    -d|--d|--de|--deb|--debu|--debug)
        debug=t; shift ;;
   
-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
        immediate=t; shift ;;
   
-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
        GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
    -r)
        shift; test "$#" -ne 0 || {
            echo 'error: -r requires an argument' >&2;
            exit 1;
        }
        run_list=$1; shift ;;
    --run=*)
        run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
    -h|--h|--he|--hel|--help)
        help=t; shift ;;
    -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
        verbose=t; shift ;;
    --verbose-only=*)
        verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    -q|--q|--qu|--qui|--quie|--quiet)
        # Ignore --quiet under a TAP::Harness. Saying how many tests
        # passed without the ok/not ok details is always an error.
        test -z "$HARNESS_ACTIVE" && quiet=t; shift ;;
    --with-dashes)
        with_dashes=t; shift ;;
    --no-color)
        color=; shift ;;
    --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
        valgrind=memcheck
        shift ;;
    --valgrind=*)
        valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    --valgrind-only=*)
        valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    --tee)
        shift ;; # was handled already
    --root=*)
        root=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    *)
        echo "error: unknown test option '$1'" >&2; exit 1 ;;
    esac


P.S.  Sorry it takes me this long to reply.  I will try to be
more responsive, should there will be a discussion :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH] Better control of the tests run by a test suite
  2014-03-25  4:58   ` Junio C Hamano
@ 2014-03-27 10:15     ` Ilya Bobyr
  0 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27 10:15 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King
  Cc: git, Thomas Rast, Eric Sunshine, Jonathan Nieder

On 3/24/2014 9:58 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:
>>
>>> Here are some examples of how functionality added by the patch
>>> could be used.  In order to run setup tests and then only a
>>> specific test (use case 1) one can do:
>>>
>>>     $ ./t0000-init.sh --run='1 2 25'
>>>
>>> or:
>>>
>>>     $ ./t0000-init.sh --run='<3 25'
>>>
>>> ('<=' is also supported, as well as '>' and '>=').
>> I don't have anything against this in principle, but I suspect it will
>> end up being a big pain to figure out which of the early tests are
>> required to set up the state, and which are not. Having "<" makes
>> specifying it easier, but you still have to read the test script to
>> figure out which tests need to be run.
> Likewise.

The idea is that you will use that option when you know what
setup the test need.  And the case that I was targeting is when
you are the author of the test, because you are also writing the
relevant functionality or you are really familiar with the test
because you are, again, working on something in that area.

It does not mean you actually have to do it.  It is just a
possibility.

And as you mentioned, "<" helps in another case - when you do not
know enough about the test, but want to run it.  For example when
you are just starting with a failed test.

My experience, thought quite limited, is that it is very simple
to understand what the test needs and where it is prepared, if
you are actually adding new test to a test suite.  Or if you
spent some time figuring how specific test works.  I think this
is mostly because all the tests are rather simple.  Which is
definitely a good thing.

This is not for cases when you treat test suites as black boxes.
For example, when you are just checking someone else code.

>> I wonder if it would make sense to "auto-select" tests that match a
>> regex like "set.?up|create"? A while ago, Jonathan made a claim that
>> this would cover most tests that are dependencies of other tests. I did
>> not believe him, but looking into it, I recall that we did seem to have
>> quite a few matching that pattern. If there were a good feature like
>> this that gave us a reason to follow that pattern, I think people might
>> fix the remainder
> This may be worth experimenting with, I would think.

I was also thinking about it a bit.  I do not have that much
knowledge on how all the tests are organized.  But I did see some
cases where this rule would fail.

One example would be "t\t0000-basic.sh".  It could probably be
considered a very special test suite, but if you skip one of
these tests:

 - "test runs if prerequisite is satisfied"
 - "unmet prerequisite causes test to be skipped"

the test suite would just exit in the middle.  There is a number
of other tests that you do not want to skip for the same reason.
Also, in the same test suite "showing tree with git ls-tree -r"
is a setup test for the next one "git ls-tree -r output for a
known tree".  And the same pattern is repeated for some other
tests.

I've also looked at "t5601-clone.sh".  There is indeed a test
called "setup" at the very beginning.  But somewhere in the
middle there is a test called "clone from .git file" that creates
a folder used in two subsequent tests.

In "t0001-init.sh", "re-init on .git file" creates a folder that
is used in the next test called "re-init to update git link".

Maybe these are just some outliers, I do not know for sure.
These were the only test suites I've looked at so far.

I think that if there is a desire to support automatic setup for
tests maybe a rule could be introduced that a test must succeed,
if there is no breakage, if all the tests that match regex
'^(setup|cleanup)\>' before it have been run.  It should not be
too complicated to create a target that would automate checking
of this rule.

I am not 100% sure that this kind of change is worth the trouble.
People who run individual tests should probably know why they are
doing it.  And as such that might know the prerequisites.

Otherwise I can not come up with a reason to run an individual
test.

On the other hand, the rule may add a bit more structure to the
tests and automated checking could enforce that.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC/PATCH v2] Better control of the tests run by a test suite
  2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
                   ` (3 preceding siblings ...)
  2014-03-24 23:03 ` [RFC/PATCH] Better control of the tests run by a test suite Jeff King
@ 2014-03-27 10:32 ` Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
                     ` (2 more replies)
  4 siblings, 3 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27 10:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones

This is an update verson of the patches I've posted here:

    [RFC/PATCH] Better control of the tests run by a test suite
    http://www.mail-archive.com/git@vger.kernel.org/msg46419.html

Chanes are only in the first patch, according to

    http://www.mail-archive.com/git@vger.kernel.org/msg46423.html
    Ramsay Jones

and

    http://www.mail-archive.com/git@vger.kernel.org/msg46512.html
    Eric Sunshine

The description below is identical to the previous one, but here
it is for convenience if someone would want to quote it in a
comment.

---

Hello,

This is a second attempt on a functionality I proposed in

    [PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
    http://www.mail-archive.com/git%40vger.kernel.org/msg44828.html

except that the implementation is quite different now.

I hope that I have accounted for the comments that were voiced so
far.  Let's see 

The idea behind the change is that sometimes it is convenient to
run only certain tests from a test suite.  Specifically I am
thinking about the following two use cases:

 1. You are developing new functionality.  You add a test that
    fails and then you add and modify code to make it pass.
    
 2. You have a failed test and you need to understand what is
    wrong.
    
In the first case you when you run the test suite, you probably
want to run some setup tests and then only one test that you are
focused on.

For code written in C time between you make a change and see a
test result is considerably increased by the compilation.  But
for script code turn around time is mostly due to the run time of
the test suite itself. [1]

For the second case you actually want the test suite to stop
after the failing test, so that you can examine the trash
directory without any modifications from the subsequent tests.
You probably do not care about them.

In the previous patch I've added an environment variable to
control tests to be run in a test suite.  I thought that it would
be similar to an already existing GIT_SKIP_TESTS.  As I did not
provide a cover letter that caused some misunderstanding I think.

This patch adds new command line argument '--run' that accepts a
list of patterns and restrictions on the test numbers that would
be included or excluded from this run of the test suite.

During discussion of the previous patch there were some talks
about extending GIT_SKIP_TESTS syntax.  In particular here:

> That is
>
>         GIT_SKIP_TESTS='t9??? !t91??'
>
> would skip nine-thousand series, but would run 91xx series, and all
> the others are not excluded.
>
> Simple rules to consider:
>
>  - If the list consists of _only_ negated patterns, pretend that
>    there is "unless otherwise specified with negatives, skip all
>    tests", i.e. treat GIT_SKIP_TESTS='!t91??' just the same way you
>    would treat GIT_SKIP_TESTS='* !t91??'.
>
>  - The orders should not matter for simplicity of the semantics;
>    before running each test, check if it matches any negative (and
>    run it if it matches, without looking at any positives), and
>    otherwise check if it matches any positive (and skip it if it
>    does not).
>
> Hmm?

    http://www.mail-archive.com/git%40vger.kernel.org/msg44922.html


I've used that as a basis, but the end result is somewhat
different.  Plus I've added boundary checks as in "<123".

Here are some examples of how functionality added by the patch
could be used.  In order to run setup tests and then only a
specific test (use case 1) one can do:

    $ ./t0000-init.sh --run='1 2 25'

or:

    $ ./t0000-init.sh --run='<3 25'

('<=' is also supported, as well as '>' and '>=').

In order to run up to a specific test (use case 2) one can do:

    $ ./t0000-init.sh --run='<=34'

or:

    $ ./t0000-init.sh --run='!>34'

Simple semantics described above is easy to implement, but at the
same time is probably not that convenient.  Rules implemented by
the patch are as follows:

 - Order does matter.  Whatever is specified on the right has
   higher precedence.

 - When the first pattern is positive the initial set of the
   tests to be run is empty.  You are adding to an empty set as
   in '1 2 25'.

   When the first pattern is negative the initial set of the
   tests to run contains all the tests.  You are subtracting
   from that set as in '!>34'.

It seems that for simple cases that gives simple syntax and is
almost unbiased if you think about preferring inclusion over
exclusion.  While it is unlikely it also allows for complicated
expressions.  And the implementation is quite simple as well.

Main functionality is in the third patch.  First two are just
minor fixes in related parts of the code.

P.S. I did not reply to the previous patch thread as this one is
quite different.


[1] Here are some times I see on Cygin:

    $ touch builtin/rev-parse.c
    
    $ time make
    ...
    
    real    0m10.382s
    user    0m3.829s
    sys     0m5.269s

Running the t0000-init.sh test suite is like this:

    $ time ./t0001-init.sh
    [...]
    1..36

    real    0m6.693s
    user    0m1.505s
    sys     0m3.937s

If I run only the 1, 2, 4 and 5th tests, it only half the time to
run the tests:

    $ time GIT_SKIP_TESTS='t0001.[36789] t0001.??' ./t0001-init.sh
    [...]
    1..36

    real    0m3.313s
    user    0m0.769s
    sys     0m1.844s 

Overall the change is from 17 to 14 seconds it is not that big.

If you only consider the test suite, as you do while you develop
an sh based tool, for example, the change is from 6.6 to 3.3
seconds.  That is quite noticeable.


 t/README         |   73 ++++++++++++--
 t/t0000-basic.sh |  296 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   96 +++++++++++++++++-
 3 files changed, 453 insertions(+), 12 deletions(-)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
@ 2014-03-27 10:32   ` Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
  2 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27 10:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones, Ilya Bobyr

Most arguments that could be provided to a test have short forms.
Unless documented, the only way to learn them is to read the code.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 Minor changes according to comments in

    http://www.mail-archive.com/git@vger.kernel.org/msg46423.html
    Ramsay Jones

 and

    http://www.mail-archive.com/git@vger.kernel.org/msg46512.html
    Eric Sunshine

 t/README |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index caeeb9d..6b93aca 100644
--- a/t/README
+++ b/t/README
@@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
 (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
 appropriately before running "make".
 
---verbose::
+-v,--verbose::
 	This makes the test more verbose.  Specifically, the
 	command being run and their output if any are also
 	output.
@@ -81,7 +81,7 @@ appropriately before running "make".
 	numbers matching <pattern>.  The number matched against is
 	simply the running count of the test within the file.
 
---debug::
+-d,--debug::
 	This may help the person who is developing a new test.
 	It causes the command defined with test_debug to run.
 	The "trash" directory (used to store all temporary data
@@ -89,14 +89,14 @@ appropriately before running "make".
 	failed tests so that you can inspect its contents after
 	the test finished.
 
---immediate::
+-i,--immediate::
 	This causes the test to immediately exit upon the first
 	failed test. Cleanup commands requested with
 	test_when_finished are not executed if the test failed,
 	in order to keep the state for inspection by the tester
 	to diagnose the bug.
 
---long-tests::
+-l,--long-tests::
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so
  2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
@ 2014-03-27 10:32   ` Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
  2 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27 10:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones, Ilya Bobyr

We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS.  Use "(GIT_SKIP_TESTS)" instead.

Plus tests that check basic GIT_SKIP_TESTS functions.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 No changes from the previous version.

 t/t0000-basic.sh |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   13 ++++++----
 2 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index a2bb63c..ae8874e 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -270,6 +270,69 @@ test_expect_success 'test --verbose-only' '
 	EOF
 '
 
+test_expect_success 'GIT_SKIP_TESTS' "
+	GIT_SKIP_TESTS='git.2' \
+		run_sub_test_lib_test git-skip-tests-basic \
+		'GIT_SKIP_TESTS' <<-\\EOF &&
+	for i in 1 2 3
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 - passing test #3
+	> # passed all 3 test(s)
+	> 1..3
+	EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS several tests' "
+	GIT_SKIP_TESTS='git.2 git.5' \
+		run_sub_test_lib_test git-skip-tests-several \
+		'GIT_SKIP_TESTS several tests' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-several <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS sh pattern' "
+	GIT_SKIP_TESTS='git.[2-5]' \
+		run_sub_test_lib_test git-skip-tests-sh-pattern \
+		'GIT_SKIP_TESTS sh pattern' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test git-skip-tests-sh-pattern <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+	> ok 3 # skip passing test #3 (GIT_SKIP_TESTS)
+	> ok 4 # skip passing test #4 (GIT_SKIP_TESTS)
+	> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 569b52d..e035f36 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -452,25 +452,28 @@ test_finish_ () {
 
 test_skip () {
 	to_skip=
+	skipped_reason=
 	if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
 	then
 		to_skip=t
+		skipped_reason="GIT_SKIP_TESTS"
 	fi
 	if test -z "$to_skip" && test -n "$test_prereq" &&
 	   ! test_have_prereq "$test_prereq"
 	then
 		to_skip=t
-	fi
-	case "$to_skip" in
-	t)
+
 		of_prereq=
 		if test "$missing_prereq" != "$test_prereq"
 		then
 			of_prereq=" of $test_prereq"
 		fi
-
+		skipped_reason="missing $missing_prereq${of_prereq}"
+	fi
+	case "$to_skip" in
+	t)
 		say_color skip >&3 "skipping test: $@"
-		say_color skip "ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
+		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
 		;;
 	*)
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
  2014-03-27 10:32   ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
@ 2014-03-27 10:32   ` Ilya Bobyr
  2014-03-28  3:36     ` Eric Sunshine
  2 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-27 10:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones, Ilya Bobyr

Allow better control of the set of tests that will be executed for a
single test suite.  Mostly useful while debugging or developing as it
allows to focus on a specific test.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 No changes from the previous version.

 t/README         |   65 ++++++++++++++-
 t/t0000-basic.sh |  233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh    |   85 ++++++++++++++++++++
 3 files changed, 379 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index 6b93aca..c911f89 100644
--- a/t/README
+++ b/t/README
@@ -100,6 +100,10 @@ appropriately before running "make".
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+-r,--run=<test numbers>::
+	This causes only specific tests to be included or excluded.  See
+	section "Skipping Tests" below for "<test numbers>" syntax.
+
 --valgrind=<tool>::
 	Execute all Git binaries under valgrind tool <tool> and exit
 	with status 126 on errors (just like regular tests, this will
@@ -187,10 +191,63 @@ and either can match the "t[0-9]{4}" part to skip the whole
 test, or t[0-9]{4} followed by ".$number" to say which
 particular test to skip.
 
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+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.
+
+--run argument is a list of patterns with optional prefixes that
+are matched against test numbers within the current test suite.
+Supported pattern:
+
+ - A number matches a test with that number.
+
+ - sh metacharacters such as '*', '?' and '[]' match as usual in
+   shell.
+
+ - A number prefixed with '<', '<=', '>', or '>=' matches all
+   tests 'before', 'before or including', 'after', or 'after or
+   including' the specified one.
+
+Optional prefixes are:
+
+ - '+' or no prefix: test(s) matching the pattern are included in
+   the run.
+
+ - '-' or '!': test(s) matching the pattern are exluded from the
+   run.
+
+If --run starts with '+' or unprefixed pattern the initial set of
+tests to run is empty. If the first pattern starts with '-' or
+'!' all the tests are added to the initial set.  After initial
+set is determined every pattern, test number or range is added or
+excluded from the set one by one, from left to right.
+
+For example, common case is to run several setup tests and then a
+specific test that relies on that setup:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='<4 21'
+
+To run only tests up to a specific test one could do this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!>=21'
+
+As noted above test set is build going though patterns left to
+right, so this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='<5 !3'
+
+will run tests 1, 2, and 4.
+
+Some tests in the existing test suite rely on previous test item,
+so you cannot arbitrarily disable one and expect the remainder of
+test to check what the test originally was intended to check.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing.  Or when you want to run up to a
+certain test.
 
 
 Naming Tests
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae8874e..4da527f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -333,6 +333,239 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
 	EOF
 "
 
+test_expect_success '--run basic' "
+	run_sub_test_lib_test run-basic \
+		'--run basic' --run='1 3 5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with <' "
+	run_sub_test_lib_test run-lt \
+		'--run with <' --run='<4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-lt <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with <=' "
+	run_sub_test_lib_test run-le \
+		'--run with <=' --run='<=4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-le <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with >' "
+	run_sub_test_lib_test run-gt \
+		'--run with >' --run='>4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-gt <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with >=' "
+	run_sub_test_lib_test run-ge \
+		'--run with >=' --run='>=4' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-ge <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with basic negation' "
+	run_sub_test_lib_test run-basic-neg \
+		'--run with basic negation' --run='-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with two negations' "
+	run_sub_test_lib_test run-two-neg \
+		'--run with two negation' --run='"'!3 !6'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run < and negation' "
+	run_sub_test_lib_test run-lt-neg \
+		'--run < and negation' --run='"'<5 !2'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-lt-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run range negation' "
+	run_sub_test_lib_test run-range-neg \
+		'--run range negation' --run='-<3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-neg <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run include, exclude and include' "
+	run_sub_test_lib_test run-inc-neg-inc \
+		'--run include, exclude and include' \
+		--run='"'<=5 !<3 1'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+test_expect_success '--run exclude and include' "
+	run_sub_test_lib_test run-neg-inc \
+		'--run exclude and include' \
+		--run='"'!>2 5'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e035f36..63e481a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -191,6 +191,14 @@ do
 		immediate=t; shift ;;
 	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
 		GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
+	-r)
+		shift; test "$#" -ne 0 || {
+			echo 'error: -r requires an argument' >&2;
+			exit 1;
+		}
+		run_list=$1; shift ;;
+	--run=*)
+		run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
 	-h|--h|--he|--hel|--help)
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
@@ -366,6 +374,76 @@ match_pattern_list () {
 	return 1
 }
 
+match_run_pattern_list () {
+	arg="$1"
+	shift
+	test -z "$*" && return 0
+
+	# If the first patern is negative we include by default.
+	include=
+	case "$1" in
+		[-!]*) include=t ;;
+	esac
+
+	for pattern_
+	do
+		orig_pattern=$pattern_
+
+		positive=t
+		case "$pattern_" in
+			[-!]*)
+				positive=
+				pattern_=${pattern_##?}
+				;;
+		esac
+
+		# Short cut for "obvious" cases
+		[ "x$include" = "x" -a "x$positive" = "x" ] && continue
+		[ "x$include" = "xt" -a "x$positive" = "xt" ] && continue
+
+		pattern_op=
+		case "$pattern_" in
+			\<=*)
+				pattern_op='-le'
+				pattern_=${pattern_##??}
+				;;
+			\<*)
+				pattern_op='-lt'
+				pattern_=${pattern_##?}
+				;;
+			\>=*)
+				pattern_op='-ge'
+				pattern_=${pattern_##??}
+				;;
+			\>*)
+				pattern_op='-gt'
+				pattern_=${pattern_##?}
+				;;
+		esac
+
+		if test -n "$pattern_op"
+		then
+			if expr "z$pattern_" : "z[0-9]*[^0-9]" >/dev/null
+			then
+				echo "error: --run: test number contains" \
+					"non-digits: '$orig_pattern'" >&2
+				exit 1
+			fi
+			if test $arg $pattern_op $pattern_
+			then
+				include=$positive
+			fi
+		else
+			case "$arg" in
+				$pattern_)
+					include=$positive
+			esac
+		fi
+	done
+
+	test -n "$include"
+}
+
 maybe_teardown_verbose () {
 	test -z "$verbose_only" && return
 	exec 4>/dev/null 3>/dev/null
@@ -470,6 +548,13 @@ test_skip () {
 		fi
 		skipped_reason="missing $missing_prereq${of_prereq}"
 	fi
+	if test -z "$to_skip" && test -n "$run_list" &&
+		! match_run_pattern_list $test_count $run_list
+	then
+		to_skip=t
+		skipped_reason="--run"
+	fi
+
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-27  9:39         ` Ilya Bobyr
@ 2014-03-27 16:35           ` Junio C Hamano
  2014-03-28 17:20             ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-03-27 16:35 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Ramsay Jones, git, Thomas Rast, Eric Sunshine

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> If there is decision on how shortening should work for all the
> options, maybe I could add a paragraph on that and make existing
> options more consistent.

We should strive to make the following from gitcli.txt apply
throughout the system:

 * many commands allow a long option `--option` to be abbreviated
   only to their unique prefix (e.g. if there is no other option
   whose name begins with `opt`, you may be able to spell `--opt` to
   invoke the `--option` flag), but you should fully spell them out
   when writing your scripts; later versions of Git may introduce a
   new option whose name shares the same prefix, e.g. `--optimize`,
   to make a short prefix that used to be unique no longer unique.

> If so, '--valgrind' becomes impossible to shorten because there
> is '--valgrind-only' that is a separate option.  Same for
> '--verbose'  and '--verbose-only'.

Correct.  If you really cared, --valgrind={yes,no,only} would be (or
have been) a better possibility, though.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-27 10:32   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
@ 2014-03-28  3:36     ` Eric Sunshine
  2014-03-28  7:05       ` Ilya Bobyr
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Sunshine @ 2014-03-28  3:36 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Git List, Junio C Hamano, Thomas Rast, Ramsay Jones

On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
> Allow better control of the set of tests that will be executed for a
> single test suite.  Mostly useful while debugging or developing as it
> allows to focus on a specific test.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  No changes from the previous version.
>
>  t/README         |   65 ++++++++++++++-
>  t/t0000-basic.sh |  233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  t/test-lib.sh    |   85 ++++++++++++++++++++
>  3 files changed, 379 insertions(+), 4 deletions(-)
>
> diff --git a/t/README b/t/README
> index 6b93aca..c911f89 100644
> --- a/t/README
> +++ b/t/README
> @@ -100,6 +100,10 @@ appropriately before running "make".
>         This causes additional long-running tests to be run (where
>         available), for more exhaustive testing.
>
> +-r,--run=<test numbers>::

Perhaps <test-selection> or something similar would be closer to the truth.

> +       This causes only specific tests to be included or excluded.  See

This is phrased somewhat oddly, as if you had already been talking
about tests being included or excluded, and that this option merely
changes that selection. Perhaps something like:

    Run only the subset of tests indicated by <test-selection>.

> +       section "Skipping Tests" below for "<test numbers>" syntax.
> +
>  --valgrind=<tool>::
>         Execute all Git binaries under valgrind tool <tool> and exit
>         with status 126 on errors (just like regular tests, this will
> @@ -187,10 +191,63 @@ and either can match the "t[0-9]{4}" part to skip the whole
>  test, or t[0-9]{4} followed by ".$number" to say which
>  particular test to skip.
>
> -Note that some tests in the existing test suite rely on previous
> -test item, so you cannot arbitrarily disable one and expect the
> -remainder of test to check what the test originally was intended
> -to check.
> +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.
> +
> +--run argument is a list of patterns with optional prefixes that

"The argument for --run is a list...

> +are matched against test numbers within the current test suite.
> +Supported pattern:
> +
> + - A number matches a test with that number.
> +
> + - sh metacharacters such as '*', '?' and '[]' match as usual in
> +   shell.
> +
> + - A number prefixed with '<', '<=', '>', or '>=' matches all
> +   tests 'before', 'before or including', 'after', or 'after or
> +   including' the specified one.

I think you want "and" rather than "or": "before and including",
"after and including".

> +Optional prefixes are:
> +
> + - '+' or no prefix: test(s) matching the pattern are included in
> +   the run.
> +
> + - '-' or '!': test(s) matching the pattern are exluded from the
> +   run.

I've been playing with --run, and I find that test selection is not
especially intuitive. For instance, ">=16 !>24 !20" is easier to
reason about when written instead with ranges, such as "16-19 21-24",
or perhaps "16-24 !20". Open-ended ranges make sense too: "5-" means
tests 5 through the last, and "-5" means tests 1 through 5. (Yes, this
conflicts with your use of '-' to mean negation, but you already have
the perfectly serviceable '!' as an alias for negation.)

> +If --run starts with '+' or unprefixed pattern the initial set of
> +tests to run is empty. If the first pattern starts with '-' or
> +'!' all the tests are added to the initial set.  After initial
> +set is determined every pattern, test number or range is added or
> +excluded from the set one by one, from left to right.
> +
> +For example, common case is to run several setup tests and then a
> +specific test that relies on that setup:

Perhaps be a bit more specific:

    ...run several setup tests (1, 2, 3) and then a
    specific test (21) that relies...

> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
> +
> +or:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<4 21'

It might be clearer to say "<=3" rather than "<4".

> +To run only tests up to a specific test one could do this:

s/specific test/specific test,/

Also perhaps:

    ...up to a specific test (21), one...

> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!>=21'
> +
> +As noted above test set is build going though patterns left to

s/above/above,/
s/test set/the test set/
s/build/built/

    As noted above, the test set is built...

> +right, so this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<5 !3'
> +
> +will run tests 1, 2, and 4.
> +
> +Some tests in the existing test suite rely on previous test item,
> +so you cannot arbitrarily disable one and expect the remainder of
> +test to check what the test originally was intended to check.
> +--run is mostly useful when you want to focus on a specific test
> +and know what you are doing.  Or when you want to run up to a
> +certain test.
>
>
>  Naming Tests
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index e035f36..63e481a 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -191,6 +191,14 @@ do
>                 immediate=t; shift ;;
>         -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
>                 GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
> +       -r)
> +               shift; test "$#" -ne 0 || {
> +                       echo 'error: -r requires an argument' >&2;
> +                       exit 1;
> +               }
> +               run_list=$1; shift ;;
> +       --run=*)
> +               run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
>         -h|--h|--he|--hel|--help)
>                 help=t; shift ;;
>         -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
> @@ -366,6 +374,76 @@ match_pattern_list () {
>         return 1
>  }
>
> +match_run_pattern_list () {
> +       arg="$1"
> +       shift
> +       test -z "$*" && return 0
> +
> +       # If the first patern is negative we include by default.

s/patern/pattern/

> +       include=
> +       case "$1" in
> +               [-!]*) include=t ;;
> +       esac
> +
> +       for pattern_
> +       do
> +               orig_pattern=$pattern_
> +
> +               positive=t
> +               case "$pattern_" in
> +                       [-!]*)
> +                               positive=
> +                               pattern_=${pattern_##?}
> +                               ;;
> +               esac
> +
> +               # Short cut for "obvious" cases
> +               [ "x$include" = "x" -a "x$positive" = "x" ] && continue

Although there are a few exceptions in this script, 'test' is
generally preferred over '['. Also, -a doesn't have great portability,
so && may be better.

    test -z "$include" && test -z "$positive" && continue

> +               [ "x$include" = "xt" -a "x$positive" = "xt" ] && continue

Since you're inside double quotes, you can drop the 'x' prefix:

    test "$include" = t && test "$positive" = t && continue

> +               pattern_op=
> +               case "$pattern_" in
> +                       \<=*)
> +                               pattern_op='-le'
> +                               pattern_=${pattern_##??}
> +                               ;;
> +                       \<*)
> +                               pattern_op='-lt'
> +                               pattern_=${pattern_##?}
> +                               ;;
> +                       \>=*)
> +                               pattern_op='-ge'
> +                               pattern_=${pattern_##??}
> +                               ;;
> +                       \>*)
> +                               pattern_op='-gt'
> +                               pattern_=${pattern_##?}
> +                               ;;
> +               esac
> +
> +               if test -n "$pattern_op"
> +               then
> +                       if expr "z$pattern_" : "z[0-9]*[^0-9]" >/dev/null

Inside double quotes: you can drop the 'z' prefix.

> +                       then
> +                               echo "error: --run: test number contains" \
> +                                       "non-digits: '$orig_pattern'" >&2
> +                               exit 1
> +                       fi
> +                       if test $arg $pattern_op $pattern_
> +                       then
> +                               include=$positive
> +                       fi
> +               else
> +                       case "$arg" in
> +                               $pattern_)
> +                                       include=$positive
> +                       esac
> +               fi
> +       done
> +
> +       test -n "$include"
> +}
> +
>  maybe_teardown_verbose () {
>         test -z "$verbose_only" && return
>         exec 4>/dev/null 3>/dev/null
> @@ -470,6 +548,13 @@ test_skip () {
>                 fi
>                 skipped_reason="missing $missing_prereq${of_prereq}"
>         fi
> +       if test -z "$to_skip" && test -n "$run_list" &&
> +               ! match_run_pattern_list $test_count $run_list
> +       then
> +               to_skip=t
> +               skipped_reason="--run"

A few pure bike-shedding comments (to be ignore if desired):

I still don't understand the need to distinguish between a test
skipped due to --run and and one skipped due to GIT_SKIP_TESTS.

The skip-reason "GIT_SKIP_TESTS" (in patch 2/3) still seems
unnecessarily verbose and loud.

The skip-reason "excluded" (suggested in an earlier review) is short
and sweet, and equally applicable to a test skipped either via --run
or GIT_SKIP_TESTS.

> +       fi
> +
>         case "$to_skip" in
>         t)
>                 say_color skip >&3 "skipping test: $@"
> --
> 1.7.9
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-28  3:36     ` Eric Sunshine
@ 2014-03-28  7:05       ` Ilya Bobyr
  2014-03-30  9:41         ` Eric Sunshine
  0 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-03-28  7:05 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano, Thomas Rast, Ramsay Jones

On 3/27/2014 8:36 PM, Eric Sunshine wrote:
> On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
>> Allow better control of the set of tests that will be executed for a
>> single test suite.  Mostly useful while debugging or developing as it
>> allows to focus on a specific test.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>>  No changes from the previous version.
>>
>>  t/README         |   65 ++++++++++++++-
>>  t/t0000-basic.sh |  233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  t/test-lib.sh    |   85 ++++++++++++++++++++
>>  3 files changed, 379 insertions(+), 4 deletions(-)
>>
>> diff --git a/t/README b/t/README
>> index 6b93aca..c911f89 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -100,6 +100,10 @@ appropriately before running "make".
>>         This causes additional long-running tests to be run (where
>>         available), for more exhaustive testing.
>>
>> +-r,--run=<test numbers>::
> Perhaps <test-selection> or something similar would be closer to the truth.

I think your naming is better.  I will include it in the next version.

>> +       This causes only specific tests to be included or excluded.  See
> This is phrased somewhat oddly, as if you had already been talking
> about tests being included or excluded, and that this option merely
> changes that selection. Perhaps something like:
>
>     Run only the subset of tests indicated by <test-selection>.

Will use that sentence as well :)

>> +       section "Skipping Tests" below for "<test numbers>" syntax.
>> +
>>  --valgrind=<tool>::
>>         Execute all Git binaries under valgrind tool <tool> and exit
>>         with status 126 on errors (just like regular tests, this will
>> @@ -187,10 +191,63 @@ and either can match the "t[0-9]{4}" part to skip the whole
>>  test, or t[0-9]{4} followed by ".$number" to say which
>>  particular test to skip.
>>
>> -Note that some tests in the existing test suite rely on previous
>> -test item, so you cannot arbitrarily disable one and expect the
>> -remainder of test to check what the test originally was intended
>> -to check.
>> +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.
>> +
>> +--run argument is a list of patterns with optional prefixes that
> "The argument for --run is a list...

I think it could be either.  But I am not a native speaker.
So, I will use your version :)

>> +are matched against test numbers within the current test suite.
>> +Supported pattern:
>> +
>> + - A number matches a test with that number.
>> +
>> + - sh metacharacters such as '*', '?' and '[]' match as usual in
>> +   shell.
>> +
>> + - A number prefixed with '<', '<=', '>', or '>=' matches all
>> +   tests 'before', 'before or including', 'after', or 'after or
>> +   including' the specified one.
> I think you want "and" rather than "or": "before and including",
> "after and including".

I was thinking about an analogy to the corresponding mathematical
operations here.  In mathematics, '<=' is called "less than or
equal to"[1].

If you are thinking about test numbers you can say that you
include a test if it has a number before or equal to the given
one.  The sentence is "A number prefixed with <= matches all
tests [with numbers] before or including the specified [number]."

Maybe if I change "one" to "number" it would be a bit less
ambiguous.  Or even include all the omitted words.

I would not mind a completely different way to say it, but I am
not yet sure that if I replace "or" with "and" it would make it
a lot better.

[1] https://en.wikipedia.org/wiki/Inequality_%28mathematics%29

>> +Optional prefixes are:
>> +
>> + - '+' or no prefix: test(s) matching the pattern are included in
>> +   the run.
>> +
>> + - '-' or '!': test(s) matching the pattern are exluded from the
>> +   run.
> I've been playing with --run, and I find that test selection is not
> especially intuitive. For instance, ">=16 !>24 !20" is easier to
> reason about when written instead with ranges, such as "16-19 21-24",
> or perhaps "16-24 !20". Open-ended ranges make sense too: "5-" means
> tests 5 through the last, and "-5" means tests 1 through 5. (Yes, this
> conflicts with your use of '-' to mean negation, but you already have
> the perfectly serviceable '!' as an alias for negation.)

I completely agree that ranges allow one to express certain
"obvious" things much easier than just inequalities.  I was even
thinking on a possible syntax.  But then I realized that I do not
have a real use case for it.

The only use case that I had is described in the cover letter: to
run several setup tests and then the target test.  For that even
simple lists were enough and I was using that original version
with an environment variable.  After a conversation on the list I
thought that it would be nice to be able to say '<', as it would
save typing several extra characters for cases like '1 2 3 4 25'.
While test suits that I've seen so far actually have no more than
two tests that do the setup.

The next use case that I could come up with was running up to a
specific test.  I was in a situation were that would have been a
nice option.  And inequalities allow that.

Once again, I do not have a use case where I would need to run
tests from 1 to 10, then 14 to 19 and then 100 and up to the end.

Do you have something on your mind where that would be useful?

As for the syntax, ! is replaced in bash by the last executed
command.  That happens inside double quotes as well and the
original command line is not preserved (at lest for me).  So if
it would be the only option that seemed like a limitation.  It is
possible to escape it or use single quotes, of course.  I was
thinking about a comma as a separator for ranges.

As for the open ended ranges - they are the same as inequalities.

>> +If --run starts with '+' or unprefixed pattern the initial set of
>> +tests to run is empty. If the first pattern starts with '-' or
>> +'!' all the tests are added to the initial set.  After initial
>> +set is determined every pattern, test number or range is added or
>> +excluded from the set one by one, from left to right.
>> +
>> +For example, common case is to run several setup tests and then a
>> +specific test that relies on that setup:
> Perhaps be a bit more specific:
>
>     ...run several setup tests (1, 2, 3) and then a
>     specific test (21) that relies...

Good idea, though it clutters the sentence a bit.
Will use it.

>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
>> +
>> +or:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<4 21'
> It might be clearer to say "<=3" rather than "<4".

I thought that < is less typing, so it should be the first one to show.
Non-strict inequalities were next.
I will change this example and show '<' and '>' in the next two.

>> +To run only tests up to a specific test one could do this:
> s/specific test/specific test,/
>
> Also perhaps:
>
>     ...up to a specific test (21), one...

Fixed.

>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!>=21'
>> +
>> +As noted above test set is build going though patterns left to
> s/above/above,/
> s/test set/the test set/
> s/build/built/
>
>     As noted above, the test set is built...

Thanks :)

>> +right, so this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<5 !3'
>> +
>> +will run tests 1, 2, and 4.
>> +
>> +Some tests in the existing test suite rely on previous test item,
>> +so you cannot arbitrarily disable one and expect the remainder of
>> +test to check what the test originally was intended to check.
>> +--run is mostly useful when you want to focus on a specific test
>> +and know what you are doing.  Or when you want to run up to a
>> +certain test.
>>
>>
>>  Naming Tests
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index e035f36..63e481a 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -191,6 +191,14 @@ do
>>                 immediate=t; shift ;;
>>         -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
>>                 GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
>> +       -r)
>> +               shift; test "$#" -ne 0 || {
>> +                       echo 'error: -r requires an argument' >&2;
>> +                       exit 1;
>> +               }
>> +               run_list=$1; shift ;;
>> +       --run=*)
>> +               run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
>>         -h|--h|--he|--hel|--help)
>>                 help=t; shift ;;
>>         -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
>> @@ -366,6 +374,76 @@ match_pattern_list () {
>>         return 1
>>  }
>>
>> +match_run_pattern_list () {
>> +       arg="$1"
>> +       shift
>> +       test -z "$*" && return 0
>> +
>> +       # If the first patern is negative we include by default.
> s/patern/pattern/

Fixed.

>> +       include=
>> +       case "$1" in
>> +               [-!]*) include=t ;;
>> +       esac
>> +
>> +       for pattern_
>> +       do
>> +               orig_pattern=$pattern_
>> +
>> +               positive=t
>> +               case "$pattern_" in
>> +                       [-!]*)
>> +                               positive=
>> +                               pattern_=${pattern_##?}
>> +                               ;;
>> +               esac
>> +
>> +               # Short cut for "obvious" cases
>> +               [ "x$include" = "x" -a "x$positive" = "x" ] && continue
> Although there are a few exceptions in this script, 'test' is
> generally preferred over '['. Also, -a doesn't have great portability,
> so && may be better.
>
>     test -z "$include" && test -z "$positive" && continue

Did not know that.  Changed.

>> +               [ "x$include" = "xt" -a "x$positive" = "xt" ] && continue
> Since you're inside double quotes, you can drop the 'x' prefix:
>
>     test "$include" = t && test "$positive" = t && continue

Did not know that either %)
Fixed.
I saw it was used like that in the part that does argument parsing.  So
I just used style from there =)

>> +               pattern_op=
>> +               case "$pattern_" in
>> +                       \<=*)
>> +                               pattern_op='-le'
>> +                               pattern_=${pattern_##??}
>> +                               ;;
>> +                       \<*)
>> +                               pattern_op='-lt'
>> +                               pattern_=${pattern_##?}
>> +                               ;;
>> +                       \>=*)
>> +                               pattern_op='-ge'
>> +                               pattern_=${pattern_##??}
>> +                               ;;
>> +                       \>*)
>> +                               pattern_op='-gt'
>> +                               pattern_=${pattern_##?}
>> +                               ;;
>> +               esac
>> +
>> +               if test -n "$pattern_op"
>> +               then
>> +                       if expr "z$pattern_" : "z[0-9]*[^0-9]" >/dev/null
> Inside double quotes: you can drop the 'z' prefix.

Thanks.

>> +                       then
>> +                               echo "error: --run: test number contains" \
>> +                                       "non-digits: '$orig_pattern'" >&2
>> +                               exit 1
>> +                       fi
>> +                       if test $arg $pattern_op $pattern_
>> +                       then
>> +                               include=$positive
>> +                       fi
>> +               else
>> +                       case "$arg" in
>> +                               $pattern_)
>> +                                       include=$positive
>> +                       esac
>> +               fi
>> +       done
>> +
>> +       test -n "$include"
>> +}
>> +
>>  maybe_teardown_verbose () {
>>         test -z "$verbose_only" && return
>>         exec 4>/dev/null 3>/dev/null
>> @@ -470,6 +548,13 @@ test_skip () {
>>                 fi
>>                 skipped_reason="missing $missing_prereq${of_prereq}"
>>         fi
>> +       if test -z "$to_skip" && test -n "$run_list" &&
>> +               ! match_run_pattern_list $test_count $run_list
>> +       then
>> +               to_skip=t
>> +               skipped_reason="--run"
> A few pure bike-shedding comments (to be ignore if desired):
>
> I still don't understand the need to distinguish between a test
> skipped due to --run and and one skipped due to GIT_SKIP_TESTS.
>
> The skip-reason "GIT_SKIP_TESTS" (in patch 2/3) still seems
> unnecessarily verbose and loud.
>
> The skip-reason "excluded" (suggested in an earlier review) is short
> and sweet, and equally applicable to a test skipped either via --run
> or GIT_SKIP_TESTS.

Well, technically, it already says that the test is skipped.  So
adding "excluded" is actually a bit redundant.  Part that is in
the parenthesis explains the reason, at least for the case when
a test is skipped because a prerequisite is missing.

When you are just starting, I think, it is nice when the tool
tells you exactly why it was skipped - so that you do not have to
know everything to search in the right direction, if it does not
work the way you want it to.  When you already know what are you
doing, you probably ignore the exact text any way.  At least this
is how it is for me.

I am not sure I understand why "GIT_SKIP_TESTS" is verbose and
loud while "excluded" is sweet :)  Maybe I do not spend enough
time in the mail lists to have that association of all caps with
loud.  But in the land of Unix command line tools all caps means
"environment variable".  This is what I think when I see
"GIT_SKIP_TESTS".

P.S. Thanks for reviewing it :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] test-lib: Document short options in t/README
  2014-03-27 16:35           ` Junio C Hamano
@ 2014-03-28 17:20             ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2014-03-28 17:20 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Ramsay Jones, git, Thomas Rast, Eric Sunshine

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

> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> If there is decision on how shortening should work for all the
>> options, maybe I could add a paragraph on that and make existing
>> options more consistent.
>
> We should strive to make the following from gitcli.txt apply
> throughout the system:
>
>  * many commands allow a long option `--option` to be abbreviated
>    only to their unique prefix (e.g. if there is no other option
>    whose name begins with `opt`, you may be able to spell `--opt` to
>    invoke the `--option` flag), but you should fully spell them out
>    when writing your scripts; later versions of Git may introduce a
>    new option whose name shares the same prefix, e.g. `--optimize`,
>    to make a short prefix that used to be unique no longer unique.
>
>> If so, '--valgrind' becomes impossible to shorten because there
>> is '--valgrind-only' that is a separate option.  Same for
>> '--verbose'  and '--verbose-only'.
>
> Correct.  If you really cared, --valgrind={yes,no,only} would be (or
> have been) a better possibility, though.

Also, these existing bits are simply being lazy.  You do not have to
emulate and spread the laziness.

 t/test-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 87f327f..f37973a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -209,10 +209,10 @@ do
 	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
 		valgrind=memcheck
 		shift ;;
-	--valgrind=*)
+	--va=*|--val=*|--valg=*|--valgr=*|--valgri=*|--valgrin=*|--valgrind=*)
 		valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
 		shift ;;
-	--valgrind-only=*)
+  	--valgrind-o=*|--valgrind-on=*|--valgrind-onl=*|--valgrind-only=*)
 		valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
 		shift ;;
 	--tee)

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-28  7:05       ` Ilya Bobyr
@ 2014-03-30  9:41         ` Eric Sunshine
  2014-03-31 17:09           ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Sunshine @ 2014-03-30  9:41 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Git List, Junio C Hamano, Thomas Rast, Ramsay Jones

On Fri, Mar 28, 2014 at 3:05 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
> On 3/27/2014 8:36 PM, Eric Sunshine wrote:
>> On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
>>> Allow better control of the set of tests that will be executed for a
>>> single test suite.  Mostly useful while debugging or developing as it
>>> allows to focus on a specific test.
>>>
>>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>>> ---
>>>  No changes from the previous version.
>>>
>>>  t/README         |   65 ++++++++++++++-
>>>  t/t0000-basic.sh |  233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  t/test-lib.sh    |   85 ++++++++++++++++++++
>>>  3 files changed, 379 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/t/README b/t/README
>>> index 6b93aca..c911f89 100644
>>> --- a/t/README
>>> +++ b/t/README
>>> @@ -100,6 +100,10 @@ appropriately before running "make".
>>>         This causes additional long-running tests to be run (where
>>>         available), for more exhaustive testing.
>>>
>>> +-r,--run=<test numbers>::
>> Perhaps <test-selection> or something similar would be closer to the truth.
>
> I think your naming is better.  I will include it in the next version.
>
>>> +       This causes only specific tests to be included or excluded.  See
>> This is phrased somewhat oddly, as if you had already been talking
>> about tests being included or excluded, and that this option merely
>> changes that selection. Perhaps something like:
>>
>>     Run only the subset of tests indicated by <test-selection>.
>
> Will use that sentence as well :)
>
>>> +       section "Skipping Tests" below for "<test numbers>" syntax.
>>> +
>>>  --valgrind=<tool>::
>>>         Execute all Git binaries under valgrind tool <tool> and exit
>>>         with status 126 on errors (just like regular tests, this will
>>> @@ -187,10 +191,63 @@ and either can match the "t[0-9]{4}" part to skip the whole
>>>  test, or t[0-9]{4} followed by ".$number" to say which
>>>  particular test to skip.
>>>
>>> -Note that some tests in the existing test suite rely on previous
>>> -test item, so you cannot arbitrarily disable one and expect the
>>> -remainder of test to check what the test originally was intended
>>> -to check.
>>> +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.
>>> +
>>> +--run argument is a list of patterns with optional prefixes that
>> "The argument for --run is a list...
>
> I think it could be either.  But I am not a native speaker.
> So, I will use your version :)
>
>>> +are matched against test numbers within the current test suite.
>>> +Supported pattern:
>>> +
>>> + - A number matches a test with that number.
>>> +
>>> + - sh metacharacters such as '*', '?' and '[]' match as usual in
>>> +   shell.
>>> +
>>> + - A number prefixed with '<', '<=', '>', or '>=' matches all
>>> +   tests 'before', 'before or including', 'after', or 'after or
>>> +   including' the specified one.
>> I think you want "and" rather than "or": "before and including",
>> "after and including".
>
> I was thinking about an analogy to the corresponding mathematical
> operations here.  In mathematics, '<=' is called "less than or
> equal to"[1].
>
> If you are thinking about test numbers you can say that you
> include a test if it has a number before or equal to the given
> one.  The sentence is "A number prefixed with <= matches all
> tests [with numbers] before or including the specified [number]."

In English, it is idiomatic to say "less than or equal" for '<=', and
"greater than or equal" for '>='. "before or including" and "after or
including" are not used and sound odd. They also sound rather odd when
"or" is replaced with "and". Using the idiomatic forms should be fine.

> Maybe if I change "one" to "number" it would be a bit less
> ambiguous.  Or even include all the omitted words.

Changing "all tests" to "all test numbers" and using the idiomatic forms gives:

    A number prefixed with '<', '<=', '>', or '>=' matches,
    respectively, all test numbers less than, less than or equal,
    greater than, or greater than or equal to the specified one.

which isn't bad, though a bit verbose.

> I would not mind a completely different way to say it, but I am
> not yet sure that if I replace "or" with "and" it would make it
> a lot better.

Since the relational operators are fairly self-explanatory, you could
drop the prose explanation, though that might make it too cryptic:

    A number prefixed with '<', '<=', '>', or '>=' matches test
    numbers meeting the specified relation.

> [1] https://en.wikipedia.org/wiki/Inequality_%28mathematics%29
>
>>> +Optional prefixes are:
>>> +
>>> + - '+' or no prefix: test(s) matching the pattern are included in
>>> +   the run.
>>> +
>>> + - '-' or '!': test(s) matching the pattern are exluded from the
>>> +   run.
>> I've been playing with --run, and I find that test selection is not
>> especially intuitive. For instance, ">=16 !>24 !20" is easier to
>> reason about when written instead with ranges, such as "16-19 21-24",
>> or perhaps "16-24 !20". Open-ended ranges make sense too: "5-" means
>> tests 5 through the last, and "-5" means tests 1 through 5. (Yes, this
>> conflicts with your use of '-' to mean negation, but you already have
>> the perfectly serviceable '!' as an alias for negation.)
>
> I completely agree that ranges allow one to express certain
> "obvious" things much easier than just inequalities.  I was even
> thinking on a possible syntax.  But then I realized that I do not
> have a real use case for it.
>
> The only use case that I had is described in the cover letter: to
> run several setup tests and then the target test.  For that even
> simple lists were enough and I was using that original version
> with an environment variable.  After a conversation on the list I
> thought that it would be nice to be able to say '<', as it would
> save typing several extra characters for cases like '1 2 3 4 25'.
> While test suits that I've seen so far actually have no more than
> two tests that do the setup.
>
> The next use case that I could come up with was running up to a
> specific test.  I was in a situation were that would have been a
> nice option.  And inequalities allow that.
>
> Once again, I do not have a use case where I would need to run
> tests from 1 to 10, then 14 to 19 and then 100 and up to the end.
>
> Do you have something on your mind where that would be useful?

I don't have a particular use-case. I was just observing how much
easier it was to reason about ranges than relational operators,
especially when negation is involved. The two modes are not mutually
exclusive, though implementing both seems overkill.

> As for the syntax, ! is replaced in bash by the last executed
> command.  That happens inside double quotes as well and the
> original command line is not preserved (at lest for me).  So if
> it would be the only option that seemed like a limitation.  It is
> possible to escape it or use single quotes, of course.

It's not unprecedented to use '!' in a command argument. 'sed',
'find', 'awk' (to name a few) all accept '!' for some use or another,
and users of those commands are likely already comfortable escaping or
using single-quotes.

Tilde (~) has some mnemonic value as an inversion operator.

> I was thinking about a comma as a separator for ranges.

Are you saying "1,5" would be the range 1-5? That likely would be too
easily misread as just tests 1 and 5.

> As for the open ended ranges - they are the same as inequalities.
>
>>> +If --run starts with '+' or unprefixed pattern the initial set of
>>> +tests to run is empty. If the first pattern starts with '-' or
>>> +'!' all the tests are added to the initial set.  After initial
>>> +set is determined every pattern, test number or range is added or
>>> +excluded from the set one by one, from left to right.
>>> +
>>> +For example, common case is to run several setup tests and then a
>>> +specific test that relies on that setup:
>> Perhaps be a bit more specific:
>>
>>     ...run several setup tests (1, 2, 3) and then a
>>     specific test (21) that relies...
>
> Good idea, though it clutters the sentence a bit.
> Will use it.
>
>>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
>>> +
>>> +or:
>>> +
>>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<4 21'
>> It might be clearer to say "<=3" rather than "<4".
>
> I thought that < is less typing, so it should be the first one to show.
> Non-strict inequalities were next.
> I will change this example and show '<' and '>' in the next two.

It's not a big deal. It just seemed slightly easer to reason about how
"<=3", rather than "<4", was the same as "1 2 3" (since "4" was not
mentioned anywhere in the preceding example).

>>> +To run only tests up to a specific test one could do this:
>> s/specific test/specific test,/
>>
>> Also perhaps:
>>
>>     ...up to a specific test (21), one...
>
> Fixed.
>
>>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!>=21'
>>> +
>>> +As noted above test set is build going though patterns left to
>> s/above/above,/
>> s/test set/the test set/
>> s/build/built/
>>
>>     As noted above, the test set is built...
>
> Thanks :)
>
>>> +right, so this:
>>> +
>>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='<5 !3'
>>> +
>>> +will run tests 1, 2, and 4.
>>> +
>>> +Some tests in the existing test suite rely on previous test item,
>>> +so you cannot arbitrarily disable one and expect the remainder of
>>> +test to check what the test originally was intended to check.
>>> +--run is mostly useful when you want to focus on a specific test
>>> +and know what you are doing.  Or when you want to run up to a
>>> +certain test.
>>>
>>>
>>>  Naming Tests
>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>> index e035f36..63e481a 100644
>>> --- a/t/test-lib.sh
>>> +++ b/t/test-lib.sh
>>> @@ -191,6 +191,14 @@ do
>>>                 immediate=t; shift ;;
>>>         -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
>>>                 GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
>>> +       -r)
>>> +               shift; test "$#" -ne 0 || {
>>> +                       echo 'error: -r requires an argument' >&2;
>>> +                       exit 1;
>>> +               }
>>> +               run_list=$1; shift ;;
>>> +       --run=*)
>>> +               run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
>>>         -h|--h|--he|--hel|--help)
>>>                 help=t; shift ;;
>>>         -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
>>> @@ -366,6 +374,76 @@ match_pattern_list () {
>>>         return 1
>>>  }
>>>
>>> +match_run_pattern_list () {
>>> +       arg="$1"
>>> +       shift
>>> +       test -z "$*" && return 0
>>> +
>>> +       # If the first patern is negative we include by default.
>> s/patern/pattern/
>
> Fixed.
>
>>> +       include=
>>> +       case "$1" in
>>> +               [-!]*) include=t ;;
>>> +       esac
>>> +
>>> +       for pattern_
>>> +       do
>>> +               orig_pattern=$pattern_
>>> +
>>> +               positive=t
>>> +               case "$pattern_" in
>>> +                       [-!]*)
>>> +                               positive=
>>> +                               pattern_=${pattern_##?}
>>> +                               ;;
>>> +               esac
>>> +
>>> +               # Short cut for "obvious" cases
>>> +               [ "x$include" = "x" -a "x$positive" = "x" ] && continue
>> Although there are a few exceptions in this script, 'test' is
>> generally preferred over '['. Also, -a doesn't have great portability,
>> so && may be better.
>>
>>     test -z "$include" && test -z "$positive" && continue
>
> Did not know that.  Changed.
>
>>> +               [ "x$include" = "xt" -a "x$positive" = "xt" ] && continue
>> Since you're inside double quotes, you can drop the 'x' prefix:
>>
>>     test "$include" = t && test "$positive" = t && continue
>
> Did not know that either %)
> Fixed.
> I saw it was used like that in the part that does argument parsing.  So
> I just used style from there =)

A reason for the "x" trick is to avoid the following problem when $foo
expands to nothing:

% foo=
% test $foo = bar
bash: test: =: unary operator expected
% test x$foo = xbar
%

Quoting "$foo" achieves the same result.

Also, if the value of $foo starts with a hyphen, then 'test' would see
it as a command-line option. Prefixing with 'x' (or any letter) avoids
such misunderstanding.

It's also common to quote the expression to avoid the following
problem when $foo expands to multiple words:

% foo='foo bar'
% test $foo = bar
bash: test: too many arguments
% text "$foo" = bar
%

In your case, the values of $include and $positive are well-controlled
(only empty or 't'), so quoting alone is fine.

>>> +               pattern_op=
>>> +               case "$pattern_" in
>>> +                       \<=*)
>>> +                               pattern_op='-le'
>>> +                               pattern_=${pattern_##??}
>>> +                               ;;
>>> +                       \<*)
>>> +                               pattern_op='-lt'
>>> +                               pattern_=${pattern_##?}
>>> +                               ;;
>>> +                       \>=*)
>>> +                               pattern_op='-ge'
>>> +                               pattern_=${pattern_##??}
>>> +                               ;;
>>> +                       \>*)
>>> +                               pattern_op='-gt'
>>> +                               pattern_=${pattern_##?}
>>> +                               ;;
>>> +               esac
>>> +
>>> +               if test -n "$pattern_op"
>>> +               then
>>> +                       if expr "z$pattern_" : "z[0-9]*[^0-9]" >/dev/null
>> Inside double quotes: you can drop the 'z' prefix.
>
> Thanks.

It might be safer to keep the 'z' (or whatever) prefix on this one
since the pattern is coming from the user, thus not under your
control, and may start with a hyphen.

>>> +                       then
>>> +                               echo "error: --run: test number contains" \
>>> +                                       "non-digits: '$orig_pattern'" >&2
>>> +                               exit 1
>>> +                       fi
>>> +                       if test $arg $pattern_op $pattern_
>>> +                       then
>>> +                               include=$positive
>>> +                       fi
>>> +               else
>>> +                       case "$arg" in
>>> +                               $pattern_)
>>> +                                       include=$positive
>>> +                       esac
>>> +               fi
>>> +       done
>>> +
>>> +       test -n "$include"
>>> +}
>>> +
>>>  maybe_teardown_verbose () {
>>>         test -z "$verbose_only" && return
>>>         exec 4>/dev/null 3>/dev/null
>>> @@ -470,6 +548,13 @@ test_skip () {
>>>                 fi
>>>                 skipped_reason="missing $missing_prereq${of_prereq}"
>>>         fi
>>> +       if test -z "$to_skip" && test -n "$run_list" &&
>>> +               ! match_run_pattern_list $test_count $run_list
>>> +       then
>>> +               to_skip=t
>>> +               skipped_reason="--run"
>> A few pure bike-shedding comments (to be ignore if desired):
>>
>> I still don't understand the need to distinguish between a test
>> skipped due to --run and and one skipped due to GIT_SKIP_TESTS.
>>
>> The skip-reason "GIT_SKIP_TESTS" (in patch 2/3) still seems
>> unnecessarily verbose and loud.
>>
>> The skip-reason "excluded" (suggested in an earlier review) is short
>> and sweet, and equally applicable to a test skipped either via --run
>> or GIT_SKIP_TESTS.
>
> Well, technically, it already says that the test is skipped.  So
> adding "excluded" is actually a bit redundant.  Part that is in
> the parenthesis explains the reason, at least for the case when
> a test is skipped because a prerequisite is missing.
>
> When you are just starting, I think, it is nice when the tool
> tells you exactly why it was skipped - so that you do not have to
> know everything to search in the right direction, if it does not
> work the way you want it to.  When you already know what are you
> doing, you probably ignore the exact text any way.  At least this
> is how it is for me.
>
> I am not sure I understand why "GIT_SKIP_TESTS" is verbose and
> loud while "excluded" is sweet :)  Maybe I do not spend enough
> time in the mail lists to have that association of all caps with
> loud.  But in the land of Unix command line tools all caps means
> "environment variable".  This is what I think when I see
> "GIT_SKIP_TESTS".

It's loud in this sense: You generally want to ignore test output,
with the exception of failures which should loudly draw your
attention. Most of the output from the tests is lowercase, but the
uppercase (GIT_SKIP_TESTS) -- being different -- draws the attention
unnecessarily.

(Having actually just run some tests, I note that skipped tests
already are colored differently and the typical skip reason is
uppercase anyhow, so the above reasoning is likely unsupportable.)

But, as noted, this is pure bike-shedding. I won't bring it up again.

> P.S. Thanks for reviewing it :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-30  9:41         ` Eric Sunshine
@ 2014-03-31 17:09           ` Junio C Hamano
  2014-03-31 19:35             ` David Tran
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-03-31 17:09 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Ilya Bobyr, Git List, Thomas Rast, Ramsay Jones

Eric Sunshine <sunshine@sunshineco.com> writes:

> Since the relational operators are fairly self-explanatory, you could
> drop the prose explanation, though that might make it too cryptic:
>
>     A number prefixed with '<', '<=', '>', or '>=' matches test
>     numbers meeting the specified relation.

I would have to say that there is already an established pattern to
pick ranges that normal people understand well and it would be silly
to invent another more verbose way to express the same thing.  You
tell your Print Dialog which page to print with e.g. "-4,7,9-12,15-",
not ">=4 7 ...".  

Would the same notation be insufficient for our purpose?  You do not
even have to worry about negation that way.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-03-31 17:09           ` Junio C Hamano
@ 2014-03-31 19:35             ` David Tran
  0 siblings, 0 replies; 32+ messages in thread
From: David Tran @ 2014-03-31 19:35 UTC (permalink / raw)
  To: git

> Junio C Hamano <gitster <at> pobox.com> writes:
>
> 
> I would have to say that there is already an established pattern to
> pick ranges that normal people understand well and it would be silly
> to invent another more verbose way to express the same thing.  You
> tell your Print Dialog which page to print with e.g. "-4,7,9-12,15-",
> not ">=4 7 ...".  
> 
> Would the same notation be insufficient for our purpose?  You do not
> even have to worry about negation that way.
> 

That will do, especially if the numbers are in ascending order. We won't
have the odd cases of including a test and removing it later on. I think
we won't need a --neg flag to take the negation of the tests in that range?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-05-06 20:53     ` Junio C Hamano
@ 2014-05-06 21:02       ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2014-05-06 21:02 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: git, Thomas Rast, Eric Sunshine, Ramsay Jones

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

> The need to explain better with longer description will reduce the
> likelyhood that the feature is understood and correctly used.  When
> you can write "1-2,4-", why accept "1-4 !3" and force yourself to
> explain to people why that is different from "!3 1-4"?

By the way, having said all that, I would understand if the result
is made not depend on the order of selectors given to the option.

That is:

 - Find all the positive ones and form the "positive" set; if there is
   no positive ones, then make the "positive" set include everything.

 - Find all the negative ones and form a "negative" set.  The
   "negative" set can be an empty set if there is no negative
   selector.

 - Subtract the "negative" set from the "positive" set, and run only
   the tests in the resulting set.

Then "1-4 !3" and "!3 1-4" would mean the same thing.  Explanation
of the semantics would be far simpler than "we do it from left to
right".

One reason why the orders shouldn't matter is, unlike the "print
dialog" case, we won't run tests out of order when given "1 4 3 2".
We will still run "1-4".  So it would be natural for the readers to
expect that the orders they give selectors would not matter.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-30  9:50   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
@ 2014-05-06 20:53     ` Junio C Hamano
  2014-05-06 21:02       ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-05-06 20:53 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: git, Thomas Rast, Eric Sunshine, Ramsay Jones

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> Allow better control of the set of tests that will be executed for a
> single test suite.  Mostly useful while debugging or developing as it
> allows to focus on a specific test.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
> A number of minor changes according to the review comments.

I think the interaction between multiple selectors, especially when
some of them are negated, are much better explained in this version,
compared to the previous round in the README.

But I still think that the negation a feature that is unnecessary
and having it makes it harder to understand for users, especially
after reading this part:

> +If --run starts with an unprefixed number or range the initial
> +set of tests to run is empty. If the first item starts with '!'
> +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.
> ...
> +As noted above, the test set is built going though items left to
> +right, so this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
> +
> +will run tests 1, 2, and 4.  Items that comes later have higher
> +precendence.  It means that this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
> +
> +would just run tests from 1 to 4, including 3.

The initial !3 means the same thing as "1-2,4-", and then 1-4 will
do what to that set?  The answer is "It is added"... wouldn't the
reader expect then that the result should be "1-", not "1-4"?  I
myself wondered what would happen to the fifth test from your
description.  Has the text told the reader that t9200 test has only
four tests?

The need to explain better with longer description will reduce the
likelyhood that the feature is understood and correctly used.  When
you can write "1-2,4-", why accept "1-4 !3" and force yourself to
explain to people why that is different from "!3 1-4"?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-30  9:40     ` Ilya Bobyr
@ 2014-04-30 14:17       ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2014-04-30 14:17 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: git, Eric Sunshine, Ramsay Jones

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

>> The above two illustrate the reason rather well why I said it would
>> be better to avoid negation because it would complicate the mental
>> model the user needs to form when using the feature.
>
> I think that you do not have to use it if you do not need it.
> It adds some expressiveness, is rather easy to implement and is already
> there :)
> I can remove it, of cause, but is it really necessary?

An extra "expressiveness" that needs explanation and careful
thinking on the part of the user to pick the same world model you
picked among multiple valid world models is not necessarily a good
addition, so none of "you do not have to use it", "it's already
there", "it is easy to implement" is a valid argument.

If it weren't there, I wouldn't have had to wonder what the notation
meant, you wouldn't have had to explain it to me, and the most
importantly, nobody has to learn there is a subtle distinction
between "!7 -11", "!7-11" and "!7- 11".

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-30  9:50 ` [RFC/PATCH v4] Better control of the tests run by a test suite Ilya Bobyr
@ 2014-04-30  9:50   ` Ilya Bobyr
  2014-05-06 20:53     ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-04-30  9:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones, Ilya Bobyr

Allow better control of the set of tests that will be executed for a
single test suite.  Mostly useful while debugging or developing as it
allows to focus on a specific test.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
A number of minor changes according to the review comments.

 t/README         |   81 ++++++++++++-
 t/t0000-basic.sh |  356 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/test-lib.sh    |  108 ++++++++++++++++
 3 files changed, 537 insertions(+), 8 deletions(-)

diff --git a/t/README b/t/README
index eaf6ecd..cd99d21 100644
--- a/t/README
+++ b/t/README
@@ -104,6 +104,12 @@ appropriately before running "make".
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+-r::
+--run=<test-selector>::
+	Run only the subset of tests indicated by
+	<test-selector>.  See section "Skipping Tests" below for
+	<test-selector> syntax.
+
 --valgrind=<tool>::
 	Execute all Git binaries under valgrind tool <tool> and exit
 	with status 126 on errors (just like regular tests, this will
@@ -191,10 +197,77 @@ and either can match the "t[0-9]{4}" part to skip the whole
 test, or t[0-9]{4} followed by ".$number" to say which
 particular test to skip.
 
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+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.
+
+If --run starts with an unprefixed number or range the initial
+set of tests to run is empty. If the first item starts with '!'
+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:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
+
+or this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-21'
+
+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'
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
+
+As noted above, the test set is built going though items left to
+right, so this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
+
+will run tests 1, 2, and 4.  Items that comes later have higher
+precendence.  It means that this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
+
+would just run tests from 1 to 4, including 3.
+
+You may use negation with ranges.  The following will run all
+test in the test suite except from 7 up to 11:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
+
+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
+expect the rest to function correctly.
+
+--run is mostly useful when you want to focus on a specific test
+and know what setup is needed for it.  Or when you want to run
+everything up to a certain test.
 
 
 Naming Tests
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae8874e..8345c8a 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -42,9 +42,9 @@ test_expect_success 'success is reported like this' '
 	:
 '
 
-run_sub_test_lib_test () {
-	name="$1" descr="$2" # stdin is the body of the test code
-	shift 2
+_run_sub_test_lib_test_common () {
+	neg="$1" name="$2" descr="$3" # stdin is the body of the test code
+	shift 3
 	mkdir "$name" &&
 	(
 		# Pretend we're not running under a test harness, whether we
@@ -70,10 +70,23 @@ run_sub_test_lib_test () {
 		export TEST_DIRECTORY &&
 		TEST_OUTPUT_DIRECTORY=$(pwd) &&
 		export TEST_OUTPUT_DIRECTORY &&
-		./"$name.sh" "$@" >out 2>err
+		if test -z "$neg"
+		then
+			./"$name.sh" "$@" >out 2>err
+		else
+			!  ./"$name.sh" "$@" >out 2>err
+		fi
 	)
 }
 
+run_sub_test_lib_test () {
+	_run_sub_test_lib_test_common '' "$@"
+}
+
+run_sub_test_lib_test_err () {
+	_run_sub_test_lib_test_common '!' "$@"
+}
+
 check_sub_test_lib_test () {
 	name="$1" # stdin is the expected output from the test
 	(
@@ -84,6 +97,18 @@ check_sub_test_lib_test () {
 	)
 }
 
+check_sub_test_lib_test_err () {
+	name="$1" # stdin is the expected output output from the test
+	# expected error output is in descriptior 3
+	(
+		cd "$name" &&
+		sed -e 's/^> //' -e 's/Z$//' >expect.out &&
+		test_cmp expect.out out &&
+		sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
+		test_cmp expect.err err
+	)
+}
+
 test_expect_success 'pretend we have a fully passing test suite' "
 	run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
 	for i in 1 2 3
@@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
 	EOF
 "
 
+test_expect_success '--run basic' "
+	run_sub_test_lib_test run-basic \
+		'--run basic' --run='1 3 5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a range' "
+	run_sub_test_lib_test run-range \
+		'--run with a range' --run='1-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-ranges <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a left open range' "
+	run_sub_test_lib_test run-left-open-range \
+		'--run with a left open range' --run='-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-left-open-range <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a right open range' "
+	run_sub_test_lib_test run-right-open-range \
+		'--run with a right open range' --run='4-' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-right-open-range <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with basic negation' "
+	run_sub_test_lib_test run-basic-neg \
+		'--run with basic negation' --run='"'!3'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with two negations' "
+	run_sub_test_lib_test run-two-neg \
+		'--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'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-and-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run range negation' "
+	run_sub_test_lib_test run-range-neg \
+		'--run range negation' --run='"'!1-3'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-neg <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run include, exclude and include, comma separated' "
+	run_sub_test_lib_test run-inc-neg-inc-comma \
+		'--run include, exclude and include, comma separated' \
+		--run=1-5,\!1-3,2 <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc-comma <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run exclude and include' "
+	run_sub_test_lib_test run-neg-inc \
+		'--run exclude and include' \
+		--run='"'!3- 5'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run empty selectors' "
+	run_sub_test_lib_test run-empty-sel \
+		'--run empty selectors' \
+		--run='1,,3,,,5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-empty-sel <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run invalid range start' "
+	run_sub_test_lib_test_err run-inv-range-start \
+		'--run invalid range start' \
+		--run='a-5' <<-\\EOF &&
+	test_expect_success \"passing test #1\" 'true'
+	test_done
+	EOF
+	check_sub_test_lib_test_err run-inv-range-start \
+		<<-\\EOF_OUT 3<<-\\EOF_ERR
+	> FATAL: Unexpected exit with code 1
+	EOF_OUT
+	> error: --run: invalid non-numeric in range start: 'a-5'
+	EOF_ERR
+"
+
+test_expect_success '--run invalid range end' "
+	run_sub_test_lib_test_err run-inv-range-end \
+		'--run invalid range end' \
+		--run='1-z' <<-\\EOF &&
+	test_expect_success \"passing test #1\" 'true'
+	test_done
+	EOF
+	check_sub_test_lib_test_err run-inv-range-end \
+		<<-\\EOF_OUT 3<<-\\EOF_ERR
+	> FATAL: Unexpected exit with code 1
+	EOF_OUT
+	> error: --run: invalid non-numeric in range end: '1-z'
+	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
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e7d9c51..91000fe 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -191,6 +191,14 @@ do
 		immediate=t; shift ;;
 	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
 		GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
+	-r)
+		shift; test "$#" -ne 0 || {
+			echo 'error: -r requires an argument' >&2;
+			exit 1;
+		}
+		run_list=$1; shift ;;
+	--run=*)
+		run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
 	-h|--h|--he|--hel|--help)
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
@@ -366,6 +374,99 @@ match_pattern_list () {
 	return 1
 }
 
+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
+		orig_selector=$selector
+
+		positive=t
+		case "$selector" in
+			!*)
+				positive=
+				selector=${selector##?}
+				;;
+		esac
+
+		test -z "$selector" && continue
+
+		case "$selector" in
+			*-*)
+				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
+				then
+					echo "error: $title: 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" \
+						"end: '$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 test" \
+						"selector: '$orig_selector'" >&2
+					exit 1
+				fi
+		esac
+
+		# Short cut for "obvious" cases
+		test -z "$include" && test -z "$positive" && continue
+		test -n "$include" && test -n "$positive" && continue
+
+		case "$selector" in
+			-*)
+				if test $arg -le ${selector#-}
+				then
+					include=$positive
+				fi
+				;;
+			*-)
+				if test $arg -ge ${selector%-}
+				then
+					include=$positive
+				fi
+				;;
+			*-*)
+				if test ${selector%%-*} -le $arg \
+					&& test $arg -le ${selector#*-}
+				then
+					include=$positive
+				fi
+				;;
+			*)
+				if test $arg -eq $selector
+				then
+					include=$positive
+				fi
+				;;
+		esac
+	done
+
+	test -n "$include"
+}
+
 maybe_teardown_verbose () {
 	test -z "$verbose_only" && return
 	exec 4>/dev/null 3>/dev/null
@@ -470,6 +571,13 @@ test_skip () {
 		fi
 		skipped_reason="missing $missing_prereq${of_prereq}"
 	fi
+	if test -z "$to_skip" && test -n "$run_list" &&
+		! match_test_selector_list '--run' $test_count "$run_list"
+	then
+		to_skip=t
+		skipped_reason="--run"
+	fi
+
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-23 19:51   ` Eric Sunshine
@ 2014-04-30  9:41     ` Ilya Bobyr
  0 siblings, 0 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-04-30  9:41 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano, Ramsay Jones

On 4/23/2014 12:51 PM, Eric Sunshine wrote:
> On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
>> Allow better control of the set of tests that will be executed for a
>> single test suite.  Mostly useful while debugging or developing as it
>> allows to focus on a specific test.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>> diff --git a/t/README b/t/README
>> index 6b93aca..2dac619 100644
>> --- a/t/README
>> +++ b/t/README
>> +As noted above, the test set is built going though items left to
>> +right, so this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
>> +
>> +will run tests 1, 2, and 4.
>> +
>> +You may use negation with ranges.  The following will run all
>> +test as a test suite except from 7 upto 11:
> s/upto/up to/
> ...or...
> s/upto/through/

Fixed.  Thanks.

>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
>> +
>> +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
>> +expect the rest to function correctly.
>> +--run is mostly useful when you want to focus on a specific test
>> +and know what you are doing.  Or when you want to run up to a
>> +certain test.
>>
>>
>>  Naming Tests
>> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
>> index ae8874e..e2589cc 100755
>> --- a/t/t0000-basic.sh
>> +++ b/t/t0000-basic.sh
>> @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
>>         )
>>  }
>>
>> +check_sub_test_lib_test_err () {
>> +       name="$1" # stdin is the expected output output from the test
>> +       # expecte error output is in descriptor 3
> s/expecte/expected/

Fixed.

>> +       (
>> +               cd "$name" &&
>> +               sed -e 's/^> //' -e 's/Z$//' >expect.out &&
>> +               test_cmp expect.out out &&
>> +               sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
>> +               test_cmp expect.err err
>> +       )
>> +}
>> +
>>  test_expect_success 'pretend we have a fully passing test suite' "
>>         run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
>>         for i in 1 2 3
>> @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
>> +test_expect_success '--run invalid range start' "
>> +       run_sub_test_lib_test_err run-inv-range-start \
>> +               '--run invalid range start' \
>> +               --run='a-5' <<-\\EOF &&
>> +       test_expect_success \"passing test #1\" 'true'
>> +       test_done
>> +       EOF
>> +       check_sub_test_lib_test_err run-inv-range-start \
>> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
>> +       > FATAL: Unexpected exit with code 1
>> +       EOF_OUT
>> +       > error: --run: range start should contain only digits: 'a-5'
> This reads rather strangely, as if it's attempting to give an example
> (after the colon) of a valid digit range, but then shows something
> that is not valid. Rewording it slightly can eliminate the ambiguity:
>
>     error: --run: invalid non-numeric range start: 'a-5'

Changed.

>> +       EOF_ERR
>> +"
>> +
>> +test_expect_success '--run invalid range end' "
>> +       run_sub_test_lib_test_err run-inv-range-end \
>> +               '--run invalid range end' \
>> +               --run='1-z' <<-\\EOF &&
>> +       test_expect_success \"passing test #1\" 'true'
>> +       test_done
>> +       EOF
>> +       check_sub_test_lib_test_err run-inv-range-end \
>> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
>> +       > FATAL: Unexpected exit with code 1
>> +       EOF_OUT
>> +       > error: --run: range end should contain only digits: '1-z'
> Ditto.

Fixed.

>> +       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: test selector should contain only digits: '1?'
> And here:
>
>     error: --run: invalid non-digit in range selector: '1?'
>
> or something.

Changed to "invalid non-digit in test selector".  This one is only shown
if it does not have a "-", so it is probably not a range.

>> +       EOF_ERR
>> +"
>> +
>> +
>>  test_set_prereq HAVEIT
>>  haveit=no
>>  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index e7d9c51..46ba513 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -366,6 +374,100 @@ match_pattern_list () {
>>         return 1
>>  }
>>
>> +match_test_selector_list () {
>> +       title="$1"
>> +       shift
>> +       arg="$1"
>> +       shift
>> +       test -z "$1" && return 0
>> +
>> +       # Both commas and spaces are accepted as separators
>> +       OLDIFS=$IFS
>> +       IFS='   ,'
> The comment mentions only space and comma, but the actual assigned IFS
> value also treats tabs as separators. Perhaps update the comment to
> say "commas and whitespace".

I thought that tab is a space character =)  Changed it.

>> +       set -- $1
>> +       IFS=$OLDIFS
>> +
>> +       # If the first selector is negative we include by default.
>> +       include=
>> +       case "$1" in
>> +               !*) include=t ;;
>> +       esac
>> +
>> +       for selector
>> +       do
>> +               orig_selector=$selector
>> +
>> +
> Unnecessary extra blank line.

Thanks.

> [...]
>> +                               ;;
>> +                       *)
>> +                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
>> +                               then
>> +                                       echo "error: $title: test selector should contain" \
>> +                                               "only digits: '$orig_selector'" >&2
>> +                                       exit 1
>> +                               fi
>> +               esac
>> +
>> +               # Short cut for "obvious" cases
>> +               test -z "$include" && test -z "$positive" && continue
>> +               test -n "$include" && test -n "$positive" && continue
>> +
>> +               case "$selector" in
>> +                       -*)
>> +                               if test $arg -le ${selector#-}
>> +                               then
>> +                                       include=$positive
>> +                               fi
>> +                               ;;
>> +                       *-)
>> +                               if test $arg -ge ${selector%-}
>> +                               then
>> +                                       include=$positive
>> +                               fi
>> +                               ;;
>> +                       *-*)
>> +                               if test ${selector%%-*} -le $arg \
>> +                                       -a $arg -le ${selector#*-}
> The -a option to 'test' is not portable [1] and is considered obsolete
> by POSIX [2]. Use "test foo && test bar" instead.
>
> [1]: http://www.gnu.org/software/autoconf/manual/autoconf.html#index-g_t_0040command_007btest_007d-1793
> [2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

Did not know that.  Thanks.  Changed it.

It is used a number of times thought:

$ git grep '\<test\>.*-a\>' | wc -l
72

About 10 matches are accidental, but the rest are '-a' uses in 'test'.

>> [...]

Thanks a lot for looking into it :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-23 18:40   ` Junio C Hamano
@ 2014-04-30  9:40     ` Ilya Bobyr
  2014-04-30 14:17       ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Ilya Bobyr @ 2014-04-30  9:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Sunshine, Ramsay Jones

On 4/23/2014 11:40 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> @@ -187,10 +192,70 @@ and either can match the "t[0-9]{4}" part to skip the whole
>>  test, or t[0-9]{4} followed by ".$number" to say which
>>  particular test to skip.
>>  
>> -Note that some tests in the existing test suite rely on previous
>> -test item, so you cannot arbitrarily disable one and expect the
>> -remainder of test to check what the test originally was intended
>> -to check.
>> +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.
>> +
>> +If --run starts with an unprefixed number or range the initial
>> +set of tests to run is empty. If the first item starts with '!'
>> +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, 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'
>> +
>> +or:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
>> +
>> +or:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
> Good and easily understandable examples. 
>
>> +To run only tests up to a specific test (21), one could do this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
>> +
>> +or this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='-21'
> These may be redundant, given that the reader would have to have
> grokked the earlier "-3 21" already at this point.

The original idea was to show two most common use cases in the examples,
so that one could just copy/paste it.
I guess you are right that the second is a bit redundant now from the
standpoint of a person who is reading all of it.

I have reordered the examples.  Single range is simpler, it comes first
and then a more complicated example.

>> +As noted above, the test set is built going though items left to
>> +right, so this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
>> +
>> +will run tests 1, 2, and 4.
> I do not quite understand what you mean by "left to right"; is that
> implementation detail necessary for the user of the feature, or is
> it talking about some limitation coming from the implementation?
> e.g. perhaps "!3 1-4" would not work as people would expect "do not
> run 3, but run tests from 1 thru 4 otherwise", and warning against
> having such an expectation that cannot be fulfilled?

I thought that it is something that you may want to understand if you
are going to build something complicated.  As I do not have a specific
use case, this is kind of a made up example.
The idea is that what is on the right overwrites what is on the left. 
I've added that sentence as an additional clarification, and your example.

>> +You may use negation with ranges.  The following will run all
>> +test as a test suite except from 7 upto 11:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
> Hmm, that is somewhat counter-intuitive or at least ambiguous.  I
> first thought you would be running everything but skipping 7 thru
> 11, but your explanation is that it is equivalent to "-6,8-11" (that
> is, to intersect set "-11" and set "!7").

Your expectation is correct.
A space or a comma is needed in order for "!7" and "-11" to be treated
separately.
I am not sure why did you read the description as "-6,8-11".  There is a
typo in the sentence: s/as a/in the/.
I've changed that, but I would not object a better explanation of cause :)

> The above two illustrate the reason rather well why I said it would
> be better to avoid negation because it would complicate the mental
> model the user needs to form when using the feature.

I think that you do not have to use it if you do not need it.
It adds some expressiveness, is rather easy to implement and is already
there :)
I can remove it, of cause, but is it really necessary?

>> +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
>> +expect the rest to function correctly.
> What this text (moved from the top of this hunk) tells the reader
> applies to both the traditional t0123.4 and the new "--run=1-3,5-"
> syntaxes, but the new placement of it make it sound as if it is only
> for skipping with "--run", especially because the text before this
> paragraph and also after this paragraph both apply only to "--run".

True, but there is another paragraph at the beginning of the section
that talks why would you want to use GIT_SKIP_TESTS:

> In some environments, certain tests have no way of succeeding
> due to platform limitation, such as lack of 'unzip' program, or
> filesystem that do not allow arbitrary sequence of non-NUL bytes
> as pathnames.

I was thinking that if you would be working with individual test suits
you would use '--run'.
And this is where you more likely to think about setup tests.
I could move that paragraph just after the GIT_SKIP_TESTS description. 
Then it would apply more to both.
I am not sure it is needed.  Let me know if you think otherwise.

>> +--run is mostly useful when you want to focus on a specific test
>> +and know what you are doing.  Or when you want to run up to a
>> +certain test.
> Likewise for "and know what you are doing" part.  I'd suggest
> dropping that phrase from here, and/or make it part of the "you
> cannot randomly omit and expect later ones to work" that covers both
> ways to skip tests.

I've made this part a bit less wage.

Thank you for reviewing it :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-22  8:19 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
  2014-04-23 18:40   ` Junio C Hamano
@ 2014-04-23 19:51   ` Eric Sunshine
  2014-04-30  9:41     ` Ilya Bobyr
  1 sibling, 1 reply; 32+ messages in thread
From: Eric Sunshine @ 2014-04-23 19:51 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: Git List, Junio C Hamano, Thomas Rast, Ramsay Jones

On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
> Allow better control of the set of tests that will be executed for a
> single test suite.  Mostly useful while debugging or developing as it
> allows to focus on a specific test.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
> diff --git a/t/README b/t/README
> index 6b93aca..2dac619 100644
> --- a/t/README
> +++ b/t/README
> +As noted above, the test set is built going though items left to
> +right, so this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
> +
> +will run tests 1, 2, and 4.
> +
> +You may use negation with ranges.  The following will run all
> +test as a test suite except from 7 upto 11:

s/upto/up to/
...or...
s/upto/through/

> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
> +
> +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
> +expect the rest to function correctly.
> +--run is mostly useful when you want to focus on a specific test
> +and know what you are doing.  Or when you want to run up to a
> +certain test.
>
>
>  Naming Tests
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index ae8874e..e2589cc 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
>         )
>  }
>
> +check_sub_test_lib_test_err () {
> +       name="$1" # stdin is the expected output output from the test
> +       # expecte error output is in descriptor 3

s/expecte/expected/

> +       (
> +               cd "$name" &&
> +               sed -e 's/^> //' -e 's/Z$//' >expect.out &&
> +               test_cmp expect.out out &&
> +               sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
> +               test_cmp expect.err err
> +       )
> +}
> +
>  test_expect_success 'pretend we have a fully passing test suite' "
>         run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
>         for i in 1 2 3
> @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
> +test_expect_success '--run invalid range start' "
> +       run_sub_test_lib_test_err run-inv-range-start \
> +               '--run invalid range start' \
> +               --run='a-5' <<-\\EOF &&
> +       test_expect_success \"passing test #1\" 'true'
> +       test_done
> +       EOF
> +       check_sub_test_lib_test_err run-inv-range-start \
> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
> +       > FATAL: Unexpected exit with code 1
> +       EOF_OUT
> +       > error: --run: range start should contain only digits: 'a-5'

This reads rather strangely, as if it's attempting to give an example
(after the colon) of a valid digit range, but then shows something
that is not valid. Rewording it slightly can eliminate the ambiguity:

    error: --run: invalid non-numeric range start: 'a-5'

> +       EOF_ERR
> +"
> +
> +test_expect_success '--run invalid range end' "
> +       run_sub_test_lib_test_err run-inv-range-end \
> +               '--run invalid range end' \
> +               --run='1-z' <<-\\EOF &&
> +       test_expect_success \"passing test #1\" 'true'
> +       test_done
> +       EOF
> +       check_sub_test_lib_test_err run-inv-range-end \
> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
> +       > FATAL: Unexpected exit with code 1
> +       EOF_OUT
> +       > error: --run: range end should contain only digits: '1-z'

Ditto.

> +       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: test selector should contain only digits: '1?'

And here:

    error: --run: invalid non-digit in range selector: '1?'

or something.

> +       EOF_ERR
> +"
> +
> +
>  test_set_prereq HAVEIT
>  haveit=no
>  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index e7d9c51..46ba513 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -366,6 +374,100 @@ match_pattern_list () {
>         return 1
>  }
>
> +match_test_selector_list () {
> +       title="$1"
> +       shift
> +       arg="$1"
> +       shift
> +       test -z "$1" && return 0
> +
> +       # Both commas and spaces are accepted as separators
> +       OLDIFS=$IFS
> +       IFS='   ,'

The comment mentions only space and comma, but the actual assigned IFS
value also treats tabs as separators. Perhaps update the comment to
say "commas and whitespace".

> +       set -- $1
> +       IFS=$OLDIFS
> +
> +       # If the first selector is negative we include by default.
> +       include=
> +       case "$1" in
> +               !*) include=t ;;
> +       esac
> +
> +       for selector
> +       do
> +               orig_selector=$selector
> +
> +

Unnecessary extra blank line.

> +               positive=t
> +               case "$selector" in
> +                       !*)
> +                               positive=
> +                               selector=${selector##?}
> +                               ;;
> +               esac
> +
> +               test -z "$selector" && continue
> +
> +               case "$selector" in
> +                       *-*)
> +                               if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
> +                               then
> +                                       echo "error: $title: range start should contain only" \
> +                                               "digits: '$orig_selector'" >&2
> +                                       exit 1
> +                               fi
> +                               if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
> +                               then
> +                                       echo "error: $title: range end should contain only" \
> +                                               "digits: '$orig_selector'" >&2
> +                                       exit 1
> +                               fi

Weird ranges like "1-4-6" and "1-!5" will be caught by the "error:
range end" clause. Okay.

> +                               ;;
> +                       *)
> +                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> +                               then
> +                                       echo "error: $title: test selector should contain" \
> +                                               "only digits: '$orig_selector'" >&2
> +                                       exit 1
> +                               fi
> +               esac
> +
> +               # Short cut for "obvious" cases
> +               test -z "$include" && test -z "$positive" && continue
> +               test -n "$include" && test -n "$positive" && continue
> +
> +               case "$selector" in
> +                       -*)
> +                               if test $arg -le ${selector#-}
> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +                       *-)
> +                               if test $arg -ge ${selector%-}
> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +                       *-*)
> +                               if test ${selector%%-*} -le $arg \
> +                                       -a $arg -le ${selector#*-}

The -a option to 'test' is not portable [1] and is considered obsolete
by POSIX [2]. Use "test foo && test bar" instead.

[1]: http://www.gnu.org/software/autoconf/manual/autoconf.html#index-g_t_0040command_007btest_007d-1793
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +                       *)
> +                               if test $arg -eq $selector
> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +               esac
> +       done
> +
> +       test -n "$include"
> +}
> +
>  maybe_teardown_verbose () {
>         test -z "$verbose_only" && return
>         exec 4>/dev/null 3>/dev/null
> @@ -470,6 +572,13 @@ test_skip () {
>                 fi
>                 skipped_reason="missing $missing_prereq${of_prereq}"
>         fi
> +       if test -z "$to_skip" && test -n "$run_list" &&
> +               ! match_test_selector_list '--run' $test_count "$run_list"
> +       then
> +               to_skip=t
> +               skipped_reason="--run"
> +       fi
> +
>         case "$to_skip" in
>         t)
>                 say_color skip >&3 "skipping test: $@"
> --
> 1.7.9
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-22  8:19 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
@ 2014-04-23 18:40   ` Junio C Hamano
  2014-04-30  9:40     ` Ilya Bobyr
  2014-04-23 19:51   ` Eric Sunshine
  1 sibling, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2014-04-23 18:40 UTC (permalink / raw)
  To: Ilya Bobyr; +Cc: git, Thomas Rast, Eric Sunshine, Ramsay Jones

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> @@ -187,10 +192,70 @@ and either can match the "t[0-9]{4}" part to skip the whole
>  test, or t[0-9]{4} followed by ".$number" to say which
>  particular test to skip.
>  
> -Note that some tests in the existing test suite rely on previous
> -test item, so you cannot arbitrarily disable one and expect the
> -remainder of test to check what the test originally was intended
> -to check.
> +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.
> +
> +If --run starts with an unprefixed number or range the initial
> +set of tests to run is empty. If the first item starts with '!'
> +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, 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'
> +
> +or:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
> +
> +or:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'

Good and easily understandable examples. 

> +To run only tests up to a specific test (21), one could do this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
> +
> +or this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='-21'

These may be redundant, given that the reader would have to have
grokked the earlier "-3 21" already at this point.

> +As noted above, the test set is built going though items left to
> +right, so this:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
> +
> +will run tests 1, 2, and 4.

I do not quite understand what you mean by "left to right"; is that
implementation detail necessary for the user of the feature, or is
it talking about some limitation coming from the implementation?
e.g. perhaps "!3 1-4" would not work as people would expect "do not
run 3, but run tests from 1 thru 4 otherwise", and warning against
having such an expectation that cannot be fulfilled?

> +You may use negation with ranges.  The following will run all
> +test as a test suite except from 7 upto 11:
> +
> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'

Hmm, that is somewhat counter-intuitive or at least ambiguous.  I
first thought you would be running everything but skipping 7 thru
11, but your explanation is that it is equivalent to "-6,8-11" (that
is, to intersect set "-11" and set "!7").

The above two illustrate the reason rather well why I said it would
be better to avoid negation because it would complicate the mental
model the user needs to form when using the feature.

> +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
> +expect the rest to function correctly.

What this text (moved from the top of this hunk) tells the reader
applies to both the traditional t0123.4 and the new "--run=1-3,5-"
syntaxes, but the new placement of it make it sound as if it is only
for skipping with "--run", especially because the text before this
paragraph and also after this paragraph both apply only to "--run".

> +--run is mostly useful when you want to focus on a specific test
> +and know what you are doing.  Or when you want to run up to a
> +certain test.

Likewise for "and know what you are doing" part.  I'd suggest
dropping that phrase from here, and/or make it part of the "you
cannot randomly omit and expect later ones to work" that covers both
ways to skip tests.

Thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 3/3] test-lib: '--run' to run only specific tests
  2014-04-22  8:19 [RFC/PATCH v3] Better control of the tests run by a test suite Ilya Bobyr
@ 2014-04-22  8:19 ` Ilya Bobyr
  2014-04-23 18:40   ` Junio C Hamano
  2014-04-23 19:51   ` Eric Sunshine
  2014-04-30  9:50 ` [RFC/PATCH v4] Better control of the tests run by a test suite Ilya Bobyr
  1 sibling, 2 replies; 32+ messages in thread
From: Ilya Bobyr @ 2014-04-22  8:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Thomas Rast, Eric Sunshine, Ramsay Jones, Ilya Bobyr

Allow better control of the set of tests that will be executed for a
single test suite.  Mostly useful while debugging or developing as it
allows to focus on a specific test.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 t/README         |   73 +++++++++++-
 t/t0000-basic.sh |  356 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/test-lib.sh    |  109 +++++++++++++++++
 3 files changed, 530 insertions(+), 8 deletions(-)

diff --git a/t/README b/t/README
index 6b93aca..2dac619 100644
--- a/t/README
+++ b/t/README
@@ -100,6 +100,11 @@ appropriately before running "make".
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+-r,--run=<test-selector>::
+	Run only the subset of tests indicated by
+	<test-selector>.  See section "Skipping Tests" below for
+	<test-selector> syntax.
+
 --valgrind=<tool>::
 	Execute all Git binaries under valgrind tool <tool> and exit
 	with status 126 on errors (just like regular tests, this will
@@ -187,10 +192,70 @@ and either can match the "t[0-9]{4}" part to skip the whole
 test, or t[0-9]{4} followed by ".$number" to say which
 particular test to skip.
 
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+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.
+
+If --run starts with an unprefixed number or range the initial
+set of tests to run is empty. If the first item starts with '!'
+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, 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'
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
+
+or:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
+
+To run only tests up to a specific test (21), one could do this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
+
+or this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='-21'
+
+As noted above, the test set is built going though items left to
+right, so this:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
+
+will run tests 1, 2, and 4.
+
+You may use negation with ranges.  The following will run all
+test as a test suite except from 7 upto 11:
+
+    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
+
+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
+expect the rest to function correctly.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing.  Or when you want to run up to a
+certain test.
 
 
 Naming Tests
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae8874e..e2589cc 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -42,9 +42,9 @@ test_expect_success 'success is reported like this' '
 	:
 '
 
-run_sub_test_lib_test () {
-	name="$1" descr="$2" # stdin is the body of the test code
-	shift 2
+_run_sub_test_lib_test_common () {
+	neg="$1" name="$2" descr="$3" # stdin is the body of the test code
+	shift 3
 	mkdir "$name" &&
 	(
 		# Pretend we're not running under a test harness, whether we
@@ -70,10 +70,23 @@ run_sub_test_lib_test () {
 		export TEST_DIRECTORY &&
 		TEST_OUTPUT_DIRECTORY=$(pwd) &&
 		export TEST_OUTPUT_DIRECTORY &&
-		./"$name.sh" "$@" >out 2>err
+		if test -z "$neg"
+		then
+			./"$name.sh" "$@" >out 2>err
+		else
+			!  ./"$name.sh" "$@" >out 2>err
+		fi
 	)
 }
 
+run_sub_test_lib_test () {
+	_run_sub_test_lib_test_common '' "$@"
+}
+
+run_sub_test_lib_test_err () {
+	_run_sub_test_lib_test_common '!' "$@"
+}
+
 check_sub_test_lib_test () {
 	name="$1" # stdin is the expected output from the test
 	(
@@ -84,6 +97,18 @@ check_sub_test_lib_test () {
 	)
 }
 
+check_sub_test_lib_test_err () {
+	name="$1" # stdin is the expected output output from the test
+	# expecte error output is in descriptior 3
+	(
+		cd "$name" &&
+		sed -e 's/^> //' -e 's/Z$//' >expect.out &&
+		test_cmp expect.out out &&
+		sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
+		test_cmp expect.err err
+	)
+}
+
 test_expect_success 'pretend we have a fully passing test suite' "
 	run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
 	for i in 1 2 3
@@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
 	EOF
 "
 
+test_expect_success '--run basic' "
+	run_sub_test_lib_test run-basic \
+		'--run basic' --run='1 3 5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a range' "
+	run_sub_test_lib_test run-range \
+		'--run with a range' --run='1-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-ranges <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a left open range' "
+	run_sub_test_lib_test run-left-open-range \
+		'--run with a left open range' --run='-3' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-left-open-range <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with a right open range' "
+	run_sub_test_lib_test run-right-open-range \
+		'--run with a right open range' --run='4-' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-right-open-range <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with basic negation' "
+	run_sub_test_lib_test run-basic-neg \
+		'--run with basic negation' --run='"'!3'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-basic-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run with two negations' "
+	run_sub_test_lib_test run-two-neg \
+		'--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'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-two-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-and-neg <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 - passing test #4
+	> ok 5 # skip passing test #5 (--run)
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run range negation' "
+	run_sub_test_lib_test run-range-neg \
+		'--run range negation' --run='"'!1-3'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-range-neg <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 - passing test #6
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+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 &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run include, exclude and include, comma separated' "
+	run_sub_test_lib_test run-inc-neg-inc-comma \
+		'--run include, exclude and include, comma separated' \
+		--run=1-5,\!1-3,2 <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-inc-neg-inc-comma <<-\\EOF
+	> ok 1 # skip passing test #1 (--run)
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 - passing test #4
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run exclude and include' "
+	run_sub_test_lib_test run-neg-inc \
+		'--run exclude and include' \
+		--run='"'!3- 5'"' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-neg-inc <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 - passing test #2
+	> ok 3 # skip passing test #3 (--run)
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run empty selectors' "
+	run_sub_test_lib_test run-empty-sel \
+		'--run empty selectors' \
+		--run='1,,3,,,5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	test_done
+	EOF
+	check_sub_test_lib_test run-empty-sel <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> # passed all 6 test(s)
+	> 1..6
+	EOF
+"
+
+test_expect_success '--run invalid range start' "
+	run_sub_test_lib_test_err run-inv-range-start \
+		'--run invalid range start' \
+		--run='a-5' <<-\\EOF &&
+	test_expect_success \"passing test #1\" 'true'
+	test_done
+	EOF
+	check_sub_test_lib_test_err run-inv-range-start \
+		<<-\\EOF_OUT 3<<-\\EOF_ERR
+	> FATAL: Unexpected exit with code 1
+	EOF_OUT
+	> error: --run: range start should contain only digits: 'a-5'
+	EOF_ERR
+"
+
+test_expect_success '--run invalid range end' "
+	run_sub_test_lib_test_err run-inv-range-end \
+		'--run invalid range end' \
+		--run='1-z' <<-\\EOF &&
+	test_expect_success \"passing test #1\" 'true'
+	test_done
+	EOF
+	check_sub_test_lib_test_err run-inv-range-end \
+		<<-\\EOF_OUT 3<<-\\EOF_ERR
+	> FATAL: Unexpected exit with code 1
+	EOF_OUT
+	> error: --run: range end should contain only digits: '1-z'
+	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: test selector should contain only digits: '1?'
+	EOF_ERR
+"
+
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e7d9c51..46ba513 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -191,6 +191,14 @@ do
 		immediate=t; shift ;;
 	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
 		GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
+	-r)
+		shift; test "$#" -ne 0 || {
+			echo 'error: -r requires an argument' >&2;
+			exit 1;
+		}
+		run_list=$1; shift ;;
+	--run=*)
+		run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
 	-h|--h|--he|--hel|--help)
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
@@ -366,6 +374,100 @@ match_pattern_list () {
 	return 1
 }
 
+match_test_selector_list () {
+	title="$1"
+	shift
+	arg="$1"
+	shift
+	test -z "$1" && return 0
+
+	# Both commas and spaces 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
+		orig_selector=$selector
+
+
+		positive=t
+		case "$selector" in
+			!*)
+				positive=
+				selector=${selector##?}
+				;;
+		esac
+
+		test -z "$selector" && continue
+
+		case "$selector" in
+			*-*)
+				if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
+				then
+					echo "error: $title: range start should contain only" \
+						"digits: '$orig_selector'" >&2
+					exit 1
+				fi
+				if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
+				then
+					echo "error: $title: range end should contain only" \
+						"digits: '$orig_selector'" >&2
+					exit 1
+				fi
+				;;
+			*)
+				if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
+				then
+					echo "error: $title: test selector should contain" \
+						"only digits: '$orig_selector'" >&2
+					exit 1
+				fi
+		esac
+
+		# Short cut for "obvious" cases
+		test -z "$include" && test -z "$positive" && continue
+		test -n "$include" && test -n "$positive" && continue
+
+		case "$selector" in
+			-*)
+				if test $arg -le ${selector#-}
+				then
+					include=$positive
+				fi
+				;;
+			*-)
+				if test $arg -ge ${selector%-}
+				then
+					include=$positive
+				fi
+				;;
+			*-*)
+				if test ${selector%%-*} -le $arg \
+					-a $arg -le ${selector#*-}
+				then
+					include=$positive
+				fi
+				;;
+			*)
+				if test $arg -eq $selector
+				then
+					include=$positive
+				fi
+				;;
+		esac
+	done
+
+	test -n "$include"
+}
+
 maybe_teardown_verbose () {
 	test -z "$verbose_only" && return
 	exec 4>/dev/null 3>/dev/null
@@ -470,6 +572,13 @@ test_skip () {
 		fi
 		skipped_reason="missing $missing_prereq${of_prereq}"
 	fi
+	if test -z "$to_skip" && test -n "$run_list" &&
+		! match_test_selector_list '--run' $test_count "$run_list"
+	then
+		to_skip=t
+		skipped_reason="--run"
+	fi
+
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2014-05-06 21:02 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-24  8:49 [RFC/PATCH] Better control of the tests run by a test suite Ilya Bobyr
2014-03-24  8:49 ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
2014-03-24 11:39   ` Ramsay Jones
2014-03-24 17:19     ` Ilya Bobyr
2014-03-25 17:23       ` Junio C Hamano
2014-03-27  9:39         ` Ilya Bobyr
2014-03-27 16:35           ` Junio C Hamano
2014-03-28 17:20             ` Junio C Hamano
2014-03-25  5:52   ` Eric Sunshine
2014-03-24  8:49 ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
2014-03-24  8:49 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
2014-03-24 23:03 ` [RFC/PATCH] Better control of the tests run by a test suite Jeff King
2014-03-25  4:58   ` Junio C Hamano
2014-03-27 10:15     ` Ilya Bobyr
2014-03-27 10:32 ` [RFC/PATCH v2] " Ilya Bobyr
2014-03-27 10:32   ` [PATCH 1/3] test-lib: Document short options in t/README Ilya Bobyr
2014-03-27 10:32   ` [PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so Ilya Bobyr
2014-03-27 10:32   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
2014-03-28  3:36     ` Eric Sunshine
2014-03-28  7:05       ` Ilya Bobyr
2014-03-30  9:41         ` Eric Sunshine
2014-03-31 17:09           ` Junio C Hamano
2014-03-31 19:35             ` David Tran
2014-04-22  8:19 [RFC/PATCH v3] Better control of the tests run by a test suite Ilya Bobyr
2014-04-22  8:19 ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
2014-04-23 18:40   ` Junio C Hamano
2014-04-30  9:40     ` Ilya Bobyr
2014-04-30 14:17       ` Junio C Hamano
2014-04-23 19:51   ` Eric Sunshine
2014-04-30  9:41     ` Ilya Bobyr
2014-04-30  9:50 ` [RFC/PATCH v4] Better control of the tests run by a test suite Ilya Bobyr
2014-04-30  9:50   ` [PATCH 3/3] test-lib: '--run' to run only specific tests Ilya Bobyr
2014-05-06 20:53     ` Junio C Hamano
2014-05-06 21:02       ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.