git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3)
@ 2020-03-25  5:54 Denton Liu
  2020-03-25  5:54 ` [PATCH 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
                   ` (9 more replies)
  0 siblings, 10 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

The overall scope of these patches is to replace inappropriate uses of
test_must_fail. IOW, we should only allow test_must_fail to run on `git`
and `test-tool`. Ultimately, we will conclude by making test_must_fail
error out on non-git commands. An advance view of the final series can
be found here[1].

This is the third part. It focuses on t5*.sh.

The first part can be found here[2]. The second part can be found here[3].

[1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2
[2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/
[3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/

Denton Liu (8):
  t5512: don't use `test_must_fail test_cmp`
  t5512: generate references with generate_references()
  t5512: stop losing return codes of git commands
  t5550: remove use of `test_might_fail grep`
  t5607: reorder `nongit test_must_fail`
  t5612: don't use `test_must_fail test_cmp`
  t5612: stop losing return codes of git commands
  t5801: teach compare_refs() to accept !

 t/t5512-ls-remote.sh       | 66 +++++++++++++++++---------------------
 t/t5550-http-fetch-dumb.sh |  2 +-
 t/t5607-clone-bundle.sh    |  2 +-
 t/t5612-clone-refspec.sh   | 26 +++++++--------
 t/t5801-remote-helpers.sh  | 10 ++++--
 5 files changed, 52 insertions(+), 54 deletions(-)

-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/8] t5512: don't use `test_must_fail test_cmp`
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  5:54 ` [PATCH 2/8] t5512: generate references with generate_references() Denton Liu
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since test_cmp() just
wraps an external command, replace `test_must_fail test_cmp` with
`! test_cmp`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 04b35402c7..08b98f12b8 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -92,7 +92,7 @@ test_expect_success 'use "origin" when no remote specified' '
 
 test_expect_success 'suppress "From <url>" with -q' '
 	git ls-remote -q 2>actual_err &&
-	test_must_fail test_cmp exp_err actual_err
+	! test_cmp exp_err actual_err
 '
 
 test_expect_success 'use branch.<name>.remote if possible' '
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/8] t5512: generate references with generate_references()
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
  2020-03-25  5:54 ` [PATCH 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  6:08   ` Eric Sunshine
  2020-03-25  6:41   ` Junio C Hamano
  2020-03-25  5:54 ` [PATCH 3/8] t5512: stop losing return codes of git commands Denton Liu
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

The expected references are generated using a here-doc with some inline
subshells. If one of the `git rev-parse` invocations within the
subshells failed, its return code is swallowed and we won't know about
it. Replace these here-docs with generate_references(), which actually
reports when `git rev-parse` fails.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 50 +++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 08b98f12b8..62d02152c7 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -4,6 +4,14 @@ test_description='git ls-remote'
 
 . ./test-lib.sh
 
+generate_references () {
+	for i
+	do
+		oid=$(git rev-parse "$i") || return 1
+		printf '%s\t%s\n' "$oid" "$i"
+	done
+}
+
 test_expect_success setup '
 	>file &&
 	git add file &&
@@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '
 '
 
 test_expect_success 'ls-remote --sort="version:refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark)	refs/tags/mark
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	EOF
+	generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&
 	git ls-remote --sort="version:refname" --tags self >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-remote --sort="-version:refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark)	refs/tags/mark
-	EOF
+	generate_references refs/tags/mark1.10 refs/tags/mark1.2 refs/tags/mark1.1 refs/tags/mark >expect &&
 	git ls-remote --sort="-version:refname" --tags self >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-remote --sort="-refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark)	refs/tags/mark
-	EOF
+	generate_references refs/tags/mark1.2 refs/tags/mark1.10 refs/tags/mark1.1 refs/tags/mark >expect &&
 	git ls-remote --sort="-refname" --tags self >actual &&
 	test_cmp expect actual
 '
@@ -212,17 +205,16 @@ test_expect_success 'protocol v2 supports hiderefs' '
 
 test_expect_success 'ls-remote --symref' '
 	git fetch origin &&
-	cat >expect <<-EOF &&
-	ref: refs/heads/master	HEAD
-	$(git rev-parse HEAD)	HEAD
-	$(git rev-parse refs/heads/master)	refs/heads/master
-	$(git rev-parse HEAD)	refs/remotes/origin/HEAD
-	$(git rev-parse refs/remotes/origin/master)	refs/remotes/origin/master
-	$(git rev-parse refs/tags/mark)	refs/tags/mark
-	$(git rev-parse refs/tags/mark1.1)	refs/tags/mark1.1
-	$(git rev-parse refs/tags/mark1.10)	refs/tags/mark1.10
-	$(git rev-parse refs/tags/mark1.2)	refs/tags/mark1.2
-	EOF
+	echo "ref: refs/heads/master	HEAD" >expect &&
+	generate_references HEAD \
+		refs/heads/master >>expect &&
+	oid=$(git rev-parse HEAD) &&
+	echo "$oid	refs/remotes/origin/HEAD" >>expect &&
+	generate_references refs/remotes/origin/master \
+		refs/tags/mark \
+		refs/tags/mark1.1 \
+		refs/tags/mark1.10 \
+		refs/tags/mark1.2 >>expect &&
 	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
 	# protocol v0 here.
 	GIT_TEST_PROTOCOL_VERSION=0 git ls-remote --symref >actual &&
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/8] t5512: stop losing return codes of git commands
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
  2020-03-25  5:54 ` [PATCH 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
  2020-03-25  5:54 ` [PATCH 2/8] t5512: generate references with generate_references() Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  5:54 ` [PATCH 4/8] t5550: remove use of `test_might_fail grep` Denton Liu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

In a pipe, only the return code of the last command is used. Thus, all
other commands will have their return codes masked. Rewrite pipes so
that there are no git commands upstream so that their failure is
reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 62d02152c7..fea64eb5c2 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -21,11 +21,11 @@ test_expect_success setup '
 	git tag mark1.1 &&
 	git tag mark1.2 &&
 	git tag mark1.10 &&
-	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
-	(
-		echo "$(git rev-parse HEAD)	HEAD" &&
-		git show-ref -d	| sed -e "s/ /	/"
-	) >expected.all &&
+	git show-ref --tags -d >expected.tag.raw &&
+	sed -e "s/ /	/" expected.tag.raw >expected.tag &&
+	generate_references HEAD >expected.all &&
+	git show-ref -d	>refs &&
+	sed -e "s/ /	/" refs >>expected.all &&
 
 	git remote add self "$(pwd)/.git"
 '
@@ -173,8 +173,8 @@ do
 		test_config $configsection.hiderefs refs/tags &&
 		git ls-remote . >actual &&
 		test_unconfig $configsection.hiderefs &&
-		git ls-remote . |
-		sed -e "/	refs\/tags\//d" >expect &&
+		git ls-remote . >expect.raw &&
+		sed -e "/	refs\/tags\//d" expect.raw >expect &&
 		test_cmp expect actual
 	'
 
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/8] t5550: remove use of `test_might_fail grep`
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (2 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 3/8] t5512: stop losing return codes of git commands Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  6:35   ` Junio C Hamano
  2020-03-25  5:54 ` [PATCH 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

The test_must_fail() family of functions (including test_might_fail())
should only be used on git commands. Rewrite the use of
test_might_fail() with grep to remove this improper usage.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5550-http-fetch-dumb.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b811d89cfd..a06294ad8f 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -248,7 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '
 '
 
 test_expect_success 'did not use upload-pack service' '
-	test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act &&
+	{ grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act || :; } &&
 	: >exp &&
 	test_cmp exp act
 '
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/8] t5607: reorder `nongit test_must_fail`
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (3 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 4/8] t5550: remove use of `test_might_fail grep` Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  5:54 ` [PATCH 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

In the future, we plan on only allowing `test_must_fail` to work on a
restricted subset of commands, including `git`. Reorder the commands so
that `nongit` comes before `test_must_fail`. This way, `test_must_fail`
operates on a git command.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5607-clone-bundle.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index 9108ff6fbd..6d5a977fcb 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -16,7 +16,7 @@ test_expect_success 'setup' '
 
 test_expect_success '"verify" needs a worktree' '
 	git bundle create tip.bundle -1 master &&
-	test_must_fail nongit git bundle verify ../tip.bundle 2>err &&
+	nongit test_must_fail git bundle verify ../tip.bundle 2>err &&
 	test_i18ngrep "need a repository" err
 '
 
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 6/8] t5612: don't use `test_must_fail test_cmp`
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (4 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  5:54 ` [PATCH 7/8] t5612: stop losing return codes of git commands Denton Liu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since test_cmp() just
wraps an external command, replace `test_must_fail test_cmp` with
`! test_cmp`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5612-clone-refspec.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index e36ac01661..28373e715a 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -87,7 +87,7 @@ test_expect_success 'by default no tags will be kept updated' '
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&
-	test_must_fail test_cmp expect actual &&
+	! test_cmp expect actual &&
 	test_line_count = 2 actual
 '
 
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 7/8] t5612: stop losing return codes of git commands
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (5 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  5:54 ` [PATCH 8/8] t5801: teach compare_refs() to accept ! Denton Liu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

In a pipe, only the return code of the last command is used. Thus, all
other commands will have their return codes masked. Rewrite pipes so
that there are no git commands upstream so that their failure is
reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5612-clone-refspec.sh | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index 28373e715a..e3b436d8ae 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -71,9 +71,9 @@ test_expect_success 'by default all branches will be kept updated' '
 	(
 		cd dir_all &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# follow both master and side
 	git for-each-ref refs/heads >expect &&
@@ -104,9 +104,9 @@ test_expect_success '--single-branch while HEAD pointing at master' '
 	(
 		cd dir_master &&
 		git fetch --force &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow master
 	git for-each-ref refs/heads/master >expect &&
@@ -126,9 +126,9 @@ test_expect_success '--single-branch while HEAD pointing at master and --no-tags
 	(
 		cd dir_master_no_tags &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow master
 	git for-each-ref refs/heads/master >expect &&
@@ -156,9 +156,9 @@ test_expect_success '--single-branch while HEAD pointing at side' '
 	(
 		cd dir_side &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow side
 	git for-each-ref refs/heads/side >expect &&
@@ -169,9 +169,9 @@ test_expect_success '--single-branch with explicit --branch side' '
 	(
 		cd dir_side2 &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow side
 	git for-each-ref refs/heads/side >expect &&
@@ -223,9 +223,9 @@ test_expect_success '--single-branch with detached' '
 	(
 		cd dir_detached &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# nothing
 	test_must_be_empty actual
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 8/8] t5801: teach compare_refs() to accept !
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (6 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 7/8] t5612: stop losing return codes of git commands Denton Liu
@ 2020-03-25  5:54 ` Denton Liu
  2020-03-25  6:31   ` Junio C Hamano
  2020-03-25  6:43 ` [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Junio C Hamano
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
  9 siblings, 1 reply; 30+ messages in thread
From: Denton Liu @ 2020-03-25  5:54 UTC (permalink / raw)
  To: Git Mailing List

Before, testing if two refs weren't equal with compare_refs() was done
with `test_must_fail compare_refs`. This was wrong for two reasons.
First, test_must_fail should only be used on git commands. Second,
negating the error code is a little heavy-handed since in the case where
one of the git invocations within compare_refs() fails, we will report
success, even though it failed at an unexpected point.

Teach compare_refs() to accept `!` as the first argument which would
_only_ negate the test_cmp()'s return code.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5801-remote-helpers.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 121e5c6edb..0f04b6cddb 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -11,9 +11,15 @@ test_description='Test remote-helper import and export commands'
 PATH="$TEST_DIRECTORY/t5801:$PATH"
 
 compare_refs() {
+	fail= &&
+	if test "x$1" = 'x!'
+	then
+		fail='!' &&
+		shift
+	fi &&
 	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
 	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
-	test_cmp expect actual
+	eval $fail test_cmp expect actual
 }
 
 test_expect_success 'setup repository' '
@@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '
 	git push origin signed-tag
 	) &&
 	compare_refs local signed-tag^{} server signed-tag^{} &&
-	test_must_fail compare_refs local signed-tag server signed-tag
+	compare_refs ! local signed-tag server signed-tag
 '
 
 test_expect_success GPG 'push signed tag with signed-tags capability' '
-- 
2.25.0.114.g5b0ca878e0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/8] t5512: generate references with generate_references()
  2020-03-25  5:54 ` [PATCH 2/8] t5512: generate references with generate_references() Denton Liu
@ 2020-03-25  6:08   ` Eric Sunshine
  2020-03-25  6:41   ` Junio C Hamano
  1 sibling, 0 replies; 30+ messages in thread
From: Eric Sunshine @ 2020-03-25  6:08 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

On Wed, Mar 25, 2020 at 1:55 AM Denton Liu <liu.denton@gmail.com> wrote:
> t5512: generate references with generate_references()

This summary doesn't say anything useful. How about this instead?

    t5512: stop losing git exit code in here-docs

> The expected references are generated using a here-doc with some inline
> subshells. If one of the `git rev-parse` invocations within the
> subshells failed, its return code is swallowed and we won't know about

s/failed/fails/

> it. Replace these here-docs with generate_references(), which actually
> reports when `git rev-parse` fails.

A couple nits below...

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> @@ -4,6 +4,14 @@ test_description='git ls-remote'
> +generate_references () {
> +       for i
> +       do
> +               oid=$(git rev-parse "$i") || return 1
> +               printf '%s\t%s\n' "$oid" "$i"

I think the more usual way to say this in our test suite would be:

    oid=$(git rev-parse "$i") &&
    printf '%s\t%s\n' "$oid" "$i" || return 1

which has the nice property that someone can come along and insert
additional code in the loop body before the final "|| return 1"
without having to spend a lot of time trying to work out if the
&&-chain is intact or broken.

> +       done
> +}
> @@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '
>  test_expect_success 'ls-remote --sort="version:refname" --tags self' '
> -       cat >expect <<-EOF &&
> -       $(git rev-parse mark)   refs/tags/mark
> -       $(git rev-parse mark1.1)        refs/tags/mark1.1
> -       $(git rev-parse mark1.2)        refs/tags/mark1.2
> -       $(git rev-parse mark1.10)       refs/tags/mark1.10
> -       EOF
> +       generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&

This gets awfully wide and loses some readability. Perhaps:

    generate_references \
        refs/tags/mark \
        refs/tags/mark1.1 \
        refs/tags/mark1.2 \
        refs/tags/mark1.10 >expect &&

>         git ls-remote --sort="version:refname" --tags self >actual &&
>         test_cmp expect actual
>  '

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 8/8] t5801: teach compare_refs() to accept !
  2020-03-25  5:54 ` [PATCH 8/8] t5801: teach compare_refs() to accept ! Denton Liu
@ 2020-03-25  6:31   ` Junio C Hamano
  2020-03-25  6:36     ` Eric Sunshine
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2020-03-25  6:31 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

>  compare_refs() {
> +	fail= &&
> +	if test "x$1" = 'x!'
> +	then
> +		fail='!' &&
> +		shift
> +	fi &&
>  	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
>  	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
> -	test_cmp expect actual
> +	eval $fail test_cmp expect actual
>  }
>  
>  test_expect_success 'setup repository' '
> @@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '
>  	git push origin signed-tag
>  	) &&
>  	compare_refs local signed-tag^{} server signed-tag^{} &&
> -	test_must_fail compare_refs local signed-tag server signed-tag
> +	compare_refs ! local signed-tag server signed-tag

While this is not wrong per-se, I do not know why we cannot just use

	! compare_refs local signed-tag server signed-tag

i.e. "we expect these two repositories have different tags"?

It is totally plausible that I am missing something obvious, as it
is getting late and I no longer am a night person.

Thanks.

>  '
>  
>  test_expect_success GPG 'push signed tag with signed-tags capability' '

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/8] t5550: remove use of `test_might_fail grep`
  2020-03-25  5:54 ` [PATCH 4/8] t5550: remove use of `test_might_fail grep` Denton Liu
@ 2020-03-25  6:35   ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2020-03-25  6:35 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

> The test_must_fail() family of functions (including test_might_fail())
> should only be used on git commands. Rewrite the use of
> test_might_fail() with grep to remove this improper usage.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  t/t5550-http-fetch-dumb.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> index b811d89cfd..a06294ad8f 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -248,7 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '
>  '
>  
>  test_expect_success 'did not use upload-pack service' '
> -	test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act &&
> +	{ grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act || :; } &&

We can use

	sed -n -e "/\/git-upload-pack/p" "$HTTPD_ROOT_PATH/access.log" >actual

instead, but "grep for the pattern but don't worry if we found no
match" is also OK.

>  	: >exp &&
>  	test_cmp exp act

Having said that, if the expectation is not to find any match,
shouldn't the whole test be just

	! grep "/git-upload-pack" "$HTTPD_ROOT_PATH/access.log"

a single liner?  In any case, the use of sq in the original is broken.

>  '

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 8/8] t5801: teach compare_refs() to accept !
  2020-03-25  6:31   ` Junio C Hamano
