public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] t2203: avoid masking exit codes in git status
@ 2026-01-17 17:58 Tian Yuchen
  2026-01-17 18:15 ` Junio C Hamano
  2026-01-18  4:35 ` [PATCH GSoC v2] " Tian Yuchen
  0 siblings, 2 replies; 4+ messages in thread
From: Tian Yuchen @ 2026-01-17 17:58 UTC (permalink / raw)
  To: git; +Cc: gitster

The test script t2203-add-intent.sh uses the pattern `git status | grep
-v` in multiple places. This pipeline masks the exit code of `git status`.
If `git status` crashes, `grep -v` will successfully filter nothing
and return 0, causing the test to pass incorrectly.

Update the tests to use an intermediate file, ensuring that the exit
codecof `git status` is checked and any crash is caught immediately.

In one instance, we also need to update the grep pattern to ignore the
temporary file it self, to avoid causing a regression in the test output
comparison.

Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
---
 t/t2203-add-intent.sh | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 192ad14b5f..ac8bafa680 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -16,7 +16,8 @@ test_expect_success 'intent to add' '
 '
 
 test_expect_success 'git status' '
-	git status --porcelain | grep -v actual >actual &&
+	git status --porcelain >tmp &&
+	grep -v -e actual -e tmp tmp >actual &&
 	cat >expect <<-\EOF &&
 	DA 1.t
 	A  elif
@@ -26,7 +27,8 @@ test_expect_success 'git status' '
 '
 
 test_expect_success 'git status with porcelain v2' '
-	git status --porcelain=v2 | grep -v "^?" >actual &&
+	git status --porcelain=v2 >tmp &&
+	grep -v "^?" tmp >actual &&
 	nam1=$(echo 1 | git hash-object --stdin) &&
 	nam2=$(git hash-object elif) &&
 	cat >expect <<-EOF &&
@@ -171,17 +173,20 @@ test_expect_success 'rename detection finds the right names' '
 		mv first third &&
 		git add -N third &&
 
-		git status | grep -v "^?" >actual.1 &&
+		git status >tmp &&
+		grep -v "^?" tmp >actual.1 &&
 		test_grep "renamed: *first -> third" actual.1 &&
 
-		git status --porcelain | grep -v "^?" >actual.2 &&
+		git status --porcelain >tmp &&
+		grep -v "^?" tmp >actual.2 &&
 		cat >expected.2 <<-\EOF &&
 		 R first -> third
 		EOF
 		test_cmp expected.2 actual.2 &&
 
 		hash=$(git hash-object third) &&
-		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		git status --porcelain=v2 >tmp &&
+		grep -v "^?" tmp >actual.3 &&
 		cat >expected.3 <<-EOF &&
 		2 .R N... 100644 100644 100644 $hash $hash R100 third	first
 		EOF
@@ -211,11 +216,13 @@ test_expect_success 'double rename detection in status' '
 		mv second third &&
 		git add -N third &&
 
-		git status | grep -v "^?" >actual.1 &&
+		git status >tmp &&
+		grep -v "^?" tmp >actual.1 &&
 		test_grep "renamed: *first -> second" actual.1 &&
 		test_grep "renamed: *second -> third" actual.1 &&
 
-		git status --porcelain | grep -v "^?" >actual.2 &&
+		git status --porcelain >tmp &&
+		grep -v "^?" tmp >actual.2 &&
 		cat >expected.2 <<-\EOF &&
 		R  first -> second
 		 R second -> third
@@ -223,7 +230,8 @@ test_expect_success 'double rename detection in status' '
 		test_cmp expected.2 actual.2 &&
 
 		hash=$(git hash-object third) &&
-		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		git status --porcelain=v2 >tmp &&
+		grep -v "^?" tmp >actual.3 &&
 		cat >expected.3 <<-EOF &&
 		2 R. N... 100644 100644 100644 $hash $hash R100 second	first
 		2 .R N... 100644 100644 100644 $hash $hash R100 third	second

base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75
-- 
2.43.0


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

* Re: [PATCH v1] t2203: avoid masking exit codes in git status
  2026-01-17 17:58 [PATCH v1] t2203: avoid masking exit codes in git status Tian Yuchen
