git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/8] tests: fix ignored & hidden exit codes
Date: Fri,  2 Dec 2022 01:06:26 +0100	[thread overview]
Message-ID: <cover-v2-0.8-00000000000-20221202T000227Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.6-00000000000-20220721T064349Z-avarab@gmail.com>

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.

The 1/8 here was originally submitted as part of another series, and
what prompted this re-submission at this time is that the exact same
fix was independently discovered in [2].

This is still just the tip of the iceberg in terms of these issues in
the test suite, but hopefully this will get some attention this time
around.

1. https://lore.kernel.org/git/xmqqmtd33e1h.fsf@gitster.g/
2. https://lore.kernel.org/git/c5b4d091-23c1-5a75-a255-99ec83973d8d@web.de/

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

 t/lib-httpd.sh                     |  3 +-
 t/lib-patch-mode.sh                |  9 +++--
 t/lib-submodule-update.sh          | 22 +++++-------
 t/t0001-init.sh                    |  4 +--
 t/t0002-gitfile.sh                 |  2 +-
 t/t0027-auto-crlf.sh               | 14 +++++---
 t/t0060-path-utils.sh              | 57 ++++++++++++++++--------------
 t/t0100-previous.sh                |  4 +--
 t/t1401-symbolic-ref.sh            |  3 +-
 t/t1504-ceiling-dirs.sh            |  6 ++--
 t/t2005-checkout-index-symlinks.sh |  2 +-
 t/t3200-branch.sh                  |  9 ++---
 t/t3701-add-interactive.sh         |  9 +++--
 t/t4023-diff-rename-typechange.sh  | 12 +++----
 t/t4205-log-pretty-formats.sh      |  2 +-
 t/t5522-pull-symlink.sh            |  2 +-
 t/t5605-clone-local.sh             |  9 +++--
 t/t7402-submodule-rebase.sh        | 10 +++---
 t/t7504-commit-msg-hook.sh         |  2 +-
 t/t7516-commit-races.sh            |  3 +-
 t/t7600-merge.sh                   |  9 +++--
 t/t7810-grep.sh                    |  2 +-
 t/test-lib-functions.sh            | 18 ++++++++++
 23 files changed, 119 insertions(+), 94 deletions(-)

