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

Various fixes for "git" on the LHS of a pipe, but mostly when in "test" expressions like:

	test str = "$(git some-command)" &&

Changes since v3[1]:

 * The previous 1/8 and 3/8 were picked independently, and have landed
   on "master". I was waiting for those to graduate before a re-roll.
 * Add a missing test_expect_success to 1/6, as pointed out by René
   (thanks!).
 * Remove commit message mention of the now-dead test_cmp_cmd (thanks
   Eric!)
 * Reword commit messages, fix typos etc. (thanks Junio, and other
   reviewers!)

I think this should address all of the feedback on the v3, except Junio's suggestion of perhaps re-arranging this series around file boundaries.

Given the potential size of that range-diff I thought it was better to mosttly keep the same structure.

1. https://lore.kernel.org/git/cover-v3-0.8-00000000000-20221202T114733Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (6):
  auto-crlf tests: don't lose exit code in loops and outside tests
  t/lib-patch-mode.sh: fix ignored exit codes
  tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
  tests: don't lose exit status with "test <op> $(git ...)"
  tests: don't lose "git" exit codes in "! ( git ... | grep )"
  tests: don't lose misc "git" exit codes

 t/lib-httpd.sh                     |   5 +-
 t/lib-patch-mode.sh                |  11 ++-
 t/lib-submodule-update.sh          |  26 ++++---
 t/t0001-init.sh                    |   9 ++-
 t/t0002-gitfile.sh                 |   4 +-
 t/t0027-auto-crlf.sh               |  66 ++++++++++--------
 t/t0055-beyond-symlinks.sh         |  14 +++-
 t/t0060-path-utils.sh              | 107 +++++++++++++++++++++--------
 t/t0100-previous.sh                |   8 ++-
 t/t1401-symbolic-ref.sh            |   3 +-
 t/t1504-ceiling-dirs.sh            |   8 ++-
 t/t2005-checkout-index-symlinks.sh |   8 ++-
 t/t3200-branch.sh                  |  13 ++--
 t/t3700-add.sh                     |  18 +++--
 t/t3701-add-interactive.sh         |   8 ++-
 t/t5522-pull-symlink.sh            |   4 +-
 t/t5605-clone-local.sh             |  15 ++--
 t/t7402-submodule-rebase.sh        |  23 +++++--
 t/t7504-commit-msg-hook.sh         |   4 +-
 t/t7516-commit-races.sh            |   3 +-
 t/t7810-grep.sh                    |   4 +-
 21 files changed, 243 insertions(+), 118 deletions(-)