@ 2020-03-25  6:36     ` Eric Sunshine
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Sunshine @ 2020-03-25  6:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Denton Liu, Git Mailing List

On Wed, Mar 25, 2020 at 2:31 AM Junio C Hamano <gitster@pobox.com> wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> >  compare_refs() {
> > +     fail= &&
> > +     if test "x$1" = 'x!'
> > +     then
> > +             fail='!' &&
> > +             shift
> > +     fi &&
> >       git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
> >       git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
> > -     test_cmp expect actual
> > +     eval $fail test_cmp expect actual
> >  }
> > -     test_must_fail compare_refs local signed-tag server signed-tag
> > +     compare_refs ! local signed-tag server signed-tag
>
> While this is not wrong per-se, I do not know why we cannot just use
>
>         ! compare_refs local signed-tag server signed-tag
>
> i.e. "we expect these two repositories have different tags"?

As mentioned in the commit message, if one of the git-rev-parse
invocations fails unexpectedly, then compare_refs() would return early
with a failure code, but the "!" would then (undesirably) turn that
failure into a success. We don't want to lose a failure code from
git-rev-parse, so the simple use of "! compare_refs ..." is avoided.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/8] t5512: generate references with generate_references()
  2020-03-25  5:54 ` [PATCH 2/8] t5512: generate references with generate_references() Denton Liu
  2020-03-25  6:08   ` Eric Sunshine
@ 2020-03-25  6:41   ` Junio C Hamano
  1 sibling, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2020-03-25  6:41 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

