| Message ID | patch-v4-6.6-94df7a1764e-20221219T101240Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9fdc79ecba0f4a3ef885f1409d2db5a1dbabd649 |
| Headers | show |
| Series | tests: fix ignored & hidden exit codes | expand |
Hi Ævar On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote: > Fix a few miscellaneous cases where: > > - We lost the "git" exit code via "git ... | grep" > - Likewise by having a $(git) argument to git itself > - Used "test -z" to check that a command emitted no output, we can use > "test_must_be_empty" and &&-chaining instead. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > [...] > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index 5841f280fb2..f1fe5d60677 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' ' > echo content >>file && > chmod +x file && > printf "y\\ny\\n" | git add -p && > - git diff --cached file | grep "new mode" && > - git diff --cached file | grep "+content" && > - test -z "$(git diff file)" > + git diff --cached file >out && > + grep "new mode" out && > + grep "+content" out && > + git diff file >out && > + test_must_be_empty out "git diff --exit-code file" would suffice here, we don't need to redirect the output and check that it is empty. Best Wishes Phillip
On Tue, Dec 27 2022, Phillip Wood wrote: > Hi Ævar > > On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote: >> Fix a few miscellaneous cases where: >> - We lost the "git" exit code via "git ... | grep" >> - Likewise by having a $(git) argument to git itself >> - Used "test -z" to check that a command emitted no output, we can use >> "test_must_be_empty" and &&-chaining instead. >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> [...] >> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh >> index 5841f280fb2..f1fe5d60677 100755 >> --- a/t/t3701-add-interactive.sh >> +++ b/t/t3701-add-interactive.sh >> @@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' ' >> echo content >>file && >> chmod +x file && >> printf "y\\ny\\n" | git add -p && >> - git diff --cached file | grep "new mode" && >> - git diff --cached file | grep "+content" && >> - test -z "$(git diff file)" >> + git diff --cached file >out && >> + grep "new mode" out && >> + grep "+content" out && >> + git diff file >out && >> + test_must_be_empty out > > "git diff --exit-code file" would suffice here, we don't need to > redirect the output and check that it is empty. Correct. Or even "git diff-files -s --exit-code", which might make things clearer still, or have this use the "diff_cmp" helper defined in the test file, as most of its siblings do. But as with a sibling comment of mine I wanted to avoid starting to refactoring these tests for general betterment, and just to narrowly avoid losing the exit code.
Phillip Wood <phillip.wood123@gmail.com> writes: >> - test -z "$(git diff file)" >> ... >> + git diff file >out && >> + test_must_be_empty out > > "git diff --exit-code file" would suffice here, we don't need to > redirect the output and check that it is empty. Yes, but the test is trying to be faithful to not just the intent but also the implementation of the original, I think.
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh index d708acdb819..5e36899d207 100755 --- a/t/t1401-symbolic-ref.sh +++ b/t/t1401-symbolic-ref.sh @@ -33,7 +33,8 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' ' reset_to_sane test_expect_success 'symbolic-ref refuses bare sha1' ' - test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD) + rev=$(git rev-parse HEAD) && + test_must_fail git symbolic-ref HEAD "$rev" ' reset_to_sane diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 5841f280fb2..f1fe5d60677 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' ' echo content >>file && chmod +x file && printf "y\\ny\\n" | git add -p && - git diff --cached file | grep "new mode" && - git diff --cached file | grep "+content" && - test -z "$(git diff file)" + git diff --cached file >out && + grep "new mode" out && + grep "+content" out && + git diff file >out && + test_must_be_empty out ' # end of tests disabled when filemode is not usable diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh index f2ce14e9071..2d38a16480e 100755 --- a/t/t7516-commit-races.sh +++ b/t/t7516-commit-races.sh @@ -10,7 +10,8 @@ test_expect_success 'race to create orphan commit' ' test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e && git show -s --pretty=format:%s >subject && grep hare subject && - test -z "$(git show -s --pretty=format:%P)" + git show -s --pretty=format:%P >out && + test_must_be_empty out ' test_expect_success 'race to create non-orphan commit' '
Fix a few miscellaneous cases where: - We lost the "git" exit code via "git ... | grep" - Likewise by having a $(git) argument to git itself - Used "test -z" to check that a command emitted no output, we can use "test_must_be_empty" and &&-chaining instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t1401-symbolic-ref.sh | 3 ++- t/t3701-add-interactive.sh | 8 +++++--- t/t7516-commit-races.sh | 3 ++- 3 files changed, 9 insertions(+), 5 deletions(-)