threads / patch / 57352

patcht0051: use "skip_all" under !MINGW in single-test file

Subject: [PATCH] t0051: use "skip_all" under !MINGW in single-test file

## tl;dr

6 messages between Feb 1, 2022 and Feb 8, 2022. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Ævar Arnfjörð Bjarmason· Feb 1, 2022, 20:35 UTC · lore

Have this file added in 06ba9d03e34 (t0051: test GIT_TRACE to a windows named pipe, 2018-09-11) use the same "skip_all" pattern as an existing Windows-only test added in 0e218f91c29 (mingw: unset PERL5LIB by default, 2018-10-30) uses.

This way TAP consumers like "prove" will show a nice summary when the test is skipped, e.g.:

    $ prove t0051-windows-named-pipe.sh
    [...]
    t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
    [...]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

A trivial UX improvement for the "prove" output, so that we'll show a notice in the same way as e.g. t0029-core-unsetenvvars.sh and t5580-unc-paths.sh do (which are both Windows-specific).

 t/t0051-windows-named-pipe.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Show changes to t/t0051-windows-named-pipe.sh +6 −1

diff --git a/t/t0051-windows-named-pipe.sh b/t/t0051-windows-named-pipe.sh
index 10ac92d2250..412f413360d 100755
--- a/t/t0051-windows-named-pipe.sh
+++ b/t/t0051-windows-named-pipe.sh
@@ -3,8 +3,13 @@
 test_description='Windows named pipes'
 
 . ./test-lib.sh
+if ! test_have_prereq MINGW
+then
+	skip_all='skipping Windows-specific tests'
+	test_done
+fi
 
-test_expect_success MINGW 'o_append write to named pipe' '
+test_expect_success 'o_append write to named pipe' '
 	GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
 	{ test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
 	pid=$! &&

-- 
2.35.0.913.g12b4baa2536

Junio C Hamano· Feb 1, 2022, 21:48 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 7 quoted lines

> Have this file added in 06ba9d03e34 (t0051: test GIT_TRACE to a
> windows named pipe, 2018-09-11) use the same "skip_all" pattern as an
> existing Windows-only test added in 0e218f91c29 (mingw: unset PERL5LIB
> by default, 2018-10-30) uses.
>
> This way TAP consumers like "prove" will show a nice summary when the
> test is skipped, e.g.:

... as opposed to? A failure? A different appearance of the log message? Something else?

Show 35 quoted lines

>
>     $ prove t0051-windows-named-pipe.sh
>     [...]
>     t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
>     [...]
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> A trivial UX improvement for the "prove" output, so that we'll show a
> notice in the same way as e.g. t0029-core-unsetenvvars.sh and
> t5580-unc-paths.sh do (which are both Windows-specific).
>
>  t/t0051-windows-named-pipe.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/t0051-windows-named-pipe.sh b/t/t0051-windows-named-pipe.sh
> index 10ac92d2250..412f413360d 100755
> --- a/t/t0051-windows-named-pipe.sh
> +++ b/t/t0051-windows-named-pipe.sh
> @@ -3,8 +3,13 @@
>  test_description='Windows named pipes'
>  
>  . ./test-lib.sh
> +if ! test_have_prereq MINGW
> +then
> +	skip_all='skipping Windows-specific tests'
> +	test_done
> +fi
>  
> -test_expect_success MINGW 'o_append write to named pipe' '
> +test_expect_success 'o_append write to named pipe' '
>  	GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
>  	{ test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
>  	pid=$! &&

Ævar Arnfjörð Bjarmason· Feb 2, 2022, 19:44 UTC · re: Junio C Hamano · lore

Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file

On Tue, Feb 01 2022, Junio C Hamano wrote:

Show 12 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Have this file added in 06ba9d03e34 (t0051: test GIT_TRACE to a
>> windows named pipe, 2018-09-11) use the same "skip_all" pattern as an
>> existing Windows-only test added in 0e218f91c29 (mingw: unset PERL5LIB
>> by default, 2018-10-30) uses.
>>
>> This way TAP consumers like "prove" will show a nice summary when the
>> test is skipped, e.g.:
>
> ... as opposed to?  A failure?  A different appearance of the log
> message?  Something else?

In "prove" we go from simply showing "ok":
    
    $ prove t0051-windows-named-pipe.sh
    t0051-windows-named-pipe.sh .. ok   
    All tests successful.
    Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.03 cusr  0.01 csys =  0.06 CPU)
    Result: PASS
    
To showing the skip message quoted here:

Show 5 quoted lines

>>
>>     $ prove t0051-windows-named-pipe.sh
>>     [...]
>>     t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
>>     [...]

But YMMV. We're now making use of the right TAP-y way to communicate this to the consumer. I.e. skipping the whole test file, v.s. skipping individual tests (in this case there's only one test).