Range-diff against v1:
6:  259b4618fcb = 1:  7c9f8d2830f log tests: don't use "exit 1" outside a sub-shell
3:  cfc1abbf7e3 = 2:  345a667d5bb auto-crlf tests: check "git checkout" exit code
1:  f8a382841d5 = 3:  c5feef1c808 diff tests: fix ignored exit codes in t4023
4:  df1b674b8a7 ! 4:  c36060934a6 test-lib-functions: add and use test_cmp_cmd
    @@ Commit message
                 test_cmp_cmd --stdin <some-cmd> <expect
                 test_cmp_cmd --ignore-stderr "output" <some-cmd>
     
    -    By using this in t0060-path.sh we'll catch cases where "git" or
    -    "test-tool" errors (such as segfaults or abort()) were previously
    -    hidden, and we'd either pass the test, or fail in some subsequent
    -    assertion.
    +    By using this in we'll catch cases where "git" or "test-tool"
    +    errors (such as segfaults or abort()) were previously hidden, and we'd
    +    either pass the test, or fail in some subsequent assertion.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    + ## t/lib-submodule-update.sh ##
    +@@ t/lib-submodule-update.sh: test_git_directory_exists () {
    + 	if test -f sub1/.git
    + 	then
    + 		# does core.worktree point at the right place?
    +-		test "$(git -C .git/modules/$1 config core.worktree)" = "../../../$1"
    ++		test_cmp_cmd "../../../$1" git -C ".git/modules/$1" config core.worktree
    + 	fi
    + }
    + 
    +
    + ## t/t0001-init.sh ##
    +@@ t/t0001-init.sh: test_expect_success 'invalid default branch name' '
    + test_expect_success 'branch -m with the initial branch' '
    + 	git init rename-initial &&
    + 	git -C rename-initial branch -m renamed &&
    +-	test renamed = $(git -C rename-initial symbolic-ref --short HEAD) &&
    ++	test_cmp_cmd renamed git -C rename-initial symbolic-ref --short HEAD &&
    + 	git -C rename-initial branch -m renamed again &&
    +-	test again = $(git -C rename-initial symbolic-ref --short HEAD)
    ++	test_cmp_cmd again git -C rename-initial symbolic-ref --short HEAD
    + '
    + 
    + test_done
    +
    + ## t/t0002-gitfile.sh ##
    +@@ t/t0002-gitfile.sh: test_expect_success 'bad setup: invalid .git file path' '
    + 
    + test_expect_success 'final setup + check rev-parse --git-dir' '
    + 	echo "gitdir: $REAL" >.git &&
    +-	test "$REAL" = "$(git rev-parse --git-dir)"
    ++	test_cmp_cmd "$REAL" git rev-parse --git-dir
    + '
    + 
    + test_expect_success 'check hash-object' '
    +
      ## t/t0060-path-utils.sh ##
     @@ t/t0060-path-utils.sh: TEST_PASSES_SANITIZE_LEAK=true
      
    @@ t/t0060-path-utils.sh: TEST_PASSES_SANITIZE_LEAK=true
     -	test_expect_success $3 "normalize path: $1 => $2" \
     -	"test \"\$(test-tool path-utils normalize_path_copy '$1')\" = '$expected'"
     +	test_expect_success $3 "normalize path: $1 => $2" "
    -+		echo '$expected' >expect &&
    -+		test-tool path-utils normalize_path_copy '$1' >actual &&
    -+		test_cmp expect actual
    ++		test_cmp_cmd '$expected' test-tool path-utils normalize_path_copy '$1'
     +	"
      }
      
      relative_path() {
    + 	expected=$(test-tool path-utils print_path "$3")
    +-	test_expect_success $4 "relative path: $1 $2 => $3" \
    +-	"test \"\$(test-tool path-utils relative_path '$1' '$2')\" = '$expected'"
    ++	test_expect_success $4 "relative path: $1 $2 => $3" "
    ++		test_cmp_cmd '$expected' test-tool path-utils relative_path '$1' '$2'
    ++	"
    + }
    + 
    + test_submodule_relative_url() {
    + 	test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" "
    +-		actual=\$(test-tool submodule resolve-relative-url '$1' '$2' '$3') &&
    +-		test \"\$actual\" = '$4'
    ++		test_cmp_cmd '$4' test-tool submodule resolve-relative-url '$1' '$2' '$3'
    + 	"
    + }
    + 
    +@@ t/t0060-path-utils.sh: ancestor() {
    + 		expected=$(($expected-$rootslash+$rootoff))
    + 		;;
    + 	esac
    +-	test_expect_success $4 "longest ancestor: $1 $2 => $expected" \
    +-	"actual=\$(test-tool path-utils longest_ancestor_length '$1' '$2') &&
    +-	 test \"\$actual\" = '$expected'"
    ++	test_expect_success $4 "longest ancestor: $1 $2 => $expected" "
    ++		test_cmp_cmd '$expected' test-tool path-utils longest_ancestor_length '$1' '$2'
    ++	"
    + }
    + 
    + # Some absolute path tests should be skipped on Windows due to path mangling
     @@ t/t0060-path-utils.sh: ancestor D:/Users/me C:/ -1 MINGW
      ancestor //server/share/my-directory //server/share/ 14 MINGW
      
    @@ t/t0060-path-utils.sh: test_expect_success 'prefix_path rejects absolute path to
      
      relative_path /foo/a/b/c/	/foo/a/b/	c/
     
    + ## t/t0100-previous.sh ##
    +@@ t/t0100-previous.sh: test_expect_success 'branch -d @{-1}' '
    + 	test_commit A &&
    + 	git checkout -b junk &&
    + 	git checkout - &&
    +-	test "$(git symbolic-ref HEAD)" = refs/heads/main &&
    ++	test_cmp_cmd refs/heads/main git symbolic-ref HEAD &&
    + 	git branch -d @{-1} &&
    + 	test_must_fail git rev-parse --verify refs/heads/junk
    + '
    +@@ t/t0100-previous.sh: test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
    + 	git reflog expire --expire=now &&
    + 	git checkout -b junk2 &&
    + 	git checkout - &&
    +-	test "$(git symbolic-ref HEAD)" = refs/heads/main &&
    ++	test_cmp_cmd refs/heads/main git symbolic-ref HEAD &&
    + 	test_must_fail git branch -d @{-12} &&
    + 	git rev-parse --verify refs/heads/main
    + '
    +
    + ## t/t1504-ceiling-dirs.sh ##
    +@@ t/t1504-ceiling-dirs.sh: TEST_PASSES_SANITIZE_LEAK=true
    + . ./test-lib.sh
    + 
    + test_prefix() {
    +-	test_expect_success "$1" \
    +-	"test '$2' = \"\$(git rev-parse --show-prefix)\""
    ++	local expect="$2" &&
    ++	test_expect_success "$1: git rev-parse --show-prefix is '$2'" '
    ++		test_cmp_cmd "$expect" git rev-parse --show-prefix
    ++	'
    + }
    + 
    + test_fail() {
    +
      ## t/test-lib-functions.sh ##
     @@ t/test-lib-functions.sh: test_cmp_rev () {
      	fi
2:  85c6ab40e91 ! 5:  f826a336c3d t/lib-patch-mode.sh: fix ignored "git" exit codes
    @@ Commit message
         t/lib-patch-mode.sh: fix ignored "git" exit codes
     
         Fix code added in b319ef70a94 (Add a small patch-mode testing library,
    -    2009-08-13) to use &&-chaining and test_cmp instead of interpolating
    -    "git" commands in a "test" statement.
    +    2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd"
    +    instead of interpolating "git" commands in a "test" statement.
     
         This fixes cases where we'd have e.g. missed memory leaks under
         SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
    @@ t/lib-patch-mode.sh: set_and_save_state () {
     +	cat "$1" >actual &&
     +	test_cmp expect actual &&
     +
    -+	echo "$3" >expect
    -+	git show :"$1" >actual &&
    -+	test_cmp expect actual
    ++	test_cmp_cmd "$3" git show :"$1"
      }
      
      # verify_saved_state <path>
    @@ t/lib-patch-mode.sh: save_head () {
      
      verify_saved_head () {
     -	test "$(cat _head)" = "$(git rev-parse HEAD)"
    -+	cat _head >expect &&
    -+	git rev-parse HEAD >actual &&
    -+	test_cmp expect actual
    ++	test_cmp_cmd "$(cat _head)" git rev-parse HEAD
      }
5:  563666f9426 = 6:  5715ff7f0f7 merge tests: don't ignore "rev-parse" exit code in helper
-:  ----------- > 7:  d090478aa84 tests: use "test_cmp_cmd" instead of "test" in sub-shells
-:  ----------- > 8:  979a7f003f8 tests: use "test_cmp_cmd" in misc tests
-- 
2.39.0.rc1.980.g92d3d4579ad


  parent reply	other threads:[~2022-12-02  0:07 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-21  6:51 [PATCH 0/6] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 1/6] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 2/6] t/lib-patch-mode.sh: fix ignored "git" exit codes Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 3/6] auto-crlf tests: check "git checkout" exit code Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 4/6] test-lib-functions: add and use test_cmp_cmd Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 5/6] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
