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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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

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

Changes since v4[1]:

 * Tried to address all outstanding concernse, some with commit
   message clarifications, others with code changes.
 * The main code change here is to avoid converting code adjacent to
   the fixed "test" code away from sub-shell use, per Junio's request.

Branch & CI for this at:
https://github.com/avar/git/tree/avar/fix-even-more-broken-test-code-hiding-failures-5

1. https://lore.kernel.org/git/cover-v4-0.6-00000000000-20221219T101240Z-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                     |   8 ++-
 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              | 108 +++++++++++++++++++++--------
 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                  |   8 ++-
 t/t3700-add.sh                     |  18 +++--
 t/t3701-add-interactive.sh         |   8 ++-
 t/t5522-pull-symlink.sh            |   4 +-
 t/t5605-clone-local.sh             |  12 +++-
 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, 245 insertions(+), 112 deletions(-)

Range-diff against v4:
1:  68d276dd421 ! 1:  66d0f91e6aa auto-crlf tests: don't lose exit code in loops and outside tests
    @@ Commit message
         this failure under SANITIZE=leak when these invocations had errored
         out, even under "--immediate".
     
    +    For checkout_files() we could run one test_expect_success() instead of
    +    the 5 we run now in a loop.
    +
    +    But as this function added in [1] is already taking pains to split up
    +    its setup into phases (there are 5 more "test_expect_success()" at the
    +    end of it already, see [1]), let's follow that existing convention.
    +
    +    1. 343151dcbdf (t0027: combinations of core.autocrlf, core.eol and text, 2014-06-29)
    +
         Helped-by: René Scharfe <l.s.r@web.de>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
2:  d351075f0ab ! 2:  97f8b6a7b86 t/lib-patch-mode.sh: fix ignored exit codes
    @@ 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.
     
    +    For "verify_saved_head()" we could make use of "test_cmp_rev" with
    +    some changes, but it uses "git rev-parse --verify", and this existing
    +    test does not. I think it could safely use it, but let's avoid the
    +    while-at-it change, and narrowly fix the exit code problem.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/lib-patch-mode.sh ##
3:  f5b2489609c ! 3:  d8f4c4a6d9e tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
    @@ Commit message
         exit status of "git" so that we notice the failing "git".
     
         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>".
    +    outputs instead.
     
         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.
    +    with spawning a sub-shell.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ t/lib-httpd.sh: test_http_push_nonff () {
      		test_must_fail git push -v origin >output 2>&1 &&
     -		(cd "$REMOTE_REPO" &&
     -		 test $HEAD = $(git rev-parse --verify HEAD))
    -+		echo "$HEAD" >expect &&
    -+		git -C "$REMOTE_REPO" rev-parse --verify HEAD >actual &&
    -+		test_cmp expect actual
    ++		(
    ++			cd "$REMOTE_REPO" &&
    ++			echo "$HEAD" >expect &&
    ++			git rev-parse --verify HEAD >actual &&
    ++			test_cmp expect actual
    ++		)
      	'
      
      	test_expect_success 'non-fast-forward push show ref status' '
    @@ t/t0060-path-utils.sh: test_expect_success 'prefix_path rejects absolute path to
      	ln -s repo repolink &&
     -	test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
     +	echo "a" >expect &&
    -+	test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
    ++	repo_path="$(cd repo && pwd)" &&
    ++	test-tool -C repo path-utils prefix_path prefix "$repo_path/../repolink/a" >actual &&
     +	test_cmp expect actual
      '
      
      relative_path /foo/a/b/c/	/foo/a/b/	c/
     
      ## t/t3200-branch.sh ##
    -@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
    - test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
    - 	git checkout -b baz &&
    - 	git worktree add -f bazdir baz &&
    --	(
    --		cd bazdir &&
    --		git branch -M baz bam &&
    +@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed within a worktree in w
    + 	(
    + 		cd bazdir &&
    + 		git branch -M baz bam &&
     -		test $(git rev-parse --abbrev-ref HEAD) = bam
    --	) &&
    ++		echo bam >expect &&
    ++		git rev-parse --abbrev-ref HEAD >actual &&
    ++		test_cmp expect actual
    + 	) &&
     -	test $(git rev-parse --abbrev-ref HEAD) = bam &&
    -+	git -C "$bazdir" branch -M baz bam &&
    -+	echo "bam" >expect &&
    -+	git -C "$bazdir" rev-parse --abbrev-ref HEAD >actual &&
    -+	test_cmp expect actual &&
    -+	echo "bam" >expect &&
    ++	echo bam >expect &&
     +	git rev-parse --abbrev-ref HEAD >actual &&
     +	test_cmp expect actual &&
      	rm -r bazdir &&
    @@ t/t5605-clone-local.sh: test_expect_success 'preparing origin repository' '
      	git bundle create b2.bundle main &&
      	mkdir dir &&
     @@ t/t5605-clone-local.sh: test_expect_success 'preparing origin repository' '
    - 
      test_expect_success 'local clone without .git suffix' '
      	git clone -l -s a b &&
    --	(cd b &&
    + 	(cd b &&
     -	test "$(git config --bool core.bare)" = false &&
    --	git fetch)
     +	echo false >expect &&
    -+	git -C b config --bool core.bare >actual &&
    ++	git config --bool core.bare >actual &&
     +	test_cmp expect actual &&
    -+	git -C b fetch
    + 	git fetch)
      '
      
    - test_expect_success 'local clone with .git suffix' '
     
      ## t/t7402-submodule-rebase.sh ##
     @@ t/t7402-submodule-rebase.sh: test_expect_success 'stash with a dirty submodule' '
4:  da66e5bf1c1 = 4:  e6f56478b65 tests: don't lose exit status with "test <op> $(git ...)"
5:  9596702978e = 5:  55abdbef49f tests: don't lose "git" exit codes in "! ( git ... | grep )"
6:  94df7a1764e = 6:  e7f10c0641a tests: don't lose misc "git" exit codes
-- 
2.39.1.1425.gac85d95d48c