mbox series

[0/6] tests: fix ignored & hidden exit codes

Message ID cover-0.6-00000000000-20220721T064349Z-avarab@gmail.com (mailing list archive)
Headers show
Series tests: fix ignored & hidden exit codes | expand

Message

Ævar Arnfjörð Bjarmason July 21, 2022, 6:51 a.m. UTC
A small set of fixes to correct git on the LHS of a pipe, and in $()
within a "test" expression, or where its exit code is otherwise
hidden.

This also includes a (reworded) version of a case where we exit'd out
of test-lib.sh itself, which Junio requested by spun off in:
https://lore.kernel.org/git/xmqqmtd33e1h.fsf@gitster.g/

As noted in [1] there's no need to rebase the other series on top of
this, it benefits from 6/6 here, but the two can proceed
independently.

This is still just the tip of the iceberg in terms of hidden exit
codes in the test suite, as can be seen with e.g.:

	git grep 'test.*\$\((git|test-tool)' -- 't/*.sh'

But these are all cases I've run into actual issues with, almost all
when testing with SANITIZE=leak. In most cases just with one of the
hunks in a given commit, but then I converted the rest of the file to
fix a similar bad pattern.

That doesn't mean that these are more important than e.g. the output
of the "grep" above, but we've got to start somewhere...

The range-diff below is to the tip of [2], to show how 6/6 was
reworded.

1. https://lore.kernel.org/git/cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (6):
  diff tests: fix ignored exit codes in t4023
  t/lib-patch-mode.sh: fix ignored "git" exit codes
  auto-crlf tests: check "git checkout" exit code
  test-lib-functions: add and use test_cmp_cmd
  merge tests: don't ignore "rev-parse" exit code in helper
  log tests: don't use "exit 1" outside a sub-shell

 t/lib-patch-mode.sh               | 13 ++++++---
 t/t0027-auto-crlf.sh              | 14 +++++++---
 t/t0060-path-utils.sh             | 45 +++++++++++++++++--------------
 t/t4023-diff-rename-typechange.sh | 12 ++++-----
 t/t4205-log-pretty-formats.sh     |  2 +-
 t/t7600-merge.sh                  |  9 +++----
 t/test-lib-functions.sh           | 18 +++++++++++++
 7 files changed, 74 insertions(+), 39 deletions(-)

Range-diff:
-:  ----------- > 1:  f8a382841d5 diff tests: fix ignored exit codes in t4023
-:  ----------- > 2:  85c6ab40e91 t/lib-patch-mode.sh: fix ignored "git" exit codes
-:  ----------- > 3:  cfc1abbf7e3 auto-crlf tests: check "git checkout" exit code
-:  ----------- > 4:  df1b674b8a7 test-lib-functions: add and use test_cmp_cmd
-:  ----------- > 5:  563666f9426 merge tests: don't ignore "rev-parse" exit code in helper
1:  9cedf0cb0e2 ! 6:  259b4618fcb log tests: don't use "exit 1" outside a sub-shell
    @@ Commit message
         Using "exit 1" outside a sub-shell will cause the test framework
         itself to exit on failure, which isn't what we want to do here.
     
    -    This issue was spotted with the new
    -    "GIT_TEST_PASSING_SANITIZE_LEAK=check" mode, i.e. that "git show"
    -    command leaks memory, and we'd thus "exit 1". Another implementation
    -    of "GIT_TEST_PASSING_SANITIZE_LEAK=check" or "--invert-exit-code"
    -    might have intercepted the "exit 1", and thus hidden the underlying
    -    issue here, but we correctly distinguish the two.
    +    This issue was spotted with the in-flight
    +    "GIT_TEST_PASSING_SANITIZE_LEAK=check" test mode[1]. This "git show"
    +    invocation currently leaks memory, and we'd thus "exit 1". This change
    +    was initially part of that topic[2] to demonstrate the correctness of
    +    the "check" implementation.
    +
    +    1. https://lore.kernel.org/git/patch-07.10-0961df2ab6c-20220719T205710Z-avarab@gmail.com/
    +    2. https://lore.kernel.org/git/patch-10.10-9cedf0cb0e2-20220719T205710Z-avarab@gmail.com/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>