@ 2026-01-17 18:15 ` Junio C Hamano
  2026-01-18  4:16   ` Yushin Tian
  2026-01-18  4:35 ` [PATCH GSoC v2] " Tian Yuchen
  1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2026-01-17 18:15 UTC (permalink / raw)
  To: Tian Yuchen; +Cc: git

Tian Yuchen <a3205153416@gmail.com> writes:

>  test_expect_success 'git status' '
> -	git status --porcelain | grep -v actual >actual &&
> +	git status --porcelain >tmp &&
> +	grep -v -e actual -e tmp tmp >actual &&

Looking at other parts of this patch makes me wonder if we can get
away by filtering lines that match "^?" out, instead of explicitly
naming 'actual' and 'tmp'.  It seems that the entire t2203 file does
not care about untraacked files at all (in other words, there is no
"expect" file that expects a line with '^?' in it), so rewriting the
above (and everything that this patch touches) to use something like

	git status --porcelain -uno >actual

makes the intent clear (i.e., we do not care about untracked files)
and simpler (i.e., we do not need "grep -v" to filter), wouldn't it?

>  	cat >expect <<-\EOF &&
>  	DA 1.t
>  	A  elif
> @@ -26,7 +27,8 @@ test_expect_success 'git status' '
>  '
>  
>  test_expect_success 'git status with porcelain v2' '
> -	git status --porcelain=v2 | grep -v "^?" >actual &&
> +	git status --porcelain=v2 >tmp &&
> +	grep -v "^?" tmp >actual &&
>  	nam1=$(echo 1 | git hash-object --stdin) &&
>  	nam2=$(git hash-object elif) &&
>  	cat >expect <<-EOF &&
> @@ -171,17 +173,20 @@ test_expect_success 'rename detection finds the right names' '
>  		mv first third &&
>  		git add -N third &&
>  
> -		git status | grep -v "^?" >actual.1 &&
> +		git status >tmp &&
> +		grep -v "^?" tmp >actual.1 &&
>  		test_grep "renamed: *first -> third" actual.1 &&
>  
> -		git status --porcelain | grep -v "^?" >actual.2 &&
> +		git status --porcelain >tmp &&
> +		grep -v "^?" tmp >actual.2 &&
>  		cat >expected.2 <<-\EOF &&
>  		 R first -> third
>  		EOF
>  		test_cmp expected.2 actual.2 &&
>  
>  		hash=$(git hash-object third) &&
> -		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
> +		git status --porcelain=v2 >tmp &&
> +		grep -v "^?" tmp >actual.3 &&
>  		cat >expected.3 <<-EOF &&
>  		2 .R N... 100644 100644 100644 $hash $hash R100 third	first
>  		EOF
> @@ -211,11 +216,13 @@ test_expect_success 'double rename detection in status' '
>  		mv second third &&
>  		git add -N third &&
>  
> -		git status | grep -v "^?" >actual.1 &&
> +		git status >tmp &&
> +		grep -v "^?" tmp >actual.1 &&
>  		test_grep "renamed: *first -> second" actual.1 &&
>  		test_grep "renamed: *second -> third" actual.1 &&
>  
> -		git status --porcelain | grep -v "^?" >actual.2 &&
> +		git status --porcelain >tmp &&
> +		grep -v "^?" tmp >actual.2 &&
>  		cat >expected.2 <<-\EOF &&
>  		R  first -> second
>  		 R second -> third
> @@ -223,7 +230,8 @@ test_expect_success 'double rename detection in status' '
>  		test_cmp expected.2 actual.2 &&
>  
>  		hash=$(git hash-object third) &&
> -		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
> +		git status --porcelain=v2 >tmp &&
> +		grep -v "^?" tmp >actual.3 &&
>  		cat >expected.3 <<-EOF &&
>  		2 R. N... 100644 100644 100644 $hash $hash R100 second	first
>  		2 .R N... 100644 100644 100644 $hash $hash R100 third	second
>
> base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75

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

* Re: [PATCH v1] t2203: avoid masking exit codes in git status
  2026-01-17 18:15 ` Junio C Hamano
@ 2026-01-18  4:16   ` Yushin Tian
  0 siblings, 0 replies; 4+ messages in thread
From: Yushin Tian @ 2026-01-18  4:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

>Looking at other parts of this patch makes me wonder if we can get
>away by filtering lines that match "^?" out, instead of explicitly
>naming 'actual' and 'tmp'.  It seems that the entire t2203 file does
>not care about untraacked files at all (in other words, there is no
>"expect" file that expects a line with '^?' in it), so rewriting the
>above (and everything that this patch touches) to use something like
>
>       git status --porcelain -uno >actual
>
>makes the intent clear (i.e., we do not care about untracked files)
>and simpler (i.e., we do not need "grep -v" to filter), wouldn't it?

You are right. Using `-uno` is much simpler and semantically closer to
what the tests are checking (changes in the index, not untracked files).
It also naturally avoids the exit code masking issue since the pipe is removed.

I have checked t2203 and confirmed that no test cases expect untracked
files in their output.

I will send a v2 patch shortly with this change, and I'll also add the [gsoc]
prefix which I missed in the first iteration.

Thanks for you suggestion!

>
> Tian Yuchen <a3205153416@gmail.com> writes:
>
> >  test_expect_success 'git status' '
> > -     git status --porcelain | grep -v actual >actual &&
> > +     git status --porcelain >tmp &&
> > +     grep -v -e actual -e tmp tmp >actual &&
>
> Looking at other parts of this patch makes me wonder if we can get
> away by filtering lines that match "^?" out, instead of explicitly
> naming 'actual' and 'tmp'.  It seems that the entire t2203 file does
> not care about untraacked files at all (in other words, there is no
> "expect" file that expects a line with '^?' in it), so rewriting the
> above (and everything that this patch touches) to use something like
>
>         git status --porcelain -uno >actual
>
> makes the intent clear (i.e., we do not care about untracked files)
> and simpler (i.e., we do not need "grep -v" to filter), wouldn't it?
>
> >       cat >expect <<-\EOF &&
> >       DA 1.t
> >       A  elif
> > @@ -26,7 +27,8 @@ test_expect_success 'git status' '
> >  '
> >
> >  test_expect_success 'git status with porcelain v2' '
> > -     git status --porcelain=v2 | grep -v "^?" >actual &&
> > +     git status --porcelain=v2 >tmp &&
> > +     grep -v "^?" tmp >actual &&
> >       nam1=$(echo 1 | git hash-object --stdin) &&
> >       nam2=$(git hash-object elif) &&
> >       cat >expect <<-EOF &&
> > @@ -171,17 +173,20 @@ test_expect_success 'rename detection finds the right names' '
> >               mv first third &&
> >               git add -N third &&
> >
> > -             git status | grep -v "^?" >actual.1 &&
> > +             git status >tmp &&
> > +             grep -v "^?" tmp >actual.1 &&
> >               test_grep "renamed: *first -> third" actual.1 &&
> >
> > -             git status --porcelain | grep -v "^?" >actual.2 &&
> > +             git status --porcelain >tmp &&
> > +             grep -v "^?" tmp >actual.2 &&
> >               cat >expected.2 <<-\EOF &&
> >                R first -> third
> >               EOF
> >               test_cmp expected.2 actual.2 &&
> >
> >               hash=$(git hash-object third) &&
> > -             git status --porcelain=v2 | grep -v "^?" >actual.3 &&
> > +             git status --porcelain=v2 >tmp &&
> > +             grep -v "^?" tmp >actual.3 &&
> >               cat >expected.3 <<-EOF &&
> >               2 .R N... 100644 100644 100644 $hash $hash R100 third   first
> >               EOF
> > @@ -211,11 +216,13 @@ test_expect_success 'double rename detection in status' '
> >               mv second third &&
> >               git add -N third &&
> >
> > -             git status | grep -v "^?" >actual.1 &&
> > +             git status >tmp &&
> > +             grep -v "^?" tmp >actual.1 &&
> >               test_grep "renamed: *first -> second" actual.1 &&
> >               test_grep "renamed: *second -> third" actual.1 &&
> >
> > -             git status --porcelain | grep -v "^?" >actual.2 &&
> > +             git status --porcelain >tmp &&
> > +             grep -v "^?" tmp >actual.2 &&
> >               cat >expected.2 <<-\EOF &&
> >               R  first -> second
> >                R second -> third
> > @@ -223,7 +230,8 @@ test_expect_success 'double rename detection in status' '
> >               test_cmp expected.2 actual.2 &&
> >
> >               hash=$(git hash-object third) &&
> > -             git status --porcelain=v2 | grep -v "^?" >actual.3 &&
> > +             git status --porcelain=v2 >tmp &&
> > +             grep -v "^?" tmp >actual.3 &&
> >               cat >expected.3 <<-EOF &&
> >               2 R. N... 100644 100644 100644 $hash $hash R100 second  first
> >               2 .R N... 100644 100644 100644 $hash $hash R100 third   second
> >
> > base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75

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

* [PATCH GSoC v2] t2203: avoid masking exit codes in git status
  2026-01-17 17:58 [PATCH v1] t2203: avoid masking exit codes in git status Tian Yuchen
  2026-01-17 18:15 ` Junio C Hamano
