git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] t0051: use "skip_all" under !MINGW in single-test file
@ 2022-02-01 20:35 Ævar Arnfjörð Bjarmason
  2022-02-01 21:48 ` Junio C Hamano
  2022-02-04 13:42 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 6+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-02-01 20:35 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff Hostetler, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason

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

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


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

* Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file
  2022-02-01 20:35 [PATCH] t0051: use "skip_all" under !MINGW in single-test file Ævar Arnfjörð Bjarmason
@ 2022-02-01 21:48 ` Junio C Hamano
  2022-02-02 19:44   ` Ævar Arnfjörð Bjarmason
  2022-02-04 13:42 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2022-02-01 21:48 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Jeff Hostetler, Johannes Schindelin

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

>
>     $ 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=$! &&

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

* Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file
  2022-02-01 21:48 ` Junio C Hamano
@ 2022-02-02 19:44   ` Ævar Arnfjörð Bjarmason
  2022-02-02 20:19     ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-02-02 19:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff Hostetler, Johannes Schindelin


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

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

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

>> 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=$! &&


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

* Re: [PATCH] t0051: use "skip_all" under !MINGW in single-test file
  2022-02-02 19:44   ` Ævar Arnfjörð Bjarmason
@ 2022-02-02 20:19     ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2022-02-02 20:19 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Jeff Hostetler, Johannes Schindelin

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

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




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

* [PATCH v2] t0051: use "skip_all" under !MINGW in single-test file
  2022-02-01 20:35 [PATCH] t0051: use "skip_all" under !MINGW in single-test file Ævar Arnfjörð Bjarmason
  2022-02-01 21:48 ` Junio C Hamano
@ 2022-02-04 13:42 ` Ævar Arnfjörð Bjarmason
  2022-02-08 13:40   ` Johannes Schindelin
  1 sibling, 1 reply; 6+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-02-04 13:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff Hostetler, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason

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


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

* Re: [PATCH v2] t0051: use "skip_all" under !MINGW in single-test file
  2022-02-04 13:42 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
@ 2022-02-08 13:40   ` Johannes Schindelin
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Schindelin @ 2022-02-08 13:40 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Jeff Hostetler

[-- Attachment #1: Type: text/plain, Size: 3291 bytes --]

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

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

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

end of thread, other threads:[~2022-02-08 13:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 20:35 [PATCH] t0051: use "skip_all" under !MINGW in single-test file Ævar Arnfjörð Bjarmason
2022-02-01 21:48 ` Junio C Hamano
2022-02-02 19:44   ` Ævar Arnfjörð Bjarmason
2022-02-02 20:19     ` Junio C Hamano
2022-02-04 13:42 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2022-02-08 13:40   ` Johannes Schindelin

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).