* [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
@ 2014-05-20 21:26 Ramsay Jones
2014-05-20 21:33 ` Junio C Hamano
2014-05-20 21:40 ` Jonathan Nieder
0 siblings, 2 replies; 6+ messages in thread
From: Ramsay Jones @ 2014-05-20 21:26 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Ilya Bobyr, Junio C Hamano
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
The test suite has been failing for me on the pu branch for
a while now. I finally found a few minutes to take a look.
This failure is specific to the dash shell (/bin/sh) on my
system (ie it may well affect other shells, but I haven't
tested them all ... :). This does not affect bash (or bash
as /bin/sh aka bash --posix).
The GIT_SKIP_TESTS, 13-15, all fail with verbose output
similar to:
$ ./t0000-basic.sh -i -v
...
--- expect 2014-05-20 20:55:54.138342361 +0000
+++ out 2014-05-20 20:55:54.134342341 +0000
@@ -1,5 +1,5 @@
ok 1 - passing test #1
-ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ok 2 - passing test #2
ok 3 - passing test #3
# passed all 3 test(s)
1..3
not ok 13 - 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
#
$
... which looks like the sub-test does not see the GIT_SKIP_TESTS
variable at all. Indeed, if I put the entire test in a sub-shell
and replace the use of that variable on the run_sub_test_lib_test
invocation with a separate explicit assignment and export, then
the tests start working. (ie if I do the opposite of some other
recent commits!)
This patch is an RFC, because I take a different approach to the
above solution, only because the diff is much smaller and easier
to read! Is it a better solution?
ATB,
Ramsay Jones
t/t0000-basic.sh | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 8345c8a..373ad8f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -296,8 +296,9 @@ test_expect_success 'test --verbose-only' '
'
test_expect_success 'GIT_SKIP_TESTS' "
- GIT_SKIP_TESTS='git.2' \
- run_sub_test_lib_test git-skip-tests-basic \
+ GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
+ test_when_finished sane_unset GIT_SKIP_TESTS &&
+ run_sub_test_lib_test git-skip-tests-basic \
'GIT_SKIP_TESTS' <<-\\EOF &&
for i in 1 2 3
do
@@ -315,8 +316,9 @@ test_expect_success 'GIT_SKIP_TESTS' "
"
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='git.2 git.5' && export GIT_SKIP_TESTS &&
+ test_when_finished sane_unset GIT_SKIP_TESTS &&
+ 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
@@ -337,8 +339,9 @@ test_expect_success 'GIT_SKIP_TESTS several tests' "
"
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='git.[2-5]' && export GIT_SKIP_TESTS &&
+ test_when_finished sane_unset GIT_SKIP_TESTS &&
+ 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
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
2014-05-20 21:26 [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests Ramsay Jones
@ 2014-05-20 21:33 ` Junio C Hamano
2014-05-20 21:40 ` Jonathan Nieder
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2014-05-20 21:33 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Ilya Bobyr
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> This patch is an RFC, because I take a different approach to the
> above solution, only because the diff is much smaller and easier
> to read! Is it a better solution?
>
> ATB,
> Ramsay Jones
>
> t/t0000-basic.sh | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 8345c8a..373ad8f 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -296,8 +296,9 @@ test_expect_success 'test --verbose-only' '
> '
>
> test_expect_success 'GIT_SKIP_TESTS' "
> - GIT_SKIP_TESTS='git.2' \
> - run_sub_test_lib_test git-skip-tests-basic \
> + GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
> + test_when_finished sane_unset GIT_SKIP_TESTS &&
> + run_sub_test_lib_test git-skip-tests-basic \
> 'GIT_SKIP_TESTS' <<-\\EOF &&
The original is clearly wrong if run_sub_test_lib_test is a shell
function. I thought we hunted those down and killed them already,
but apparently we didn't.
I think exporting the variable and then clearing it in
test-when-finished is fine, and doing the export and run in a
subshell so that you do not have to clear is also fine.
> for i in 1 2 3
> do
> @@ -315,8 +316,9 @@ test_expect_success 'GIT_SKIP_TESTS' "
> "
>
> 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='git.2 git.5' && export GIT_SKIP_TESTS &&
> + test_when_finished sane_unset GIT_SKIP_TESTS &&
> + 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
> @@ -337,8 +339,9 @@ test_expect_success 'GIT_SKIP_TESTS several tests' "
> "
>
> 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='git.[2-5]' && export GIT_SKIP_TESTS &&
> + test_when_finished sane_unset GIT_SKIP_TESTS &&
> + 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
2014-05-20 21:26 [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests Ramsay Jones
2014-05-20 21:33 ` Junio C Hamano
@ 2014-05-20 21:40 ` Jonathan Nieder
2014-05-20 22:20 ` Ramsay Jones
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2014-05-20 21:40 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Ilya Bobyr, Junio C Hamano
Ramsay Jones wrote:
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -296,8 +296,9 @@ test_expect_success 'test --verbose-only' '
> '
>
> test_expect_success 'GIT_SKIP_TESTS' "
> - GIT_SKIP_TESTS='git.2' \
> - run_sub_test_lib_test git-skip-tests-basic \
> + GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
> + test_when_finished sane_unset GIT_SKIP_TESTS &&
Oof. Good catch.
What should happen if I have set GIT_SKIP_TESTS explicitly to run
only some of the tests in t0000-basic?
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
2014-05-20 21:40 ` Jonathan Nieder
@ 2014-05-20 22:20 ` Ramsay Jones
2014-05-20 22:44 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Ramsay Jones @ 2014-05-20 22:20 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: GIT Mailing-list, Ilya Bobyr, Junio C Hamano
On 20/05/14 22:40, Jonathan Nieder wrote:
> Ramsay Jones wrote:
>
>> --- a/t/t0000-basic.sh
>> +++ b/t/t0000-basic.sh
>> @@ -296,8 +296,9 @@ test_expect_success 'test --verbose-only' '
>> '
>>
>> test_expect_success 'GIT_SKIP_TESTS' "
>> - GIT_SKIP_TESTS='git.2' \
>> - run_sub_test_lib_test git-skip-tests-basic \
>> + GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
>> + test_when_finished sane_unset GIT_SKIP_TESTS &&
>
> Oof. Good catch.
>
> What should happen if I have set GIT_SKIP_TESTS explicitly to run
> only some of the tests in t0000-basic?
A quick test (with the above patch applied) shows that
it works as I would expect:
$ GIT_SKIP_TESTS=t0000.1[2-6] ./t0000-basic.sh
...
ok 11 - test --verbose
ok 12 # skip test --verbose-only (GIT_SKIP_TESTS)
ok 13 # skip GIT_SKIP_TESTS (GIT_SKIP_TESTS)
ok 14 # skip GIT_SKIP_TESTS several tests (GIT_SKIP_TESTS)
ok 15 # skip GIT_SKIP_TESTS sh pattern (GIT_SKIP_TESTS)
ok 16 # skip --run basic (GIT_SKIP_TESTS)
ok 17 - --run with a range
...
ok 77 - very long name in the index handled sanely
# passed all 77 test(s)
1..77
$
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
2014-05-20 22:20 ` Ramsay Jones
@ 2014-05-20 22:44 ` Jonathan Nieder
2014-05-20 23:33 ` Ramsay Jones
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2014-05-20 22:44 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Ilya Bobyr, Junio C Hamano
Hi,
Ramsay Jones wrote:
> On 20/05/14 22:40, Jonathan Nieder wrote:
>> What should happen if I have set GIT_SKIP_TESTS explicitly to run
>> only some of the tests in t0000-basic?
>
> A quick test (with the above patch applied) shows that
> it works as I would expect:
>
> $ GIT_SKIP_TESTS=t0000.1[2-6] ./t0000-basic.sh
> ...
> ok 11 - test --verbose
> ok 12 # skip test --verbose-only (GIT_SKIP_TESTS)
> ok 13 # skip GIT_SKIP_TESTS (GIT_SKIP_TESTS)
> ok 14 # skip GIT_SKIP_TESTS several tests (GIT_SKIP_TESTS)
> ok 15 # skip GIT_SKIP_TESTS sh pattern (GIT_SKIP_TESTS)
> ok 16 # skip --run basic (GIT_SKIP_TESTS)
> ok 17 - --run with a range
Try GIT_SKIP_TESTS=t0000.17 ./t0000-basic.sh:
ok 13 - GIT_SKIP_TESTS
ok 14 - GIT_SKIP_TESTS several tests
ok 15 - GIT_SKIP_TESTS sh pattern
ok 16 - --run basic
ok 17 - --run with a range
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
2014-05-20 22:44 ` Jonathan Nieder
@ 2014-05-20 23:33 ` Ramsay Jones
0 siblings, 0 replies; 6+ messages in thread
From: Ramsay Jones @ 2014-05-20 23:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: GIT Mailing-list, Ilya Bobyr, Junio C Hamano
On 20/05/14 23:44, Jonathan Nieder wrote:
> Hi,
>
> Ramsay Jones wrote:
>> On 20/05/14 22:40, Jonathan Nieder wrote:
>
>>> What should happen if I have set GIT_SKIP_TESTS explicitly to run
>>> only some of the tests in t0000-basic?
>>
>> A quick test (with the above patch applied) shows that
>> it works as I would expect:
>>
>> $ GIT_SKIP_TESTS=t0000.1[2-6] ./t0000-basic.sh
>> ...
>> ok 11 - test --verbose
>> ok 12 # skip test --verbose-only (GIT_SKIP_TESTS)
>> ok 13 # skip GIT_SKIP_TESTS (GIT_SKIP_TESTS)
>> ok 14 # skip GIT_SKIP_TESTS several tests (GIT_SKIP_TESTS)
>> ok 15 # skip GIT_SKIP_TESTS sh pattern (GIT_SKIP_TESTS)
>> ok 16 # skip --run basic (GIT_SKIP_TESTS)
>> ok 17 - --run with a range
>
> Try GIT_SKIP_TESTS=t0000.17 ./t0000-basic.sh:
>
> ok 13 - GIT_SKIP_TESTS
> ok 14 - GIT_SKIP_TESTS several tests
> ok 15 - GIT_SKIP_TESTS sh pattern
> ok 16 - --run basic
> ok 17 - --run with a range
Ah, yes. So my original patch was the better patch. :-P
That patch is given below; 'git diff -w' makes it easier
to see. However, given that this is still in pu on the
'ib/test-selectively-run' branch, hopefully Ilya can
simply squash this into a re-roll of his patches.
ATB,
Ramsay Jones
-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Subject: [PATCH] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
t/t0000-basic.sh | 108 +++++++++++++++++++++++++++++--------------------------
1 file changed, 57 insertions(+), 51 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 8345c8a..f10ba4a 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -296,66 +296,72 @@ test_expect_success 'test --verbose-only' '
'
test_expect_success 'GIT_SKIP_TESTS' "
- GIT_SKIP_TESTS='git.2' \
+ (
+ GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
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
+ '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' \
+ (
+ GIT_SKIP_TESTS='git.2 git.5' && export GIT_SKIP_TESTS &&
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
+ '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]' \
+ (
+ GIT_SKIP_TESTS='git.[2-5]' && export GIT_SKIP_TESTS &&
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
+ '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_expect_success '--run basic' "
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-20 23:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-20 21:26 [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests Ramsay Jones
2014-05-20 21:33 ` Junio C Hamano
2014-05-20 21:40 ` Jonathan Nieder
2014-05-20 22:20 ` Ramsay Jones
2014-05-20 22:44 ` Jonathan Nieder
2014-05-20 23:33 ` Ramsay Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).