@ 2026-01-18  4:35 ` Tian Yuchen
  1 sibling, 0 replies; 4+ messages in thread
From: Tian Yuchen @ 2026-01-18  4:35 UTC (permalink / raw)
  To: git; +Cc: gitster

The test script t2203-add-intent.sh uses the pattern
`git status | grep -v` in multiple places. This pipeline
masks the exit code of `git status`.

Update the tests to use `git status -uno >actual`.
This excludes untracked files from the output entirely,
removing the need for filtering with `grep` and allowing us
to drop the pipeline, ensuring any crash in `git status`
is caught immediately.

Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
---
 t/t2203-add-intent.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 192ad14b5f..5bdd12c91f 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -16,7 +16,7 @@ test_expect_success 'intent to add' '
 '
 
 test_expect_success 'git status' '
-	git status --porcelain | grep -v actual >actual &&
+	git status --porcelain -uno >actual &&
 	cat >expect <<-\EOF &&
 	DA 1.t
 	A  elif
@@ -26,7 +26,7 @@ test_expect_success 'git status' '
 '
 
 test_expect_success 'git status with porcelain v2' '
-	git status --porcelain=v2 | grep -v "^?" >actual &&
+	git status --porcelain=v2 -uno >actual &&
 	nam1=$(echo 1 | git hash-object --stdin) &&
 	nam2=$(git hash-object elif) &&
 	cat >expect <<-EOF &&
@@ -171,17 +171,17 @@ test_expect_success 'rename detection finds the right names' '
 		mv first third &&
 		git add -N third &&
 
-		git status | grep -v "^?" >actual.1 &&
+		git status -uno >actual.1 &&
 		test_grep "renamed: *first -> third" actual.1 &&
 
-		git status --porcelain | grep -v "^?" >actual.2 &&
+		git status --porcelain -uno >actual.2 &&
 		cat >expected.2 <<-\EOF &&
 		 R first -> third
 		EOF
 		test_cmp expected.2 actual.2 &&
 
 		hash=$(git hash-object third) &&
-		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		git status --porcelain=v2 -uno >actual.3 &&
 		cat >expected.3 <<-EOF &&
 		2 .R N... 100644 100644 100644 $hash $hash R100 third	first
 		EOF
@@ -211,11 +211,11 @@ test_expect_success 'double rename detection in status' '
 		mv second third &&
 		git add -N third &&
 
-		git status | grep -v "^?" >actual.1 &&
+		git status -uno >actual.1 &&
 		test_grep "renamed: *first -> second" actual.1 &&
 		test_grep "renamed: *second -> third" actual.1 &&
 
-		git status --porcelain | grep -v "^?" >actual.2 &&
+		git status --porcelain -uno >actual.2 &&
 		cat >expected.2 <<-\EOF &&
 		R  first -> second
 		 R second -> third
@@ -223,7 +223,7 @@ test_expect_success 'double rename detection in status' '
 		test_cmp expected.2 actual.2 &&
 
 		hash=$(git hash-object third) &&
-		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		git status --porcelain=v2 -uno >actual.3 &&
 		cat >expected.3 <<-EOF &&
 		2 R. N... 100644 100644 100644 $hash $hash R100 second	first
 		2 .R N... 100644 100644 100644 $hash $hash R100 third	second

base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75
-- 
2.43.0


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

end of thread, other threads:[~2026-01-18  4:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-17 17:58 [PATCH v1] t2203: avoid masking exit codes in git status Tian Yuchen
2026-01-17 18:15 ` Junio C Hamano
2026-01-18  4:16   ` Yushin Tian
2026-01-18  4:35 ` [PATCH GSoC v2] " Tian Yuchen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox