| Message ID | 20210307132905.14212-1-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mktag tests: fix broken "&&" chain | expand |
On Sun, Mar 07, 2021 at 02:29:05PM +0100, Ævar Arnfjörð Bjarmason wrote: > Remove a stray "xb" I inadvertently introduced in 780aa0a21e0 (tests: > remove last uses of GIT_TEST_GETTEXT_POISON=false, 2021-02-11). This > would have been a failed attempt to type "C-x C-b" that snuck into the > code. > > The chainlint check did not catch this one, but I don't know where to > start patching the wall-of-sed that is chainlint.sed to fix that. Chain-lint only checks 'test_expect_{success,failure}' blocks, but this && chain is in a helper function. > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t3800-mktag.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh > index 60a666da595..6275c98523f 100755 > --- a/t/t3800-mktag.sh > +++ b/t/t3800-mktag.sh > @@ -17,7 +17,7 @@ check_verify_failure () { > grep '$2' message && > if test '$3' != '--no-strict' > then > - test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&xb > + test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict && > grep '$2' message.no-strict > fi > " > -- > 2.31.0.rc0.126.g04f22c5b82 >
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Remove a stray "xb" I inadvertently introduced in 780aa0a21e0 (tests: > remove last uses of GIT_TEST_GETTEXT_POISON=false, 2021-02-11). This > would have been a failed attempt to type "C-x C-b" that snuck into the > code. > > The chainlint check did not catch this one, but I don't know where to > start patching the wall-of-sed that is chainlint.sed to fix that. I do not think the chainlint check is designed to deal with helper functions, but I wonder why nobody noticed a runtime failure. Is this an unused/dead codepath? > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t3800-mktag.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh > index 60a666da595..6275c98523f 100755 > --- a/t/t3800-mktag.sh > +++ b/t/t3800-mktag.sh > @@ -17,7 +17,7 @@ check_verify_failure () { > grep '$2' message && > if test '$3' != '--no-strict' > then > - test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&xb > + test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict && > grep '$2' message.no-strict > fi > "
On Sun, Mar 7, 2021 at 3:43 PM Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > - test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&xb > > + test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict && > > grep '$2' message.no-strict > > I do not think the chainlint check is designed to deal with helper > functions, but I wonder why nobody noticed a runtime failure. Is > this an unused/dead codepath? The `git mktag` invocation would still have produced the expected stderr output regardless of what happens later (by "later", I mean the failing invocation of the non-existent `xb` command), which means that the subsequent `grep` will find what it's looking for, making the function succeed as expected. Someone looking at the verbose output might have had a chance of spotting the "xp: not found" error message, but there likely wasn't any reason to scan the verbose output since the test succeeded.
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh index 60a666da595..6275c98523f 100755 --- a/t/t3800-mktag.sh +++ b/t/t3800-mktag.sh @@ -17,7 +17,7 @@ check_verify_failure () { grep '$2' message && if test '$3' != '--no-strict' then - test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&xb + test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict && grep '$2' message.no-strict fi "
Remove a stray "xb" I inadvertently introduced in 780aa0a21e0 (tests: remove last uses of GIT_TEST_GETTEXT_POISON=false, 2021-02-11). This would have been a failed attempt to type "C-x C-b" that snuck into the code. The chainlint check did not catch this one, but I don't know where to start patching the wall-of-sed that is chainlint.sed to fix that. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t3800-mktag.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)