> +generate_references () {
> +	for i

Is it just me who thinks variables used in iteration should be
called i, j, etc. only when they are integers?

> +	do
> +		oid=$(git rev-parse "$i") || return 1
> +		printf '%s\t%s\n' "$oid" "$i"
> +	done
> +}
> +
>  test_expect_success setup '
>  	>file &&
>  	git add file &&
> @@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '
>  '
>  
>  test_expect_success 'ls-remote --sort="version:refname" --tags self' '
> -	cat >expect <<-EOF &&
> -	$(git rev-parse mark)	refs/tags/mark
> -	$(git rev-parse mark1.1)	refs/tags/mark1.1
> -	$(git rev-parse mark1.2)	refs/tags/mark1.2
> -	$(git rev-parse mark1.10)	refs/tags/mark1.10
> -	EOF
> +	generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&

Hmph, can we avoid overlong lines like this one?

	generate_references >expect <<-EOF
	refs/tags/mark
	refs/tags/mark1.1
	refs/tags/mark1.2
	refs/tags/mark1.10
	EOF

i.e. teaching the helper function to read from its standard input
stream, may make it more readable (i.e. it is more obvious what the
order of expected output lines are, as you are listing them one by
one on a line of its own).


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3)
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (7 preceding siblings ...)
  2020-03-25  5:54 ` [PATCH 8/8] t5801: teach compare_refs() to accept ! Denton Liu
@ 2020-03-25  6:43 ` Junio C Hamano
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
  9 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2020-03-25  6:43 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

> The overall scope of these patches is to replace inappropriate uses of
> test_must_fail. IOW, we should only allow test_must_fail to run on `git`
> and `test-tool`. Ultimately, we will conclude by making test_must_fail
> error out on non-git commands. An advance view of the final series can
> be found here[1].
>
> This is the third part. It focuses on t5*.sh.
>
> The first part can be found here[2]. The second part can be found here[3].
>
> [1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2
> [2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/
> [3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/

I left comments on individual patches, but I think they were all
minor.  Mostly the series looks good.

Thanks.

The dl/test-must-fail-fixes-* topics are always fun to read ;-)

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 0/8] t: replace incorrect test_must_fail usage (part 3)
  2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
                   ` (8 preceding siblings ...)
  2020-03-25  6:43 ` [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Junio C Hamano
@ 2020-03-26  8:27 ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
                     ` (8 more replies)
  9 siblings, 9 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

The overall scope of these patches is to replace inappropriate uses of
test_must_fail. IOW, we should only allow test_must_fail to run on `git`
and `test-tool`. Ultimately, we will conclude by making test_must_fail
error out on non-git commands. An advance view of the final series can
be found here[1].

This is the third part. It focuses on t5*.sh.

The first part can be found here[2]. The second part can be found here[3].

Changes since v1:

* Broke up long generate_references() lines for clarity in 2/8

* For-loop &&-chain and renamed variable to "ref" in 2/8

* Cleaned up commit message in 2/8

* Rewrote 4/8 with a simpler test case

[1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2
[2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/
[3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/

Denton Liu (8):
  t5512: don't use `test_must_fail test_cmp`
  t5512: stop losing git exit code in here-docs
  t5512: stop losing return codes of git commands
  t5550: simplify no matching line check
  t5607: reorder `nongit test_must_fail`
  t5612: don't use `test_must_fail test_cmp`
  t5612: stop losing return codes of git commands
  t5801: teach compare_refs() to accept !

 t/t5512-ls-remote.sh       | 80 ++++++++++++++++++++------------------
 t/t5550-http-fetch-dumb.sh |  4 +-
 t/t5607-clone-bundle.sh    |  2 +-
 t/t5612-clone-refspec.sh   | 26 ++++++-------
 t/t5801-remote-helpers.sh  | 10 ++++-
 5 files changed, 66 insertions(+), 56 deletions(-)

Range-diff against v1:
1:  3d7dc8428d = 1:  3d7dc8428d t5512: don't use `test_must_fail test_cmp`
2:  674de50db2 ! 2:  97bc46e4a9 t5512: generate references with generate_references()
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    t5512: generate references with generate_references()
    +    t5512: stop losing git exit code in here-docs
     
         The expected references are generated using a here-doc with some inline
    -    subshells. If one of the `git rev-parse` invocations within the
    -    subshells failed, its return code is swallowed and we won't know about
    -    it. Replace these here-docs with generate_references(), which actually
    -    reports when `git rev-parse` fails.
    +    command substitutions. If one of the `git rev-parse` invocations within
    +    the command substitutions fails, its return code is swallowed and we
    +    won't know about it. Replace these command substitutions with
    +    generate_references(), which actually reports when `git rev-parse`
    +    fails.
     
      ## t/t5512-ls-remote.sh ##
     @@ t/t5512-ls-remote.sh: test_description='git ls-remote'
    @@ t/t5512-ls-remote.sh: test_description='git ls-remote'
      . ./test-lib.sh
      
     +generate_references () {
    -+	for i
    ++	for ref
     +	do
    -+		oid=$(git rev-parse "$i") || return 1
    -+		printf '%s\t%s\n' "$oid" "$i"
    ++		oid=$(git rev-parse "$ref") &&
    ++		printf '%s\t%s\n' "$oid" "$ref" || return 1
     +	done
     +}
     +
    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '
     -	$(git rev-parse mark1.2)	refs/tags/mark1.2
     -	$(git rev-parse mark1.10)	refs/tags/mark1.10
     -	EOF
    -+	generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&
    ++	generate_references \
    ++		refs/tags/mark \
    ++		refs/tags/mark1.1 \
    ++		refs/tags/mark1.2 \
    ++		refs/tags/mark1.10 >expect &&
      	git ls-remote --sort="version:refname" --tags self >actual &&
      	test_cmp expect actual
      '
    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '
     -	$(git rev-parse mark1.1)	refs/tags/mark1.1
     -	$(git rev-parse mark)	refs/tags/mark
     -	EOF
    -+	generate_references refs/tags/mark1.10 refs/tags/mark1.2 refs/tags/mark1.1 refs/tags/mark >expect &&
    ++	generate_references \
    ++		refs/tags/mark1.10 \
    ++		refs/tags/mark1.2 \
    ++		refs/tags/mark1.1 \
    ++		refs/tags/mark >expect &&
      	git ls-remote --sort="-version:refname" --tags self >actual &&
      	test_cmp expect actual
      '
    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '
     -	$(git rev-parse mark1.1)	refs/tags/mark1.1
     -	$(git rev-parse mark)	refs/tags/mark
     -	EOF
    -+	generate_references refs/tags/mark1.2 refs/tags/mark1.10 refs/tags/mark1.1 refs/tags/mark >expect &&
    ++	generate_references \
    ++		refs/tags/mark1.2 \
    ++		refs/tags/mark1.10 \
    ++		refs/tags/mark1.1 \
    ++		refs/tags/mark >expect &&
      	git ls-remote --sort="-refname" --tags self >actual &&
      	test_cmp expect actual
      '
    @@ t/t5512-ls-remote.sh: test_expect_success 'protocol v2 supports hiderefs' '
     -	$(git rev-parse refs/tags/mark1.2)	refs/tags/mark1.2
     -	EOF
     +	echo "ref: refs/heads/master	HEAD" >expect &&
    -+	generate_references HEAD \
    ++	generate_references \
    ++		HEAD \
     +		refs/heads/master >>expect &&
     +	oid=$(git rev-parse HEAD) &&
     +	echo "$oid	refs/remotes/origin/HEAD" >>expect &&
    -+	generate_references refs/remotes/origin/master \
    ++	generate_references \
    ++	refs/remotes/origin/master \
     +		refs/tags/mark \
     +		refs/tags/mark1.1 \
     +		refs/tags/mark1.10 \
3:  b3addeb985 = 3:  986ba1dd39 t5512: stop losing return codes of git commands
4:  68c911e29b < -:  ---------- t5550: remove use of `test_might_fail grep`
-:  ---------- > 4:  53e64e7077 t5550: simplify no matching line check
5:  4253f51fea = 5:  4955b701e1 t5607: reorder `nongit test_must_fail`
6:  2bac5f4a29 = 6:  a3d9d3673b t5612: don't use `test_must_fail test_cmp`
7:  b490ccace0 = 7:  bc0e90d5ba t5612: stop losing return codes of git commands
8:  b51f97f6ae = 8:  6a9d3cef2a t5801: teach compare_refs() to accept !
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp`
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 2/8] t5512: stop losing git exit code in here-docs Denton Liu
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since test_cmp() just
wraps an external command, replace `test_must_fail test_cmp` with
`! test_cmp`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 04b35402c7..08b98f12b8 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -92,7 +92,7 @@ test_expect_success 'use "origin" when no remote specified' '
 
 test_expect_success 'suppress "From <url>" with -q' '
 	git ls-remote -q 2>actual_err &&
-	test_must_fail test_cmp exp_err actual_err
+	! test_cmp exp_err actual_err
 '
 
 test_expect_success 'use branch.<name>.remote if possible' '
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 2/8] t5512: stop losing git exit code in here-docs
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
  2020-03-26  8:27   ` [PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26 15:24     ` Eric Sunshine
  2020-03-26  8:27   ` [PATCH v2 3/8] t5512: stop losing return codes of git commands Denton Liu
                     ` (6 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

The expected references are generated using a here-doc with some inline
command substitutions. If one of the `git rev-parse` invocations within
the command substitutions fails, its return code is swallowed and we
won't know about it. Replace these command substitutions with
generate_references(), which actually reports when `git rev-parse`
fails.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 64 ++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 08b98f12b8..dcb7349b0b 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -4,6 +4,14 @@ test_description='git ls-remote'
 
 . ./test-lib.sh
 
+generate_references () {
+	for ref
+	do
+		oid=$(git rev-parse "$ref") &&
+		printf '%s\t%s\n' "$oid" "$ref" || return 1
+	done
+}
+
 test_expect_success setup '
 	>file &&
 	git add file &&
@@ -43,34 +51,31 @@ test_expect_success 'ls-remote self' '
 '
 
 test_expect_success 'ls-remote --sort="version:refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark)	refs/tags/mark
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	EOF
+	generate_references \
+		refs/tags/mark \
+		refs/tags/mark1.1 \
+		refs/tags/mark1.2 \
+		refs/tags/mark1.10 >expect &&
 	git ls-remote --sort="version:refname" --tags self >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-remote --sort="-version:refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark)	refs/tags/mark
-	EOF
+	generate_references \
+		refs/tags/mark1.10 \
+		refs/tags/mark1.2 \
+		refs/tags/mark1.1 \
+		refs/tags/mark >expect &&
 	git ls-remote --sort="-version:refname" --tags self >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-remote --sort="-refname" --tags self' '
-	cat >expect <<-EOF &&
-	$(git rev-parse mark1.2)	refs/tags/mark1.2
-	$(git rev-parse mark1.10)	refs/tags/mark1.10
-	$(git rev-parse mark1.1)	refs/tags/mark1.1
-	$(git rev-parse mark)	refs/tags/mark
-	EOF
+	generate_references \
+		refs/tags/mark1.2 \
+		refs/tags/mark1.10 \
+		refs/tags/mark1.1 \
+		refs/tags/mark >expect &&
 	git ls-remote --sort="-refname" --tags self >actual &&
 	test_cmp expect actual
 '
@@ -212,17 +217,18 @@ test_expect_success 'protocol v2 supports hiderefs' '
 
 test_expect_success 'ls-remote --symref' '
 	git fetch origin &&
-	cat >expect <<-EOF &&
-	ref: refs/heads/master	HEAD
-	$(git rev-parse HEAD)	HEAD
-	$(git rev-parse refs/heads/master)	refs/heads/master
-	$(git rev-parse HEAD)	refs/remotes/origin/HEAD
-	$(git rev-parse refs/remotes/origin/master)	refs/remotes/origin/master
-	$(git rev-parse refs/tags/mark)	refs/tags/mark
-	$(git rev-parse refs/tags/mark1.1)	refs/tags/mark1.1
-	$(git rev-parse refs/tags/mark1.10)	refs/tags/mark1.10
-	$(git rev-parse refs/tags/mark1.2)	refs/tags/mark1.2
-	EOF
+	echo "ref: refs/heads/master	HEAD" >expect &&
+	generate_references \
+		HEAD \
+		refs/heads/master >>expect &&
+	oid=$(git rev-parse HEAD) &&
+	echo "$oid	refs/remotes/origin/HEAD" >>expect &&
+	generate_references \
+	refs/remotes/origin/master \
+		refs/tags/mark \
+		refs/tags/mark1.1 \
+		refs/tags/mark1.10 \
+		refs/tags/mark1.2 >>expect &&
 	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
 	# protocol v0 here.
 	GIT_TEST_PROTOCOL_VERSION=0 git ls-remote --symref >actual &&
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 3/8] t5512: stop losing return codes of git commands
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
  2020-03-26  8:27   ` [PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 2/8] t5512: stop losing git exit code in here-docs Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 4/8] t5550: simplify no matching line check Denton Liu
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

In a pipe, only the return code of the last command is used. Thus, all
other commands will have their return codes masked. Rewrite pipes so
that there are no git commands upstream so that their failure is
reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index dcb7349b0b..8928d1f62d 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -21,11 +21,11 @@ test_expect_success setup '
 	git tag mark1.1 &&
 	git tag mark1.2 &&
 	git tag mark1.10 &&
-	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
-	(
-		echo "$(git rev-parse HEAD)	HEAD" &&
-		git show-ref -d	| sed -e "s/ /	/"
-	) >expected.all &&
+	git show-ref --tags -d >expected.tag.raw &&
+	sed -e "s/ /	/" expected.tag.raw >expected.tag &&
+	generate_references HEAD >expected.all &&
+	git show-ref -d	>refs &&
+	sed -e "s/ /	/" refs >>expected.all &&
 
 	git remote add self "$(pwd)/.git"
 '
@@ -185,8 +185,8 @@ do
 		test_config $configsection.hiderefs refs/tags &&
 		git ls-remote . >actual &&
 		test_unconfig $configsection.hiderefs &&
-		git ls-remote . |
-		sed -e "/	refs\/tags\//d" >expect &&
+		git ls-remote . >expect.raw &&
+		sed -e "/	refs\/tags\//d" expect.raw >expect &&
 		test_cmp expect actual
 	'
 
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 4/8] t5550: simplify no matching line check
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (2 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 3/8] t5512: stop losing return codes of git commands Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26 15:26     ` Eric Sunshine
  2020-03-26  8:27   ` [PATCH v2 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
                     ` (4 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

In the 'did not use upload-pack service' test, we have a complicated
song-and-dance to ensure that there are no "/git-upload-pack" lines in
"$HTTPD_ROOT_PATH/access.log". Simplify this by just checking that grep
returns a non-zero exit code.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5550-http-fetch-dumb.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b811d89cfd..bcde886b87 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -248,9 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '
 '
 
 test_expect_success 'did not use upload-pack service' '
-	test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act &&
-	: >exp &&
-	test_cmp exp act
+	! grep "/git-upload-pack" "$HTTPD_ROOT_PATH/access.log"
 '
 
 test_expect_success 'git client shows text/plain errors' '
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 5/8] t5607: reorder `nongit test_must_fail`
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (3 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 4/8] t5550: simplify no matching line check Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