Show 29 quoted lines

>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> A trivial UX improvement for the "prove" output, so that we'll show a
>> notice in the same way as e.g. t0029-core-unsetenvvars.sh and
>> t5580-unc-paths.sh do (which are both Windows-specific).
>>
>>  t/t0051-windows-named-pipe.sh | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/t0051-windows-named-pipe.sh b/t/t0051-windows-named-pipe.sh
>> index 10ac92d2250..412f413360d 100755
>> --- a/t/t0051-windows-named-pipe.sh
>> +++ b/t/t0051-windows-named-pipe.sh
>> @@ -3,8 +3,13 @@
>>  test_description='Windows named pipes'
>>  
>>  . ./test-lib.sh
>> +if ! test_have_prereq MINGW
>> +then
>> +	skip_all='skipping Windows-specific tests'
>> +	test_done
>> +fi
>>  
>> -test_expect_success MINGW 'o_append write to named pipe' '
>> +test_expect_success 'o_append write to named pipe' '
>>  	GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
>>  	{ test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
>>  	pid=$! &&

Junio C Hamano· Feb 2, 2022, 20:19 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 15 quoted lines

>>> This way TAP consumers like "prove" will show a nice summary when the
>>> test is skipped, e.g.:
>>
>> ... as opposed to?  A failure?  A different appearance of the log
>> message?  Something else?
>
> In "prove" we go from simply showing "ok":
>     
>     $ prove t0051-windows-named-pipe.sh
>     t0051-windows-named-pipe.sh .. ok   
>     All tests successful.
>     Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.03 cusr  0.01 csys =  0.06 CPU)
>     Result: PASS
>     
> To showing the skip message quoted here:

Because you have been here long enough, I trust that you already know this, but for the benefit of those reading from sidelines...

As with many questions I give during my code review, this was not a request:"I do not understand this---please explain it to me". It poined out the part in the proposed log message that future readers of "git log" would be puzzled about and needs to be improved.

Thanks for a reply.

Ævar Arnfjörð Bjarmason· Feb 4, 2022, 13:42 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2] t0051: use "skip_all" under !MINGW in single-test file

This way TAP consumers like "prove" will show a nice summary when the test is skipped. Instead of:

    $ prove t0051-windows-named-pipe.sh
    [...]
    t0051-windows-named-pipe.sh .. ok
    [...]

We will prominently show a "skipped" notice:

    $ prove t0051-windows-named-pipe.sh
    [...]
    t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
    [...]