2022-07-21  6:51 ` [PATCH 6/6] log tests: don't use "exit 1" outside a sub-shell Ævar Arnfjörð Bjarmason
2022-12-02  0:06 ` Ævar Arnfjörð Bjarmason [this message]
2022-12-02  0:06   ` [PATCH v2 1/8] " Ævar Arnfjörð Bjarmason
2022-12-02  1:02     ` Eric Sunshine
2022-12-02  1:48       ` Junio C Hamano
2022-12-02  2:45         ` Ævar Arnfjörð Bjarmason
2022-12-02  9:03           ` Eric Sunshine
2022-12-02 10:02             ` Ævar Arnfjörð Bjarmason
2022-12-07  6:09               ` Eric Sunshine
2022-12-02  3:24       ` Junio C Hamano
2022-12-02  0:06   ` [PATCH v2 2/8] auto-crlf tests: check "git checkout" exit code Ævar Arnfjörð Bjarmason
2022-12-02  1:02     ` René Scharfe
2022-12-02  1:10       ` Eric Sunshine
2022-12-02  5:59       ` Torsten Bögershausen
2022-12-02  6:03         ` Eric Sunshine
2022-12-02  0:06   ` [PATCH v2 3/8] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
2022-12-02  2:02     ` Junio C Hamano
2022-12-02  0:06   ` [PATCH v2 4/8] test-lib-functions: add and use test_cmp_cmd Ævar Arnfjörð Bjarmason
2022-12-02  1:30     ` René Scharfe
2022-12-02  1:33     ` Eric Sunshine
2022-12-02  1:45       ` Eric Sunshine
2022-12-02  1:52         ` Eric Sunshine
2022-12-02  3:41         ` Junio C Hamano
2022-12-02  0:06   ` [PATCH v2 5/8] t/lib-patch-mode.sh: fix ignored "git" exit codes Ævar Arnfjörð Bjarmason
2022-12-02  1:31     ` René Scharfe
2022-12-02  0:06   ` [PATCH v2 6/8] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
2022-12-02  1:41     ` René Scharfe
2022-12-02  0:06   ` [PATCH v2 7/8] tests: use "test_cmp_cmd" instead of "test" in sub-shells Ævar Arnfjörð Bjarmason
2022-12-02  0:06   ` [PATCH v2 8/8] tests: use "test_cmp_cmd" in misc tests Ævar Arnfjörð Bjarmason
2022-12-02  2:19     ` Junio C Hamano
2022-12-02 11:52   ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
2022-12-02 11:52     ` [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
2022-12-05  0:24       ` Junio C Hamano
2022-12-02 11:52     ` [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
2022-12-02 15:59       ` René Scharfe
2022-12-02 11:52     ` [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
2022-12-05  0:26       ` Junio C Hamano
2022-12-02 11:52     ` [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
2022-12-02 15:59       ` René Scharfe
2022-12-04  0:45       ` Eric Sunshine
2022-12-02 11:52     ` [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells Ævar Arnfjörð Bjarmason
2022-12-05  0:39       ` Junio C Hamano
2022-12-02 11:52     ` [PATCH v3 6/8] tests: don't lose 'test <str> = $(cmd ...)"' exit code Ævar Arnfjörð Bjarmason
2022-12-02 11:52     ` [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
2022-12-02 18:31       ` René Scharfe
2022-12-02 11:52     ` [PATCH v3 8/8] tests: don't lose mist "git" exit codes Ævar Arnfjörð Bjarmason
2022-12-04  0:40       ` Eric Sunshine
2022-12-05  0:45         ` Junio C Hamano
2022-12-19 10:19     ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
2022-12-19 10:19       ` [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
2022-12-19 12:07         ` René Scharfe
2022-12-19 10:19       ` [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
2022-12-20  0:09         ` Junio C Hamano
2022-12-27 16:40         ` Phillip Wood
2022-12-27 18:14           ` Ævar Arnfjörð Bjarmason
2022-12-19 10:19       ` [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
2022-12-20  0:20         ` Junio C Hamano
2022-12-19 10:19       ` [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
2022-12-26  1:14         ` Junio C Hamano
2022-12-19 10:19       ` [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
2022-12-26  1:18         ` Junio C Hamano
2022-12-27 16:44         ` Phillip Wood
2022-12-27 17:13           ` Phillip Wood
2022-12-27 23:16           ` Junio C Hamano
2022-12-19 10:19       ` [PATCH v4 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
2022-12-27 16:46         ` Phillip Wood
2022-12-27 18:18           ` Ævar Arnfjörð Bjarmason
2022-12-27 23:17           ` Junio C Hamano
2022-12-20  0:06       ` [PATCH v4 0/6] tests: fix ignored & hidden " Junio C Hamano
2023-02-06 22:44       ` [PATCH v5 " Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
2023-02-06 22:44         ` [PATCH v5 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
2023-02-06 23:33         ` [PATCH v5 0/6] tests: fix ignored & hidden " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover-v2-0.8-00000000000-20221202T000227Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).