Subject: [PATCH v2 00/11] tests: add a bash "set -o pipefail" test mode

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 16 Jan 2021 16:35:43 +0100

An unstated aim of v1 of this series was to fix up the tests on
vanilla bash's "set -o pipefail" enough that the test suite would have
some failures, but wouldn't look like a complete dumpster fire.

But this was confusing and relied on a side-quest to change the
test_{must,might}_fail helpers. See
https://lore.kernel.org/git/[email-masked]/

I've now ejected all of that, in favor of just fixing some of the
tests instead as Jeff suggested. Jeff, I added your Signed-off-by to
06/11 which you're mostly the author of. Please Ack that you're OK
with that (the original diff-for-discussin didn't have a SOB).

Jeff King (1):
  git-svn tests: rewrite brittle tests to use "--[no-]merges".

Ævar Arnfjörð Bjarmason (10):
  cache-tree tests: remove unused $2 parameter
  cache-tree tests: use a sub-shell with less indirection
  cache-tree tests: refactor overly complex function
  git svn mergeinfo tests: modernize redirection & quoting style
  git svn mergeinfo tests: refactor "test -z" to use test_must_be_empty
  rm tests: actually test for SIGPIPE in SIGPIPE test
  upload-pack tests: avoid a non-zero "grep" exit status
  archive tests: use a cheaper "zipinfo -h" invocation to get header
  tests: split up bash detection library
  tests: add a "set -o pipefail" for a patched bash

 t/README                           |  5 ++++
 t/lib-bash-detection.sh            |  8 ++++++
 t/lib-bash.sh                      |  4 ++-
 t/t0000-basic.sh                   |  4 +++
 t/t0005-signals.sh                 |  4 +--
 t/t0090-cache-tree.sh              | 31 +++++++--------------
 t/t3600-rm.sh                      |  7 +++--
 t/t5000-tar-tree.sh                |  2 +-
 t/t5004-archive-corner-cases.sh    |  3 ++-
 t/t5703-upload-pack-ref-in-want.sh |  6 ++++-
 t/t9151-svn-mergeinfo.sh           | 43 ++++++++++++++----------------
 t/t9902-completion.sh              |  5 ++++
 t/test-lib.sh                      | 29 ++++++++++++++++++++
 13 files changed, 99 insertions(+), 52 deletions(-)
 create mode 100644 t/lib-bash-detection.sh

Range-diff:
 1:  d950fbb967 <  -:  ---------- test-lib: add tests for test_might_fail
 2:  1a0ffb1159 <  -:  ---------- test-lib: add ok=* support to test_might_fail
 3:  f7eaceeb3e <  -:  ---------- test_lib: allow test_{must,might}_fail to accept non-git on "sigpipe"
 4:  0e77779947 <  -:  ---------- tests: use "test_might_fail ok=sigpipe grep" when appropriate
 -:  ---------- >  1:  8e8e03fa3d cache-tree tests: remove unused $2 parameter
 -:  ---------- >  2:  828d25533c cache-tree tests: use a sub-shell with less indirection
 -:  ---------- >  3:  fefdc570a5 cache-tree tests: refactor overly complex function
 -:  ---------- >  4:  a16938e58d git svn mergeinfo tests: modernize redirection & quoting style
 -:  ---------- >  5:  b520656240 git svn mergeinfo tests: refactor "test -z" to use test_must_be_empty
 -:  ---------- >  6:  f2e70ac911 git-svn tests: rewrite brittle tests to use "--[no-]merges".
 -:  ---------- >  7:  dcf001e165 rm tests: actually test for SIGPIPE in SIGPIPE test
 -:  ---------- >  8:  2212fa65eb upload-pack tests: avoid a non-zero "grep" exit status
 -:  ---------- >  9:  8167c2e346 archive tests: use a cheaper "zipinfo -h" invocation to get header
 5:  c3916b8e7b = 10:  30c454ae7c tests: split up bash detection library
 6:  4a988d1c73 ! 11:  6f290f850c tests: add a "set -o pipefail" for a patched bash
    @@ Commit message
         wind with current bash semantics of failing on SIGPIPE.
     
         This series relies on a patch of mine to bash, which I'm submitting
    -    upstream. Vanilla bash ignores SIGPIPE under "set -e" since version
    -    3.1. It's only under "set -o pipefail" (added in 3.2) that it doesn't
    -    take account of SIGPIPE, in a seeming omission nobody bothered to fix
    -    yet.
    +    upstream, while not breaking anything for vanilla bash users. They
    +    won't have GIT_TEST_PIPEFAIL turned on for them, and will only get
    +    breakages if they turn it on explicitly with "GIT_TEST_PIPEFAIL=true".
    +
    +    Vanilla bash ignores SIGPIPE under "set -e" since version 3.1. It's
    +    only under "set -o pipefail" (added in 3.2) that it doesn't take
    +    account of SIGPIPE, in a seeming omission nobody bothered to fix yet.
     
         Patching bash[4] with:
     
    @@ Commit message
                 }
                    while (p != jobs[job]->pipe);
     
    -    Makes it useful for something like the git test suite. With vanilla
    -    bash and GIT_TEST_PIPEFAIL=true we'll fail 4 tests in my one-off test.
    +    Makes it useful for something like the git test suite.
     
    -    With my patched bash the only tests we need to skip are those that are
    -    explicitly testing that a piped command returned SIGPIPE.
    +    Under this test mode we only tests we need to skip those tests which
    +    are explicitly testing that a piped command returned SIGPIPE. Those
    +    tests will now return 0 instead of an exit code indicating SIGPIPE.
     
    -    As Jeff noted in [3] that count isn't reliable, as more will fail in a
    -    way that's hard to reproduce due to the racy nature of vanilla "set -o
    -    pipefail"
    +    Forcing the mode to run under vanilla bash with
    +    "GIT_TEST_PIPEFAIL=true" doesn't fail any tests for me, except the
    +    test in t0000-basic.sh which explicitly checks for the desired
    +    pipefail semantics. However, as Jeff noted in [3] that absence of
    +    failure isn't reliable. I might not see some of the failures due to
    +    the racy nature of how vanilla "set -o pipefail" interacts with *nix
    +    pipe semantics.
     
         1. a378fee5b0 (Documentation: add shell guidelines, 2018-10-05)
         2. https://lore.kernel.org/git/[email-masked]/
    @@ t/README: GIT_TEST_DEFAULT_HASH=<hash-algo> specifies which hash algorithm to
     +GIT_TEST_PIPEFAIL=<boolean>, when true, run 'set -o pipefail' to catch
     +failures in commands that aren't the last in a pipe. Defaults to true
     +on bash versions which know how to ignore SIGPIPE failures under the
    -+'set -o pipefail' mode (as of 2021-01-14 only in an out-of-tree patch
    -+to bash).
    ++'set -o pipefail' mode.
     +
      Naming Tests
      ------------

      ## t/t0000-basic.sh ##
    -@@ t/t0000-basic.sh: test_expect_success 'test_{must,might}_fail accept non-git on "sigpipe"' '
    - 	test_cmp badobjects out
    +@@ t/t0000-basic.sh: test_expect_success 'test_must_fail rejects a non-git command with env' '
    + 	grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
      '
      
    -+test_expect_failure BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
    -+	grep string </dev/null | true
    -+'
    -+
    -+test_expect_failure BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
    -+	test_must_fail grep string </dev/null | true &&
    -+	test_might_fail grep string </dev/null | true
    -+'
    -+
    -+test_expect_success BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
    -+	test_must_fail ok=sigpipe grep string </dev/null | true &&
    -+	test_might_fail ok=sigpipe grep string </dev/null | true
    ++test_expect_success BASH_SET_O_PIPEFAIL 'our bash under "set -o pipefail" mode ignores SIGPIPE failures' '
    ++	yes | head -n 1 | true
     +'
     +
      test_done
    @@ t/t0005-signals.sh: test_expect_success 'create blob' '
      	test_match_signal 13 "$OUT"
      '
     
    + ## t/t3600-rm.sh ##
    +@@ t/t3600-rm.sh: test_expect_success 'choking "git rm" should not let it die with cruft' '
    + 		i=$(( $i + 1 ))
    + 	done | git update-index --index-info &&
    + 	OUT=$( ((trap "" PIPE; git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) &&
    +-	test_match_signal 13 "$OUT" &&
    ++	if ! test_have_prereq BASH_SET_O_PIPEFAIL
    ++	then
    ++		test_match_signal 13 "$OUT"
    ++	fi &&
    + 	test_path_is_missing .git/index.lock
    + '
    + 
    +
      ## t/t5000-tar-tree.sh ##
     @@ t/t5000-tar-tree.sh: test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '
      
-- 
2.29.2.222.g5d2a92d10f8