This is because we are now making use of the right TAP-y way to communicate this to the consumer. I.e. skipping the whole test file, v.s. skipping individual tests (in this case there's only one test).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Range-diff against v1:
1:  34ff968dcb8 ! 1:  1bc93bcba4b t0051: use "skip_all" under !MINGW in single-test file
    @@ Commit message
         by default, 2018-10-30) uses.
     
         This way TAP consumers like "prove" will show a nice summary when the
    -    test is skipped, e.g.:
    +    test is skipped. Instead of:
    +
    +        $ prove t0051-windows-named-pipe.sh
    +        [...]
    +        t0051-windows-named-pipe.sh .. ok
    +        [...]
    +
    +    We will prominently show a "skipped" notice:
     
             $ prove t0051-windows-named-pipe.sh
             [...]
             t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
             [...]
     
    +    This is because we are now making use of the right TAP-y way to
    +    communicate this to the consumer. I.e. skipping the whole test file,
    +    v.s. skipping individual tests (in this case there's only one test).
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t0051-windows-named-pipe.sh ##

 t/t0051-windows-named-pipe.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Show changes to t/t0051-windows-named-pipe.sh +6 −1

diff --git a/t/t0051-windows-named-pipe.sh b/t/t0051-windows-named-pipe.sh
index 10ac92d2250..412f413360d 100755
--- a/t/t0051-windows-named-pipe.sh
+++ b/t/t0051-windows-named-pipe.sh
@@ -3,8 +3,13 @@
 test_description='Windows named pipes'
 
 . ./test-lib.sh
+if ! test_have_prereq MINGW
+then
+	skip_all='skipping Windows-specific tests'
+	test_done
+fi
 
-test_expect_success MINGW 'o_append write to named pipe' '
+test_expect_success 'o_append write to named pipe' '
 	GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
 	{ test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
 	pid=$! &&

-- 
2.35.1.940.ge7a5b4b05f2

Johannes Schindelin· Feb 8, 2022, 13:40 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] t0051: use "skip_all" under !MINGW in single-test file

Hi Ævar,

On Fri, 4 Feb 2022, Ævar Arnfjörð Bjarmason wrote:

> Have this file added in 06ba9d03e34 (t0051: test GIT_TRACE to a
> windows named pipe, 2018-09-11) use the same "skip_all" pattern as an
> existing Windows-only test added in 0e218f91c29 (mingw: unset PERL5LIB
> by default, 2018-10-30) uses.

This is not a nit, even if I won't insist on changing it in this instance: This sentence is unnecessarily convoluted and therefore harder to read than it has to be. Please pay attention to readability next time you craft a commit message.

The rest of the commit message as well as the diff look good to me.

Thanks, Johannes

Show 77 quoted lines

>
> This way TAP consumers like "prove" will show a nice summary when the
> test is skipped. Instead of:
>
>     $ prove t0051-windows-named-pipe.sh
>     [...]
>     t0051-windows-named-pipe.sh .. ok
>     [...]
>
> We will prominently show a "skipped" notice:
>
>     $ prove t0051-windows-named-pipe.sh
>     [...]
>     t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
>     [...]
>
> This is because we are now making use of the right TAP-y way to
> communicate this to the consumer. I.e. skipping the whole test file,
> v.s. skipping individual tests (in this case there's only one test).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> Range-diff against v1:
> 1:  34ff968dcb8 ! 1:  1bc93bcba4b t0051: use "skip_all" under !MINGW in single-test file
>     @@ Commit message
>          by default, 2018-10-30) uses.
>
>          This way TAP consumers like "prove" will show a nice summary when the
>     -    test is skipped, e.g.:
>     +    test is skipped. Instead of:
>     +
>     +        $ prove t0051-windows-named-pipe.sh
>     +        [...]
>     +        t0051-windows-named-pipe.sh .. ok
>     +        [...]
>     +
>     +    We will prominently show a "skipped" notice:
>
>              $ prove t0051-windows-named-pipe.sh
>              [...]
>              t0051-windows-named-pipe.sh ... skipped: skipping Windows-specific tests
>              [...]
>
>     +    This is because we are now making use of the right TAP-y way to
>     +    communicate this to the consumer. I.e. skipping the whole test file,
>     +    v.s. skipping individual tests (in this case there's only one test).
>     +
>          Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
>       ## t/t0051-windows-named-pipe.sh ##
>
>  t/t0051-windows-named-pipe.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/t0051-windows-named-pipe.sh b/t/t0051-windows-named-pipe.sh
> index 10ac92d2250..412f413360d 100755
> --- a/t/t0051-windows-named-pipe.sh
> +++ b/t/t0051-windows-named-pipe.sh
> @@ -3,8 +3,13 @@
>  test_description='Windows named pipes'
>
>  . ./test-lib.sh
> +if ! test_have_prereq MINGW
> +then
> +	skip_all='skipping Windows-specific tests'
> +	test_done
> +fi
>
> -test_expect_success MINGW 'o_append write to named pipe' '
> +test_expect_success 'o_append write to named pipe' '
>  	GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
>  	{ test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
>  	pid=$! &&
> --
> 2.35.1.940.ge7a5b4b05f2
>
>