patcht7004: replace wc -l with modern test helpers
2 messages between Apr 1, 2026 and Apr 1, 2026, from Siddharth Shrimali, Junio C Hamano.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
Pipelines of the form "test $(git tag | wc -l) -eq 0" suppress git's exit code. This means a crash or unexpected failure from git tag would go undetected. Additionally, the use of $(...) creates a subshell for each check, which adds unnecessary overhead.
Replace these patterns with test_must_be_empty and test_line_count. These helpers check the output of git directly from a file, ensuring git's exit code is captured properly via the preceding "&&" chain. They also provide better diagnostics on failure by printing the contents of the file when a check does not pass.
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
t/t7004-tag.sh | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
Show changes to t/t7004-tag.sh +10 −5
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index ce2ff2a28a..faf7d97fc4 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -33,8 +33,10 @@ test_expect_success 'listing all tags in an empty tree should succeed' '
'
test_expect_success 'listing all tags in an empty tree should output nothing' '
- test $(git tag -l | wc -l) -eq 0 &&
- test $(git tag | wc -l) -eq 0
+ git tag -l >actual &&
+ test_must_be_empty actual &&
+ git tag >actual &&
+ test_must_be_empty actual
'
test_expect_success 'sort tags, ignore case' '
@@ -178,7 +180,8 @@ test_expect_success 'listing tags using a non-matching pattern should succeed' '
'
test_expect_success 'listing tags using a non-matching pattern should output nothing' '
- test $(git tag -l xxx | wc -l) -eq 0
+ git tag -l xxx >actual &&
+ test_must_be_empty actual
'
# special cases for creating tags:
@@ -188,13 +191,15 @@ test_expect_success 'trying to create a tag with the name of one existing should
'
test_expect_success 'trying to create a tag with a non-valid name should fail' '
- test $(git tag -l | wc -l) -eq 1 &&
+ git tag -l >actual &&
+ test_line_count = 1 actual &&
test_must_fail git tag "" &&
test_must_fail git tag .othertag &&
test_must_fail git tag "other tag" &&
test_must_fail git tag "othertag^" &&
test_must_fail git tag "other~tag" &&
- test $(git tag -l | wc -l) -eq 1
+ git tag -l >actual &&
+ test_line_count = 1 actual
'
test_expect_success 'creating a tag using HEAD directly should succeed' '
--
2.51.2
Re: [PATCH] t7004: replace wc -l with modern test helpers
Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:
Show 15 quoted lines
> Pipelines of the form "test $(git tag | wc -l) -eq 0" suppress git's
> exit code. This means a crash or unexpected failure from git tag would
> go undetected. Additionally, the use of $(...) creates a subshell for
> each check, which adds unnecessary overhead.
>
> Replace these patterns with test_must_be_empty and test_line_count.
> These helpers check the output of git directly from a file, ensuring
> git's exit code is captured properly via the preceding "&&" chain.
> They also provide better diagnostics on failure by printing the
> contents of the file when a check does not pass.
>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
> t/t7004-tag.sh | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
They all look the result of good and mechanical transformation. Will queue.
Some tests, however, may want to be cleaned up on top in a follow-up patch.
For example, taking a look at this one:
Show 13 quoted lines
> test_expect_success 'trying to create a tag with a non-valid name should fail' '
> - test $(git tag -l | wc -l) -eq 1 &&
> + git tag -l >actual &&
> + test_line_count = 1 actual &&
> test_must_fail git tag "" &&
> test_must_fail git tag .othertag &&
> test_must_fail git tag "other tag" &&
> test_must_fail git tag "othertag^" &&
> test_must_fail git tag "other~tag" &&
> - test $(git tag -l | wc -l) -eq 1
> + git tag -l >actual &&
> + test_line_count = 1 actual
> '
it is dubious to insist that we have a single tag at the beginning of the test, as more tests can be added before this step, or some tests may be retired before this step, changing the expected state before we start this step. As the test title says, all the tag creation tests that we mark with test_must_fail are very relevant to this one, but the last one that counts the existing tag again is pointless, as it adds an unnecessary dependency on the state left by previous tests (i.e., if we start with a single tag in the repository, and we expect all the tag creation in this test fail, then we should end up with a single tag in the repository---but this means that every time a future change makes previous tests leave no tag or two or more tags before this test runs, this test needs to be updated to expect different number of tags).
I didn't bother looking at all the other tests outside what was visible in the patch, so please do not just react on this comment and update only this test. If we were to make such a clean up that is more than mechanical rewrite, it should be done with a better understanding of the whole picture of the test script, not just what I happened to have noticed within the context of this patch that changed something unrelated (i.e., use of "| wc -l").
Thanks.