git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 00/15] tests: don't ignore "git" exit codes

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 7, 2022, 12:48 UTC
Message-ID
<cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com>
In-Reply-To
<cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com>

This series fixes issues where we ignored the exit code of "git" due to it being on the LHS of a pipe, or because we interpolated its output with $() in a "test" construct, or had missing &&- chains in helper functions etc.

As noted in v1 (https://lore.kernel.org/git/cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com/) this is not a general search/replacement, but fixes real issues we run into with SANITIZE=leak.

Changes since v1:
 * Minor commit message improvement for 11/15
 * Added a comment for some non-obvious prereq setup code, as
   suggested by Junio.
Ævar Arnfjörð Bjarmason (15):
  tests: change some 'test $(git) = "x"' to test_cmp
  tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"
  read-tree tests: check "diff-files" exit code on failure
  diff tests: don't ignore "git diff" exit code
  diff tests: don't ignore "git diff" exit code in "read" loop
  apply tests: use "test_must_fail" instead of ad-hoc pattern
  merge tests: use "test_must_fail" instead of ad-hoc pattern
  rev-parse tests: don't ignore "git reflog" exit code
  notes tests: don't ignore "git" exit code
  diff tests: don't ignore "git rev-list" exit code
  rev-list tests: don't hide abort() in "test_expect_failure"
  gettext tests: don't ignore "test-tool regex" exit code
  apply tests: don't ignore "git ls-files" exit code, drop sub-shell
  checkout tests: don't ignore "git <cmd>" exit code
  rev-list simplify tests: don't ignore "git" exit code
 t/t0002-gitfile.sh                     |   6 +-
 t/t1001-read-tree-m-2way.sh            |   6 +-
 t/t1002-read-tree-m-u-2way.sh          |   6 +-
 t/t1503-rev-parse-verify.sh            |   5 +-
 t/t2012-checkout-last.sh               |  51 ++++++---
 t/t2200-add-update.sh                  |  33 ++++--
 t/t3302-notes-index-expensive.sh       |   6 +-
 t/t3303-notes-subtrees.sh              |   9 +-
 t/t3305-notes-fanout.sh                |  14 +--
 t/t4020-diff-external.sh               | 153 ++++++++++++-------------
 t/t4027-diff-submodule.sh              |   7 +-
 t/t4123-apply-shrink.sh                |  18 +--
 t/t4128-apply-root.sh                  |  36 +++---
 t/t6005-rev-list-count.sh              |  43 ++++---
 t/t6012-rev-list-simplify.sh           |  12 +-
 t/t6102-rev-list-unexpected-objects.sh |  13 ++-
 t/t6407-merge-binary.sh                |  22 +---
 t/t7103-reset-bare.sh                  |   7 +-
 t/t7812-grep-icase-non-ascii.sh        |  16 ++-
 19 files changed, 245 insertions(+), 218 deletions(-)
Range-diff against v1:
 1:  78b9c52551f =  1:  78b9c52551f tests: change some 'test $(git) = "x"' to test_cmp
 2:  e1105b029d6 =  2:  e1105b029d6 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"
 3:  5f02e30d1ab =  3:  5f02e30d1ab read-tree tests: check "diff-files" exit code on failure
 4:  a425ced5609 =  4:  a425ced5609 diff tests: don't ignore "git diff" exit code
 5:  b1aeac3f68e =  5:  b1aeac3f68e diff tests: don't ignore "git diff" exit code in "read" loop
 6:  7952ae1f3b5 =  6:  7952ae1f3b5 apply tests: use "test_must_fail" instead of ad-hoc pattern
 7:  276be19e35e =  7:  276be19e35e merge tests: use "test_must_fail" instead of ad-hoc pattern
 8:  dca2ac3a171 =  8:  dca2ac3a171 rev-parse tests: don't ignore "git reflog" exit code
 9:  ca9e12f2bac =  9:  ca9e12f2bac notes tests: don't ignore "git" exit code
10:  946397033d4 = 10:  946397033d4 diff tests: don't ignore "git rev-list" exit code
11:  26c838f0560 ! 11:  52397b3575a rev-list tests: don't hide abort() in "test_expect_failure"
    @@ Commit message
         "test_expect_success", but would allow us say "test_todo" here (and
         "success" would emit a "not ok [...] # TODO", not "ok [...]".
     
    -    In lieu of those larger changes let's more narrowly fix the problem at
    +    So even though using "test_expect_success" here comes with its own
    +    problems[2], let's use it as a narrow change to fix the problem at
         hand here and stop conflating the current "success" with actual
         SANITIZE=leak failures.
     
         1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/
    +    2. https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
12:  f3cc5bc7eb9 ! 12:  552dcac705d gettext tests: don't ignore "test-tool regex" exit code
    @@ t/t7812-grep-icase-non-ascii.sh: test_expect_success GETTEXT_LOCALE 'setup' '
     -test-tool regex "HALLÓ" "Halló" ICASE &&
     -test_set_prereq REGEX_LOCALE
     +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '
    ++	# This "test-tool" invocation is identical...
     +	if test-tool regex "HALLÓ" "Halló" ICASE
     +	then
     +		test_set_prereq REGEX_LOCALE
     +	else
    ++
    ++		# ... to this one, but this way "test_must_fail" will
    ++		# tell a segfault or abort() from the regexec() test
    ++		# itself
     +		test_must_fail test-tool regex "HALLÓ" "Halló" ICASE
     +	fi
     +'
13:  834809b1b8a = 13:  dbe8d168401 apply tests: don't ignore "git ls-files" exit code, drop sub-shell
14:  34cada14fec = 14:  22b81d7f93a checkout tests: don't ignore "git <cmd>" exit code
15:  4ee216711cf = 15:  16889ed154f rev-list simplify tests: don't ignore "git" exit code
-- 
2.35.1.1242.gfeba0eae32b
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 33 of 48 in “tests: don't ignore "git" exit codes”
  1. 00/15 tests: don't ignore "git" exit codesÆvar Arnfjörð Bjarmason, Mar 2, 2022
  2. 07/15 merge tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 2, 2022
  3. 08/15 rev-parse tests: don't ignore "git reflog" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  4. 02/15 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  5. 05/15 diff tests: don't ignore "git diff" exit code in "read" loopÆvar Arnfjörð Bjarmason, Mar 2, 2022
  6. 01/15 tests: change some 'test $(git) = "x"' to test_cmpÆvar Arnfjörð Bjarmason, Mar 2, 2022
  7. 12/15 gettext tests: don't ignore "test-tool regex" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  8. Junio C HamanoMar 2, 2022
  9. Ævar Arnfjörð BjarmasonMar 3, 2022
  10. Junio C HamanoMar 3, 2022
  11. 13/15 apply tests: don't ignore "git ls-files" exit code, drop sub-shellÆvar Arnfjörð Bjarmason, Mar 2, 2022
  12. 14/15 checkout tests: don't ignore "git <cmd>" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  13. 15/15 rev-list simplify tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  14. 11/15 rev-list tests: don't hide abort() in "test_expect_failure"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  15. Junio C HamanoMar 2, 2022
  16. Ævar Arnfjörð BjarmasonMar 3, 2022
  17. 10/15 diff tests: don't ignore "git rev-list" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  18. 09/15 notes tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  19. 03/15 read-tree tests: check "diff-files" exit code on failureÆvar Arnfjörð Bjarmason, Mar 2, 2022
  20. Junio C HamanoMar 2, 2022
  21. 06/15 apply tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 2, 2022
  22. Junio C HamanoMar 2, 2022
  23. 04/15 diff tests: don't ignore "git diff" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  24. Junio C HamanoMar 2, 2022
  25. Derrick StoleeMar 3, 2022
  26. Ævar Arnfjörð BjarmasonMar 3, 2022
  27. Phillip WoodMar 3, 2022
  28. Ævar Arnfjörð BjarmasonMar 3, 2022
  29. Junio C HamanoMar 4, 2022
  30. Ævar Arnfjörð BjarmasonMar 4, 2022
  31. Shubham MishraMar 3, 2022
  32. Ævar Arnfjörð BjarmasonMar 3, 2022
  33. 00/15 tests: don't ignore "git" exit codesÆvar Arnfjörð Bjarmason, Mar 7, 2022
  34. 01/15 tests: change some 'test $(git) = "x"' to test_cmpÆvar Arnfjörð Bjarmason, Mar 7, 2022
  35. 02/15 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  36. 03/15 read-tree tests: check "diff-files" exit code on failureÆvar Arnfjörð Bjarmason, Mar 7, 2022
  37. 04/15 diff tests: don't ignore "git diff" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  38. 05/15 diff tests: don't ignore "git diff" exit code in "read" loopÆvar Arnfjörð Bjarmason, Mar 7, 2022
  39. 06/15 apply tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 7, 2022
  40. 07/15 merge tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 7, 2022
  41. 08/15 rev-parse tests: don't ignore "git reflog" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  42. 09/15 notes tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  43. 10/15 diff tests: don't ignore "git rev-list" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  44. 12/15 gettext tests: don't ignore "test-tool regex" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  45. 13/15 apply tests: don't ignore "git ls-files" exit code, drop sub-shellÆvar Arnfjörð Bjarmason, Mar 7, 2022
  46. 14/15 checkout tests: don't ignore "git <cmd>" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  47. 11/15 rev-list tests: don't hide abort() in "test_expect_failure"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  48. 15/15 rev-list simplify tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.