In the future, we plan on only allowing `test_must_fail` to work on a
restricted subset of commands, including `git`. Reorder the commands so
that `nongit` comes before `test_must_fail`. This way, `test_must_fail`
operates on a git command.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5607-clone-bundle.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index 9108ff6fbd..6d5a977fcb 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -16,7 +16,7 @@ test_expect_success 'setup' '
 
 test_expect_success '"verify" needs a worktree' '
 	git bundle create tip.bundle -1 master &&
-	test_must_fail nongit git bundle verify ../tip.bundle 2>err &&
+	nongit test_must_fail git bundle verify ../tip.bundle 2>err &&
 	test_i18ngrep "need a repository" err
 '
 
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 6/8] t5612: don't use `test_must_fail test_cmp`
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (4 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 7/8] t5612: stop losing return codes of git commands Denton Liu
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since test_cmp() just
wraps an external command, replace `test_must_fail test_cmp` with
`! test_cmp`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5612-clone-refspec.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index e36ac01661..28373e715a 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -87,7 +87,7 @@ test_expect_success 'by default no tags will be kept updated' '
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&
-	test_must_fail test_cmp expect actual &&
+	! test_cmp expect actual &&
 	test_line_count = 2 actual
 '
 
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 7/8] t5612: stop losing return codes of git commands
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (5 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-26  8:27   ` [PATCH v2 8/8] t5801: teach compare_refs() to accept ! Denton Liu
  2020-03-27  0:43   ` [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs Denton Liu
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

In a pipe, only the return code of the last command is used. Thus, all
other commands will have their return codes masked. Rewrite pipes so
that there are no git commands upstream so that their failure is
reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5612-clone-refspec.sh | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index 28373e715a..e3b436d8ae 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -71,9 +71,9 @@ test_expect_success 'by default all branches will be kept updated' '
 	(
 		cd dir_all &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# follow both master and side
 	git for-each-ref refs/heads >expect &&
@@ -104,9 +104,9 @@ test_expect_success '--single-branch while HEAD pointing at master' '
 	(
 		cd dir_master &&
 		git fetch --force &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow master
 	git for-each-ref refs/heads/master >expect &&
@@ -126,9 +126,9 @@ test_expect_success '--single-branch while HEAD pointing at master and --no-tags
 	(
 		cd dir_master_no_tags &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow master
 	git for-each-ref refs/heads/master >expect &&
@@ -156,9 +156,9 @@ test_expect_success '--single-branch while HEAD pointing at side' '
 	(
 		cd dir_side &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow side
 	git for-each-ref refs/heads/side >expect &&
@@ -169,9 +169,9 @@ test_expect_success '--single-branch with explicit --branch side' '
 	(
 		cd dir_side2 &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# only follow side
 	git for-each-ref refs/heads/side >expect &&
@@ -223,9 +223,9 @@ test_expect_success '--single-branch with detached' '
 	(
 		cd dir_detached &&
 		git fetch &&
-		git for-each-ref refs/remotes/origin |
+		git for-each-ref refs/remotes/origin >refs &&
 		sed -e "/HEAD$/d" \
-		    -e "s|/remotes/origin/|/heads/|" >../actual
+		    -e "s|/remotes/origin/|/heads/|" refs >../actual
 	) &&
 	# nothing
 	test_must_be_empty actual
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 8/8] t5801: teach compare_refs() to accept !
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (6 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 7/8] t5612: stop losing return codes of git commands Denton Liu
@ 2020-03-26  8:27   ` Denton Liu
  2020-03-27  0:43   ` [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs Denton Liu
  8 siblings, 0 replies; 30+ messages in thread
From: Denton Liu @ 2020-03-26  8:27 UTC (permalink / raw)
  To: Git Mailing List

Before, testing if two refs weren't equal with compare_refs() was done
with `test_must_fail compare_refs`. This was wrong for two reasons.
First, test_must_fail should only be used on git commands. Second,
negating the error code is a little heavy-handed since in the case where
one of the git invocations within compare_refs() fails, we will report
success, even though it failed at an unexpected point.

Teach compare_refs() to accept `!` as the first argument which would
_only_ negate the test_cmp()'s return code.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5801-remote-helpers.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 121e5c6edb..0f04b6cddb 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -11,9 +11,15 @@ test_description='Test remote-helper import and export commands'
 PATH="$TEST_DIRECTORY/t5801:$PATH"
 
 compare_refs() {
+	fail= &&
+	if test "x$1" = 'x!'
+	then
+		fail='!' &&
+		shift
+	fi &&
 	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
 	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
-	test_cmp expect actual
+	eval $fail test_cmp expect actual
 }
 
 test_expect_success 'setup repository' '
@@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '
 	git push origin signed-tag
 	) &&
 	compare_refs local signed-tag^{} server signed-tag^{} &&
-	test_must_fail compare_refs local signed-tag server signed-tag
+	compare_refs ! local signed-tag server signed-tag
 '
 
 test_expect_success GPG 'push signed tag with signed-tags capability' '
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/8] t5512: stop losing git exit code in here-docs
  2020-03-26  8:27   ` [PATCH v2 2/8] t5512: stop losing git exit code in here-docs Denton Liu
@ 2020-03-26 15:24     ` Eric Sunshine
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Sunshine @ 2020-03-26 15:24 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

On Thu, Mar 26, 2020 at 4:28 AM Denton Liu <liu.denton@gmail.com> wrote:
> The expected references are generated using a here-doc with some inline
> command substitutions. If one of the `git rev-parse` invocations within
> the command substitutions fails, its return code is swallowed and we
> won't know about it. Replace these command substitutions with
> generate_references(), which actually reports when `git rev-parse`
> fails.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> @@ -212,17 +217,18 @@ test_expect_success 'protocol v2 supports hiderefs' '
>  test_expect_success 'ls-remote --symref' '
> +       echo "$oid      refs/remotes/origin/HEAD" >>expect &&
> +       generate_references \
> +       refs/remotes/origin/master \
> +               refs/tags/mark \
> +               refs/tags/mark1.1 \
> +               refs/tags/mark1.10 \
> +               refs/tags/mark1.2 >>expect &&

Botched indentation of the "refs/remotes/origin/master" line.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 4/8] t5550: simplify no matching line check
  2020-03-26  8:27   ` [PATCH v2 4/8] t5550: simplify no matching line check Denton Liu
@ 2020-03-26 15:26     ` Eric Sunshine
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Sunshine @ 2020-03-26 15:26 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Junio C Hamano

On Thu, Mar 26, 2020 at 4:28 AM Denton Liu <liu.denton@gmail.com> wrote:
> In the 'did not use upload-pack service' test, we have a complicated
> song-and-dance to ensure that there are no "/git-upload-pack" lines in
> "$HTTPD_ROOT_PATH/access.log". Simplify this by just checking that grep
> returns a non-zero exit code.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> @@ -248,9 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '
>  test_expect_success 'did not use upload-pack service' '
> -       test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act &&
> -       : >exp &&
> -       test_cmp exp act
> +       ! grep "/git-upload-pack" "$HTTPD_ROOT_PATH/access.log"
>  '

It would have been nice for the commit message to have mentioned that
the change is also eliminating an incorrect application of
test_might_fail(), but it's probably not worth a re-roll.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
  2020-03-26  8:27 ` [PATCH v2 " Denton Liu
                     ` (7 preceding siblings ...)
  2020-03-26  8:27   ` [PATCH v2 8/8] t5801: teach compare_refs() to accept ! Denton Liu
@ 2020-03-27  0:43   ` Denton Liu
  2020-03-27 17:46     ` Junio C Hamano
  8 siblings, 1 reply; 30+ messages in thread
From: Denton Liu @ 2020-03-27  0:43 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t5512-ls-remote.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 8928d1f62d..e98c3a0174 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -224,7 +224,7 @@ test_expect_success 'ls-remote --symref' '
 	oid=$(git rev-parse HEAD) &&
 	echo "$oid	refs/remotes/origin/HEAD" >>expect &&
 	generate_references \
-	refs/remotes/origin/master \
+		refs/remotes/origin/master \
 		refs/tags/mark \
 		refs/tags/mark1.1 \
 		refs/tags/mark1.10 \
-- 
2.26.0.159.g23e2136ad0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
  2020-03-27  0:43   ` [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs Denton Liu
@ 2020-03-27 17:46     ` Junio C Hamano
  2020-03-27 20:29       ` Eric Sunshine
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2020-03-27 17:46 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs

Forgot to do the final "rebase -i --autosquash"?

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  t/t5512-ls-remote.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index 8928d1f62d..e98c3a0174 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -224,7 +224,7 @@ test_expect_success 'ls-remote --symref' '
>  	oid=$(git rev-parse HEAD) &&
>  	echo "$oid	refs/remotes/origin/HEAD" >>expect &&
>  	generate_references \
> -	refs/remotes/origin/master \
> +		refs/remotes/origin/master \
>  		refs/tags/mark \
>  		refs/tags/mark1.1 \
>  		refs/tags/mark1.10 \

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
  2020-03-27 17:46     ` Junio C Hamano
@ 2020-03-27 20:29       ` Eric Sunshine
  2020-03-27 21:39         ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Sunshine @ 2020-03-27 20:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Denton Liu, Git Mailing List

On Fri, Mar 27, 2020 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
>
> Forgot to do the final "rebase -i --autosquash"?

My guess is that Denton sent this "fixup!" to have it queued atop [1]
in response to the review comment[2] pointing out the bad indentation
(rather than re-sending the entire series just for one minor change).

[1]: https://lore.kernel.org/git/cover.1585209554.git.liu.denton@gmail.com/
[2]: https://lore.kernel.org/git/CAPig+cTpV2n66nYJdQpG0tYCu5f=PB-a9Boa_NNfVaUOshDVug@mail.gmail.com/

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
  2020-03-27 20:29       ` Eric Sunshine
@ 2020-03-27 21:39         ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2020-03-27 21:39 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Denton Liu, Git Mailing List

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Mar 27, 2020 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>> > Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs
>>
>> Forgot to do the final "rebase -i --autosquash"?
>
> My guess is that Denton sent this "fixup!" to have it queued atop [1]
> in response to the review comment[2] pointing out the bad indentation

Ah, [9/9] fooled me (and caused me not to see eight in [1/8]-[8/8]
and I mistook they were [1/9]-[8/9].

Usually people do the "oops, here is a hotfix" by marking such a
"beyond the original set" patch as [9/8].

In any case, I think I've squashed it to the original.
Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-03-27 21:39 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25  5:54 [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Denton Liu
2020-03-25  5:54 ` [PATCH 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
2020-03-25  5:54 ` [PATCH 2/8] t5512: generate references with generate_references() Denton Liu
2020-03-25  6:08   ` Eric Sunshine
2020-03-25  6:41   ` Junio C Hamano
2020-03-25  5:54 ` [PATCH 3/8] t5512: stop losing return codes of git commands Denton Liu
2020-03-25  5:54 ` [PATCH 4/8] t5550: remove use of `test_might_fail grep` Denton Liu
2020-03-25  6:35   ` Junio C Hamano
2020-03-25  5:54 ` [PATCH 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
2020-03-25  5:54 ` [PATCH 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
2020-03-25  5:54 ` [PATCH 7/8] t5612: stop losing return codes of git commands Denton Liu
2020-03-25  5:54 ` [PATCH 8/8] t5801: teach compare_refs() to accept ! Denton Liu
2020-03-25  6:31   ` Junio C Hamano
2020-03-25  6:36     ` Eric Sunshine
2020-03-25  6:43 ` [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3) Junio C Hamano
2020-03-26  8:27 ` [PATCH v2 " Denton Liu
2020-03-26  8:27   ` [PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp` Denton Liu
2020-03-26  8:27   ` [PATCH v2 2/8] t5512: stop losing git exit code in here-docs Denton Liu
2020-03-26 15:24     ` Eric Sunshine
2020-03-26  8:27   ` [PATCH v2 3/8] t5512: stop losing return codes of git commands Denton Liu
2020-03-26  8:27   ` [PATCH v2 4/8] t5550: simplify no matching line check Denton Liu
2020-03-26 15:26     ` Eric Sunshine
2020-03-26  8:27   ` [PATCH v2 5/8] t5607: reorder `nongit test_must_fail` Denton Liu
2020-03-26  8:27   ` [PATCH v2 6/8] t5612: don't use `test_must_fail test_cmp` Denton Liu
2020-03-26  8:27   ` [PATCH v2 7/8] t5612: stop losing return codes of git commands Denton Liu
2020-03-26  8:27   ` [PATCH v2 8/8] t5801: teach compare_refs() to accept ! Denton Liu
2020-03-27  0:43   ` [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs Denton Liu
2020-03-27 17:46     ` Junio C Hamano
2020-03-27 20:29       ` Eric Sunshine
2020-03-27 21:39         ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).