Range-diff against v3:
1:  64dfec31fb3 < -:  ----------- merge tests: don't ignore "rev-parse" exit code in helper
2:  394d5e46494 ! 1:  68d276dd421 auto-crlf tests: don't lose exit code in loops and outside tests
    @@ t/t0027-auto-crlf.sh: commit_MIX_chkwrn () {
      	test_expect_success "commit file with mixed EOL onto LF crlf=$crlf attr=$attr" '
      		check_warning "$lfwarn" ${pfx}_LF.err
     @@ t/t0027-auto-crlf.sh: checkout_files () {
    + 	lfmixcrlf=$1 ; shift
    + 	lfmixcr=$1 ; shift
    + 	crlfnul=$1 ; shift
    +-	create_gitattributes "$attr" $ident $aeol &&
    +-	git config core.autocrlf $crlf &&
    ++	test_expect_success "setup config for checkout attr=$attr ident=$ident aeol=$aeol core.autocrlf=$crlf" '
    ++		create_gitattributes "$attr" $ident $aeol &&
    ++		git config core.autocrlf $crlf
    ++	'
      	pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
      	for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
      	do
3:  4ec075689f6 < -:  ----------- diff tests: fix ignored exit codes in t4023
4:  c080899dd5f ! 2:  d351075f0ab t/lib-patch-mode.sh: fix ignored exit codes
    @@ Commit message
         t/lib-patch-mode.sh: fix ignored exit codes
     
         Fix code added in b319ef70a94 (Add a small patch-mode testing library,
    -    2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
    +    2009-08-13) to use &&-chaining.
    +
         This avoids losing both the exit code of a "git" and the "cat"
         processes.
     
    @@ Commit message
         SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
         discovered it while looking at leaks in related code.
     
    -    The "cat _head >expect" here is redundant, we could simply give
    -    "_head" to "test_cmp", but let's be consistent in using the "expect"
    -    and "actual" names for clarity.
    -
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/lib-patch-mode.sh ##
5:  58ac6fe5604 ! 3:  f5b2489609c tests: use "test_cmp" instead of "test" in sub-shells
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    tests: use "test_cmp" instead of "test" in sub-shells
    +    tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
     
    -    Convert a few cases where we were using "test" inside a sub-shell, and
    -    were losing the exit code of "git".
    +    Rewrite tests that ran "git" inside command substitution and lost the
    +    exit status of "git" so that we notice the failing "git".
     
    -    In the case of "t3200-branch.sh" some adjacent code outside of a
    -    sub-shell that was losing the exit code is also being converted, as
    -    it's within the same hunk.
    +    Have them use modern patterns such as a "test_cmp" of the expected
    +    outputs instead, and avoid needlessly spawning sub-shell in favor of
    +    using "git -C <dir>".
    +
    +    We'll fix more of these these in the subsequent commit, for now we're
    +    only converting the cases where this loss of exit code was combined
    +    with spawning a sub-shell. The one exception to that is the casein
    +    "t3200-branch.sh" where adjacent code didn't use a sub-shell, let's
    +    convert that here as it's within the same hunk.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
6:  51f32b42ce6 ! 4:  da66e5bf1c1 tests: don't lose 'test <str> = $(cmd ...)"' exit code
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    tests: don't lose 'test <str> = $(cmd ...)"' exit code
    +    tests: don't lose exit status with "test <op> $(git ...)"
     
    -    Convert some cases in the test suite where we'd lose the exit code of
    -    a command being interpolated as one of the arguments to the "test"
    -    builtin function to use &&-chaining and "test_cmp" instead.
    -
    -    This way we won't lose the exit code, and the failure output will be
    -    more helpful.
    +    As with the preceding commit, rewrite tests that ran "git" inside
    +    command substitution and lost the exit status of "git" so that we
    +    notice the failing "git". This time around we're converting cases that
    +    didn't involve a containing sub-shell around the command substitution.
     
         In the case of "t0060-path-utils.sh" and
         "t2005-checkout-index-symlinks.sh" convert the relevant code to using
7:  307f25db831 ! 5:  9596702978e tests: don't lose "git" exit codes in "! ( git ... | grep )"
    @@ Commit message
         - In the case of "t0055-beyond-symlinks.sh" compare against the
           expected output instead.
     
    +      We could use the same pattern as in the "t3700-add.sh" below, doing
    +      so would have the advantage that if we added an earlier test we
    +      wouldn't need to adjust the "expect" output.
    +
    +      But as "t0055-beyond-symlinks.sh" is a small and focused test (less
    +      than 40 lines in total) let's use "test_cmp" instead.
    +
         - For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
    -      and use "test_must_be_empty" to assert that it's not there.
    +      and use "test_must_be_empty" to assert that it's not there. If we used
    +      "grep" we'd get a non-zero exit code.
    +
    +      We could use "test_expect_code 1 grep", but this is more consistent
    +      with existing patterns in the test suite.
     
           We can also remove a repeated invocation of "git ls-files" for the
           last test that's being modified in that file, and search the
8:  37c75f4a097 ! 6:  94df7a1764e tests: don't lose mist "git" exit codes
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    tests: don't lose mist "git" exit codes
    +    tests: don't lose misc "git" exit codes
     
         Fix a few miscellaneous cases where: