Git development
 help / color / mirror / Atom feed
From: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, ps@pks.im, abdobngad@gmail.com,
	bence@ferdinandy.com, john.a.passaro@gmail.com,
	r.siddharth.shrimali@gmail.com
Subject: [PATCH v2 3/3] t7004: avoid subshells to capture git exit codes
Date: Tue, 21 Apr 2026 11:03:34 +0530	[thread overview]
Message-ID: <20260421053334.5414-4-r.siddharth.shrimali@gmail.com> (raw)
In-Reply-To: <20260421053334.5414-1-r.siddharth.shrimali@gmail.com>

Several tests in t7004 use the 'test$(git ...) = ...' or the '! (git ...)'
subshell pattern. This swallows git's exit code. If git crashes
(e.g. segmentation fault) the crash would go undetected, and the test
would fail due to a mismatch or an inverted exit code.

Modernize these tests by directly writing output to files(actual) and
verifying them with 'test_cmp' or 'test_grep'. Replace subshell
negations with 'test_must_fail'. This way, if git crashes, the test
fails immediately and clearly instead of hiding the error behind a
string mismatch.

Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 t/t7004-tag.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index bef7618da2..d918005dd9 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -155,8 +155,10 @@ test_expect_success 'Multiple -l or --list options are equivalent to one -l opti
 '
 
 test_expect_success 'listing all tags if one exists should output that tag' '
-	test $(git tag -l) = mytag &&
-	test $(git tag) = mytag
+	git tag -l >actual &&
+	test_grep "^mytag$" actual &&
+	git tag >actual &&
+	test_grep "^mytag$" actual
 '
 
 # pattern matching:
@@ -166,11 +168,15 @@ test_expect_success 'listing a tag using a matching pattern should succeed' '
 '
 
 test_expect_success 'listing a tag with --ignore-case' '
-	test $(git tag -l --ignore-case MYTAG) = mytag
+	echo mytag >expect &&
+	git tag -l --ignore-case MYTAG >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'listing a tag using a matching pattern should output that tag' '
-	test $(git tag -l mytag) = mytag
+	echo mytag >expect &&
+	git tag -l mytag >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'listing tags using a non-matching pattern should succeed' '
@@ -430,8 +436,12 @@ test_expect_success 'listing tags -n in column with column.ui ignored' '
 
 test_expect_success 'a non-annotated tag created without parameters should point to HEAD' '
 	git tag non-annotated-tag &&
-	test $(git cat-file -t non-annotated-tag) = commit &&
-	test $(git rev-parse non-annotated-tag) = $(git rev-parse HEAD)
+	echo commit >expect &&
+	git cat-file -t non-annotated-tag >actual &&
+	test_cmp expect actual &&
+	git rev-parse HEAD >expect &&
+	git rev-parse non-annotated-tag >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'trying to verify an unknown tag should fail' '
@@ -1520,11 +1530,11 @@ test_expect_success GPG 'verify signed tag fails when public key is not present'
 '
 
 test_expect_success 'git tag -a fails if tag annotation is empty' '
-	! (GIT_EDITOR=cat git tag -a initial-comment)
+	test_must_fail env GIT_EDITOR=cat git tag -a initial-comment
 '
 
 test_expect_success 'message in editor has initial comment' '
-	! (GIT_EDITOR=cat git tag -a initial-comment >actual)
+	test_must_fail env GIT_EDITOR=cat git tag -a initial-comment >actual
 '
 
 test_expect_success 'message in editor has initial comment: first line' '
-- 
2.51.2


  parent reply	other threads:[~2026-04-21  5:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14 14:18 [PATCH 0/3] t7004: cleanup and modernize brittle tests Siddharth Shrimali
2026-04-14 14:18 ` [PATCH 1/3] t7004: drop hardcoded tag count in invalid name test Siddharth Shrimali
2026-04-14 16:54   ` Junio C Hamano
2026-04-20  7:13     ` Patrick Steinhardt
2026-04-14 14:18 ` [PATCH 2/3] t7004: dynamically grab expected state in tests Siddharth Shrimali
2026-04-14 17:00   ` Junio C Hamano
2026-04-14 14:18 ` [PATCH 3/3] t7004: avoid subshells to capture git exit codes Siddharth Shrimali
2026-04-21  5:33 ` [PATCH v2 0/3] t7004: cleanup and modernize brittle tests Siddharth Shrimali
2026-04-21  5:33   ` [PATCH v2 1/3] t7004: drop hardcoded tag count for state verification Siddharth Shrimali
2026-04-21  5:33   ` [PATCH v2 2/3] t7004: dynamically grab expected state in tests Siddharth Shrimali
2026-04-21  5:33   ` Siddharth Shrimali [this message]
2026-04-21  5:38   ` [PATCH v2 0/3] t7004: cleanup and modernize brittle tests Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260421053334.5414-4-r.siddharth.shrimali@gmail.com \
    --to=r.siddharth.shrimali@gmail.com \
    --cc=abdobngad@gmail.com \
    --cc=bence@ferdinandy.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john.a.passaro@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox