git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t4002: fix "diff can read from stdin" syntax
@ 2023-07-14 13:30 D. Ben Knoble via GitGitGadget
  2023-07-14 14:44 ` René Scharfe
  2023-07-14 16:52 ` Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: D. Ben Knoble via GitGitGadget @ 2023-07-14 13:30 UTC (permalink / raw)
  To: git; +Cc: D. Ben Knoble, D. Ben Knoble

From: "D. Ben Knoble" <ben.knoble+github@gmail.com>

I noticed this test was producing output like

```
t4002-diff-basic.sh: test_expect_successdiff can read from stdin: not found
```

which is rather odd. Investigation shows an error of shell syntax:
foo'abc' is the same as fooabc to the shell. Perhaps obviously, this is
not a valid command for the test.

I am surprised this doesn't count as an error in the test, but that
accounts for it going unnoticed.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
    t4002: fix "diff can read from stdin" syntax
    
    I noticed this test was producing output like
    
    t4002-diff-basic.sh: test_expect_successdiff can read from stdin: not found
    
    
    which is rather odd. Investigation shows an error of shell syntax:
    foo'abc' is the same as fooabc to the shell. Perhaps obviously, this is
    not a valid command for the test.
    
    I am surprised this doesn't count as an error in the test, but that
    accounts for it going unnoticed.
    
    ------------------------------------------------------------------------
    
    I would be interested in knowing how to "unsilence" failures like this
    so they do not go unnoticed in the future.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1545%2Fbenknoble%2Ft4002-diff-stdin-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1545/benknoble/t4002-diff-stdin-v1
Pull-Request: https://github.com/git/git/pull/1545

 t/t4002-diff-basic.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
index d524d4057dc..7afc883ec37 100755
--- a/t/t4002-diff-basic.sh
+++ b/t/t4002-diff-basic.sh
@@ -403,7 +403,7 @@ test_expect_success 'diff-tree -r B A == diff-tree -r -R A B' '
 	git diff-tree -r -R $tree_A $tree_B >.test-b &&
 	cmp -s .test-a .test-b'
 
-test_expect_success'diff can read from stdin' '
+test_expect_success 'diff can read from stdin' '
 	test_must_fail git diff --no-index -- MN - < NN |
 		grep -v "^index" | sed "s#/-#/NN#" >.test-a &&
 	test_must_fail git diff --no-index -- MN NN |

base-commit: aa9166bcc0ba654fc21f198a30647ec087f733ed
-- 
gitgitgadget

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

* Re: [PATCH] t4002: fix "diff can read from stdin" syntax
  2023-07-14 13:30 [PATCH] t4002: fix "diff can read from stdin" syntax D. Ben Knoble via GitGitGadget
@ 2023-07-14 14:44 ` René Scharfe
  2023-07-14 14:53   ` D. Ben Knoble
  2023-07-14 18:56   ` John Cai
  2023-07-14 16:52 ` Junio C Hamano
  1 sibling, 2 replies; 5+ messages in thread
From: René Scharfe @ 2023-07-14 14:44 UTC (permalink / raw)
  To: D. Ben Knoble via GitGitGadget, git; +Cc: D. Ben Knoble, John Cai

Am 14.07.23 um 15:30 schrieb D. Ben Knoble via GitGitGadget:
> diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
> index d524d4057dc..7afc883ec37 100755
> --- a/t/t4002-diff-basic.sh
> +++ b/t/t4002-diff-basic.sh
> @@ -403,7 +403,7 @@ test_expect_success 'diff-tree -r B A == diff-tree -r -R A B' '
>  	git diff-tree -r -R $tree_A $tree_B >.test-b &&
>  	cmp -s .test-a .test-b'
>
> -test_expect_success'diff can read from stdin' '
> +test_expect_success 'diff can read from stdin' '
>  	test_must_fail git diff --no-index -- MN - < NN |
>  		grep -v "^index" | sed "s#/-#/NN#" >.test-a &&
>  	test_must_fail git diff --no-index -- MN NN |
>

Good find!  Introduced by 9cfcbcc095 (t4002-diff-basic: modernize test
format, 2023-05-18).  Perhaps an automatic formatter would have avoided
it?  E.g. some sed(1) scripting, or shfmt (https://github.com/mvdan/sh)?
(Just discovered shfmt, never used it.)

René

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

* Re: [PATCH] t4002: fix "diff can read from stdin" syntax
  2023-07-14 14:44 ` René Scharfe
@ 2023-07-14 14:53   ` D. Ben Knoble
  2023-07-14 18:56   ` John Cai
  1 sibling, 0 replies; 5+ messages in thread
From: D. Ben Knoble @ 2023-07-14 14:53 UTC (permalink / raw)
  To: René Scharfe; +Cc: D. Ben Knoble via GitGitGadget, git, John Cai

On Fri, Jul 14, 2023 at 10:44 AM René Scharfe <l.s.r@web.de> wrote:
>
> Good find!  Introduced by 9cfcbcc095 (t4002-diff-basic: modernize test
> format, 2023-05-18).  Perhaps an automatic formatter would have avoided
> it?  E.g. some sed(1) scripting, or shfmt (https://github.com/mvdan/sh)?
> (Just discovered shfmt, never used it.)

I'm doubtful: it would have to know things about the test_* commands.
(I would expect context-blind formatters to leave it alone, as it is a
perfectly correct way to spell a command with spaces in the name,
though who would do such a thing, I don't know.)

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

* Re: [PATCH] t4002: fix "diff can read from stdin" syntax
  2023-07-14 13:30 [PATCH] t4002: fix "diff can read from stdin" syntax D. Ben Knoble via GitGitGadget
  2023-07-14 14:44 ` René Scharfe
@ 2023-07-14 16:52 ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-07-14 16:52 UTC (permalink / raw)
  To: D. Ben Knoble via GitGitGadget; +Cc: git, D. Ben Knoble

"D. Ben Knoble via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: "D. Ben Knoble" <ben.knoble+github@gmail.com>
>
> I noticed this test was producing output like
>
> ```
> t4002-diff-basic.sh: test_expect_successdiff can read from stdin: not found
> ```
>
> which is rather odd. Investigation shows an error of shell syntax:
> foo'abc' is the same as fooabc to the shell. Perhaps obviously, this is
> not a valid command for the test.
>
> I am surprised this doesn't count as an error in the test, but that
> accounts for it going unnoticed.
>
> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
> ---

Nicely spotted.  Thanks.

Will queue.

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

* Re: [PATCH] t4002: fix "diff can read from stdin" syntax
  2023-07-14 14:44 ` René Scharfe
  2023-07-14 14:53   ` D. Ben Knoble
@ 2023-07-14 18:56   ` John Cai
  1 sibling, 0 replies; 5+ messages in thread
From: John Cai @ 2023-07-14 18:56 UTC (permalink / raw)
  To: René Scharfe; +Cc: D. Ben Knoble via GitGitGadget, git, D. Ben Knoble


On 14 Jul 2023, at 10:44, René Scharfe wrote:

> Am 14.07.23 um 15:30 schrieb D. Ben Knoble via GitGitGadget:
>> diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
>> index d524d4057dc..7afc883ec37 100755
>> --- a/t/t4002-diff-basic.sh
>> +++ b/t/t4002-diff-basic.sh
>> @@ -403,7 +403,7 @@ test_expect_success 'diff-tree -r B A == diff-tree -r -R A B' '
>>  	git diff-tree -r -R $tree_A $tree_B >.test-b &&
>>  	cmp -s .test-a .test-b'
>>
>> -test_expect_success'diff can read from stdin' '
>> +test_expect_success 'diff can read from stdin' '
>>  	test_must_fail git diff --no-index -- MN - < NN |
>>  		grep -v "^index" | sed "s#/-#/NN#" >.test-a &&
>>  	test_must_fail git diff --no-index -- MN NN |
>>
>
> Good find!  Introduced by 9cfcbcc095 (t4002-diff-basic: modernize test
> format, 2023-05-18).  Perhaps an automatic formatter would have avoided
> it?  E.g. some sed(1) scripting, or shfmt (https://github.com/mvdan/sh)?
> (Just discovered shfmt, never used it.)

Thanks for finding this and fixing!

>
> René

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

end of thread, other threads:[~2023-07-14 18:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-14 13:30 [PATCH] t4002: fix "diff can read from stdin" syntax D. Ben Knoble via GitGitGadget
2023-07-14 14:44 ` René Scharfe
2023-07-14 14:53   ` D. Ben Knoble
2023-07-14 18:56   ` John Cai
2023-07-14 16:52 ` Junio C Hamano

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