t3200: fix antipatterns in existing branch tests

PATCH Superseded Tao Klerks via GitGitGadget 4 participants 7 messages Mar 21, 2022 – May 12, 2022

lore

Share & tools

lore

Tao Klerks via GitGitGadget

author

2022-03-21 06:52 UTC #0 lore raw

PATCH 1/1 t3200: fix antipatterns in existing branch tests

From: Tao Klerks <[email protected]>

Fix issues in t3200 branch tests that, if copied, might catch new contributors out:

Use test_config to show that config state is not being intentionally left to spill over into other tests.

Use test_cmp_config instead of git config in subshells, so that git's error code is not lost if/when an unexpected error occurs.

Use output redirection and later content checking instead of subshells, so that git's error code is not lost if/when an unexpected error occurs.

Try to eliminate local-fetch-avoiding optimization as it is error-prone (it is easy to check the wrong thing), hides segfaults, and yields only a marginal performance improvement anyway.

Introduce local helper test_set_remote to simplify the common local pattern of setting up a remote via config.

Signed-off-by Tao Klerks <[email protected]>

---
RFC: t3200: fix antipatterns in existing branch tests

This is a cleanup of the branch tests following a case where I was adding some, and did substantially the wrong thing by following existing examples.

I'm submitting this as RFC because I have a couple of significant doubts:

1. Does it make sense to do this? I believe it's a good idea to keep things "clean" so that newcomers more easily do the right thing than the wrong thing, but on the other hand, I've definitely read that we have a "don't change things unnecessarily" bias somewhere.
2. What's the right pattern for the "(git show-ref -q refs/remotes/local/main || git fetch local)" fetch-avoidance optimization? Removing it adds a second to test runtimes, but Ævar warned it hides segfaults

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1182%2FTaoK%2Fcleanup-t3200-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1182/TaoK/cleanup-t3200-tests-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1182

 t/t3200-branch.sh | 448 ++++++++++++++++++++++------------------------
 1 file changed, 212 insertions(+), 236 deletions(-)

t/t3200-branch.sh +212 ~~-236~~

index 1bc3795847d..a4cbedd7f04 100755
@@ -11,6 +11,20 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+test_set_remote () {
+	test_config "remote.$1.url" "$2" &&
+	test_config "remote.$1.fetch" "${3:-"refs/heads/*:refs/remotes/$1/*"}"
+}
+
+fetch_if_remote_ref_missing () {
+	# this is an anti-pattern: swallows segfault
+	#git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+	# this is slightly slower, up to 1s out of 6s on this set of tests:
+	git fetch "$2"
+	# this doesn't work
+	#test_might_fail git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+}
+
 test_expect_success 'prepare a trivial repository' '
 	echo Hello >A &&
 	git update-index --add A &&

@@ -372,11 +386,9 @@ EOF
 '
 
 test_expect_success 'git branch with column.*' '
-	git config column.ui column &&
-	git config column.branch "dense" &&
+	test_config column.ui column &&
+	test_config column.branch "dense" &&
 	COLUMNS=80 git branch >actual &&
-	git config --unset column.branch &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c   bam   foo   l   * main   n     o/p   r
   abc     bar   j/k   m/m   mb     o/o   q     topic

@@ -389,9 +401,8 @@ test_expect_success 'git branch --column -v should fail' '
 '
 
 test_expect_success 'git branch -v with column.ui ignored' '
-	git config column.ui column &&
+	test_config column.ui column &&
 	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
   abc

@@ -435,7 +446,7 @@ test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
 '
 
 test_expect_success 'config information was renamed, too' '
-	test $(git config branch.s.dummy) = Hello &&
+	test_cmp_config Hello branch.s.dummy &&
 	test_must_fail git config branch.s/s.dummy
 '

@@ -493,63 +504,57 @@ test_expect_success 'git branch --copy dumps usage' '
 test_expect_success 'git branch -c d e should work' '
 	git branch --create-reflog d &&
 	git reflog exists refs/heads/d &&
-	git config branch.d.dummy Hello &&
+	test_config branch.d.dummy Hello &&
 	git branch -c d e &&
 	git reflog exists refs/heads/d &&
 	git reflog exists refs/heads/e &&
-	echo Hello >expect &&
-	git config branch.e.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.d.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.e.dummy &&
+	test_cmp_config Hello branch.d.dummy
 '
 
 test_expect_success 'git branch --copy is a synonym for -c' '
 	git branch --create-reflog copy &&
 	git reflog exists refs/heads/copy &&
-	git config branch.copy.dummy Hello &&
+	test_config branch.copy.dummy Hello &&
 	git branch --copy copy copy-to &&
 	git reflog exists refs/heads/copy &&
 	git reflog exists refs/heads/copy-to &&
-	echo Hello >expect &&
-	git config branch.copy.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.copy-to.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.copy.dummy &&
+	test_cmp_config Hello branch.copy-to.dummy
 '
 
 test_expect_success 'git branch -c ee ef should copy ee to create branch ef' '
 	git checkout -b ee &&
 	git reflog exists refs/heads/ee &&
-	git config branch.ee.dummy Hello &&
+	test_config branch.ee.dummy Hello &&
 	git branch -c ee ef &&
 	git reflog exists refs/heads/ee &&
 	git reflog exists refs/heads/ef &&
-	test $(git config branch.ee.dummy) = Hello &&
-	test $(git config branch.ef.dummy) = Hello &&
-	test $(git rev-parse --abbrev-ref HEAD) = ee
+	test_cmp_config Hello branch.ee.dummy &&
+	test_cmp_config Hello branch.ef.dummy &&
+	echo ee >expect &&
+	git rev-parse --abbrev-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch -c f/f g/g should work' '
 	git branch --create-reflog f/f &&
 	git reflog exists refs/heads/f/f &&
-	git config branch.f/f.dummy Hello &&
+	test_config branch.f/f.dummy Hello &&
 	git branch -c f/f g/g &&
 	git reflog exists refs/heads/f/f &&
 	git reflog exists refs/heads/g/g &&
-	test $(git config branch.f/f.dummy) = Hello &&
-	test $(git config branch.g/g.dummy) = Hello
+	test_cmp_config Hello branch.f/f.dummy &&
+	test_cmp_config Hello branch.g/g.dummy
 '
 
 test_expect_success 'git branch -c m2 m2 should work' '
 	git branch --create-reflog m2 &&
 	git reflog exists refs/heads/m2 &&
-	git config branch.m2.dummy Hello &&
+	test_config branch.m2.dummy Hello &&
 	git branch -c m2 m2 &&
 	git reflog exists refs/heads/m2 &&
-	test $(git config branch.m2.dummy) = Hello
+	test_cmp_config Hello branch.m2.dummy
 '
 
 test_expect_success 'git branch -c zz zz/zz should fail' '

@@ -619,15 +624,15 @@ test_expect_success 'git branch -C main5 main5 should work when main is checked
 test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
 	git branch --create-reflog cd &&
 	git reflog exists refs/heads/cd &&
-	git config branch.cd.dummy CD &&
+	test_config branch.cd.dummy CD &&
 	git branch --create-reflog ab &&
 	git reflog exists refs/heads/ab &&
-	git config branch.ab.dummy AB &&
+	test_config branch.ab.dummy AB &&
 	git branch -C ab cd &&
 	git reflog exists refs/heads/ab &&
 	git reflog exists refs/heads/cd &&
-	test $(git config branch.ab.dummy) = AB &&
-	test $(git config branch.cd.dummy) = AB
+	test_cmp_config AB branch.ab.dummy &&
+	test_cmp_config AB branch.cd.dummy
 '
 
 test_expect_success 'git branch -c correctly copies multiple config sections' '

@@ -761,75 +766,67 @@ test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
 '
 
 test_expect_success 'test tracking setup via --track' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track my1 local/main &&
-	test $(git config branch.my1.remote) = local &&
-	test $(git config branch.my1.merge) = refs/heads/main
+	test_cmp_config local branch.my1.remote &&
+	test_cmp_config refs/heads/main branch.my1.merge
 '
 
 test_expect_success 'test tracking setup (non-wildcard, matching)' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/main:refs/remotes/local/main &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . refs/heads/main:refs/remotes/local/main &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track my4 local/main &&
-	test $(git config branch.my4.remote) = local &&
-	test $(git config branch.my4.merge) = refs/heads/main
+	test_cmp_config local branch.my4.remote &&
+	test_cmp_config refs/heads/main branch.my4.merge
 '
 
 test_expect_success 'tracking setup fails on non-matching refspec' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
-	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
+	test_config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	test_must_fail git branch --track my5 local/main &&
 	test_must_fail git config branch.my5.remote &&
 	test_must_fail git config branch.my5.merge
 '
 
 test_expect_success 'test tracking setup via config' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch my3 local/main &&
-	test $(git config branch.my3.remote) = local &&
-	test $(git config branch.my3.merge) = refs/heads/main
+	test_cmp_config local branch.my3.remote &&
+	test_cmp_config refs/heads/main branch.my3.merge
 '
 
 test_expect_success 'test overriding tracking setup via --no-track' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track my2 local/main &&
-	git config branch.autosetupmerge false &&
-	! test "$(git config branch.my2.remote)" = local &&
-	! test "$(git config branch.my2.merge)" = refs/heads/main
+	! test_cmp_config local branch.my2.remote &&
+	! test_cmp_config refs/heads/main branch.my2.merge
 '
 
 test_expect_success 'no tracking without .fetch entries' '
-	git config branch.autosetupmerge true &&
+	test_config branch.autosetupmerge true &&
 	git branch my6 s &&
-	git config branch.autosetupmerge false &&
-	test -z "$(git config branch.my6.remote)" &&
-	test -z "$(git config branch.my6.merge)"
+	test_cmp_config "" --default "" branch.my6.remote &&
+	test_cmp_config "" --default "" branch.my6.merge
 '
 
 test_expect_success 'test tracking setup via --track but deeper' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o/o || git fetch local) &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing o/o local &&
 	git branch --track my7 local/o/o &&
-	test "$(git config branch.my7.remote)" = local &&
-	test "$(git config branch.my7.merge)" = refs/heads/o/o
+	test_cmp_config local branch.my7.remote &&
+	test_cmp_config refs/heads/o/o branch.my7.merge
 '
 
 test_expect_success 'test deleting branch deletes branch config' '
 	git branch -d my7 &&
-	test -z "$(git config branch.my7.remote)" &&
-	test -z "$(git config branch.my7.merge)"
+	test_cmp_config "" --default "" branch.my7.remote &&
+	test_cmp_config "" --default "" branch.my7.merge
 '
 
 test_expect_success 'test deleting branch without config' '

@@ -850,14 +847,15 @@ test_expect_success 'deleting currently checked out branch fails' '
 
 test_expect_success 'test --track without .fetch entries' '
 	git branch --track my8 &&
-	test "$(git config branch.my8.remote)" &&
-	test "$(git config branch.my8.merge)"
+	git config branch.my8.remote >out &&
+	test -s out &&
+	git config branch.my8.merge >out &&
+	test -s out
 '
 
 test_expect_success 'branch from non-branch HEAD w/autosetupmerge=always' '
-	git config branch.autosetupmerge always &&
-	git branch my9 HEAD^ &&
-	git config branch.autosetupmerge false
+	test_config branch.autosetupmerge always &&
+	git branch my9 HEAD^
 '
 
 test_expect_success 'branch from non-branch HEAD w/--track causes failure' '

@@ -913,16 +911,16 @@ test_expect_success 'use --set-upstream-to modify HEAD' '
 	test_config branch.main.merge foo &&
 	git branch my12 &&
 	git branch --set-upstream-to my12 &&
-	test "$(git config branch.main.remote)" = "." &&
-	test "$(git config branch.main.merge)" = "refs/heads/my12"
+	test_cmp_config "." branch.main.remote &&
+	test_cmp_config "refs/heads/my12" branch.main.merge
 '
 
 test_expect_success 'use --set-upstream-to modify a particular branch' '
 	git branch my13 &&
 	git branch --set-upstream-to main my13 &&
 	test_when_finished "git branch --unset-upstream my13" &&
-	test "$(git config branch.my13.remote)" = "." &&
-	test "$(git config branch.my13.merge)" = "refs/heads/main"
+	test_cmp_config "." branch.my13.remote &&
+	test_cmp_config "refs/heads/main" branch.my13.merge
 '
 
 test_expect_success '--unset-upstream should fail if given a non-existent branch' '

@@ -1003,273 +1001,251 @@ test_expect_success 'git checkout -b g/h/i -l should create a branch and a log'
 
 test_expect_success 'checkout -b makes reflog by default' '
 	git checkout main &&
-	git config --unset core.logAllRefUpdates &&
+	test_unconfig core.logAllRefUpdates &&
 	git checkout -b alpha &&
 	git rev-parse --verify alpha@{0}
 '
 
 test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
-	git config core.logAllRefUpdates false &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -b beta &&
 	test_must_fail git rev-parse --verify beta@{0}
 '
 
 test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -lb gamma &&
-	git config --unset core.logAllRefUpdates &&
 	git rev-parse --verify gamma@{0}
 '
 
 test_expect_success 'avoid ambiguous track' '
-	git config branch.autosetupmerge true &&
-	git config remote.ambi1.url lalala &&
-	git config remote.ambi1.fetch refs/heads/lalala:refs/heads/main &&
-	git config remote.ambi2.url lilili &&
-	git config remote.ambi2.fetch refs/heads/lilili:refs/heads/main &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote ambi1 lalala refs/heads/lalala:refs/heads/main &&
+	test_set_remote ambi2 lilili refs/heads/lilili:refs/heads/main &&
 	test_must_fail git branch all1 main &&
-	test -z "$(git config branch.all1.merge)"
+	test_cmp_config "" --default "" branch.all1.merge
 '
 
 test_expect_success 'autosetuprebase local on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	fetch_if_remote_ref_missing o local &&
 	git branch mybase &&
 	git branch --track myr1 mybase &&
-	test "$(git config branch.myr1.remote)" = . &&
-	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr1.rebase)" = true
+	test_cmp_config . branch.myr1.remote &&
+	test_cmp_config refs/heads/mybase branch.myr1.merge &&
+	test_cmp_config true branch.myr1.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase always &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase always &&
+	fetch_if_remote_ref_missing o local &&
 	git branch mybase2 &&
 	git branch --track myr2 mybase &&
-	test "$(git config branch.myr2.remote)" = . &&
-	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr2.rebase)" = true
+	test_cmp_config . branch.myr2.remote &&
+	test_cmp_config refs/heads/mybase branch.myr2.merge &&
+	test_cmp_config true branch.myr2.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	fetch_if_remote_ref_missing o local &&
 	git branch mybase3 &&
 	git branch --track myr3 mybase2 &&
-	test "$(git config branch.myr3.remote)" = . &&
-	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr3.rebase)" = true
+	test_cmp_config . branch.myr3.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr3.merge &&
+	! test_cmp_config true branch.myr3.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	fetch_if_remote_ref_missing o local &&
 	git branch mybase4 &&
 	git branch --track myr4 mybase2 &&
-	test "$(git config branch.myr4.remote)" = . &&
-	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr4.rebase)" = true
+	test_cmp_config . branch.myr4.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr4.merge &&
+	! test_cmp_config true branch.myr4.rebase
 '
 
 test_expect_success 'autosetuprebase local on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track myr5 local/main &&
-	test "$(git config branch.myr5.remote)" = local &&
-	test "$(git config branch.myr5.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr5.rebase)" = true
+	test_cmp_config local branch.myr5.remote &&
+	test_cmp_config refs/heads/main branch.myr5.merge &&
+	! test_cmp_config true branch.myr5.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track myr6 local/main &&
-	test "$(git config branch.myr6.remote)" = local &&
-	test "$(git config branch.myr6.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr6.rebase)" = true
+	test_cmp_config local branch.myr6.remote &&
+	test_cmp_config refs/heads/main branch.myr6.merge &&
+	! test_cmp_config true branch.myr6.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track myr7 local/main &&
-	test "$(git config branch.myr7.remote)" = local &&
-	test "$(git config branch.myr7.merge)" = refs/heads/main &&
-	test "$(git config branch.myr7.rebase)" = true
+	test_cmp_config local branch.myr7.remote &&
+	test_cmp_config refs/heads/main branch.myr7.merge &&
+	test_cmp_config true branch.myr7.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track myr8 local/main &&
-	test "$(git config branch.myr8.remote)" = local &&
-	test "$(git config branch.myr8.merge)" = refs/heads/main &&
-	test "$(git config branch.myr8.rebase)" = true
+	test_cmp_config local branch.myr8.remote &&
+	test_cmp_config refs/heads/main branch.myr8.merge &&
+	test_cmp_config true branch.myr8.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
-	git config --unset branch.autosetuprebase &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_unconfig branch.autosetuprebase &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --track myr9 local/main &&
-	test "$(git config branch.myr9.remote)" = local &&
-	test "$(git config branch.myr9.merge)" = refs/heads/main &&
-	test "z$(git config branch.myr9.rebase)" = z
+	test_cmp_config local branch.myr9.remote &&
+	test_cmp_config refs/heads/main branch.myr9.merge &&
+	test_cmp_config "" --default "" branch.myr9.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing o local &&
 	git branch mybase10 &&
 	git branch --track myr10 mybase2 &&
-	test "$(git config branch.myr10.remote)" = . &&
-	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
-	test "z$(git config branch.myr10.rebase)" = z
+	test_cmp_config . branch.myr10.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr10.merge &&
+	test_cmp_config "" --default "" branch.myr10.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr11 mybase2 &&
-	test "z$(git config branch.myr11.remote)" = z &&
-	test "z$(git config branch.myr11.merge)" = z &&
-	test "z$(git config branch.myr11.rebase)" = z
+	test_cmp_config "" --default "" branch.myr11.remote &&
+	test_cmp_config "" --default "" branch.myr11.merge &&
+	test_cmp_config "" --default "" branch.myr11.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr12 local/main &&
-	test "z$(git config branch.myr12.remote)" = z &&
-	test "z$(git config branch.myr12.merge)" = z &&
-	test "z$(git config branch.myr12.rebase)" = z
+	test_cmp_config "" --default "" branch.myr12.remote &&
+	test_cmp_config "" --default "" branch.myr12.merge &&
+	test_cmp_config "" --default "" branch.myr12.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked local branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr13 mybase2 &&
-	test "z$(git config branch.myr13.remote)" = z &&
-	test "z$(git config branch.myr13.merge)" = z &&
-	test "z$(git config branch.myr13.rebase)" = z
+	test_cmp_config "" --default "" branch.myr13.remote &&
+	test_cmp_config "" --default "" branch.myr13.merge &&
+	test_cmp_config "" --default "" branch.myr13.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked local branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr14 mybase2 &&
-	test "z$(git config branch.myr14.remote)" = z &&
-	test "z$(git config branch.myr14.merge)" = z &&
-	test "z$(git config branch.myr14.rebase)" = z
+	test_cmp_config "" --default "" branch.myr14.remote &&
+	test_cmp_config "" --default "" branch.myr14.merge &&
+	test_cmp_config "" --default "" branch.myr14.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked local branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr15 mybase2 &&
-	test "z$(git config branch.myr15.remote)" = z &&
-	test "z$(git config branch.myr15.merge)" = z &&
-	test "z$(git config branch.myr15.rebase)" = z
+	test_cmp_config "" --default "" branch.myr15.remote &&
+	test_cmp_config "" --default "" branch.myr15.merge &&
+	test_cmp_config "" --default "" branch.myr15.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked local branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr16 mybase2 &&
-	test "z$(git config branch.myr16.remote)" = z &&
-	test "z$(git config branch.myr16.merge)" = z &&
-	test "z$(git config branch.myr16.rebase)" = z
+	test_cmp_config "" --default "" branch.myr16.remote &&
+	test_cmp_config "" --default "" branch.myr16.merge &&
+	test_cmp_config "" --default "" branch.myr16.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked remote branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr17 local/main &&
-	test "z$(git config branch.myr17.remote)" = z &&
-	test "z$(git config branch.myr17.merge)" = z &&
-	test "z$(git config branch.myr17.rebase)" = z
+	test_cmp_config "" --default "" branch.myr17.remote &&
+	test_cmp_config "" --default "" branch.myr17.merge &&
+	test_cmp_config "" --default "" branch.myr17.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked remote branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr18 local/main &&
-	test "z$(git config branch.myr18.remote)" = z &&
-	test "z$(git config branch.myr18.merge)" = z &&
-	test "z$(git config branch.myr18.rebase)" = z
+	test_cmp_config "" --default "" branch.myr18.remote &&
+	test_cmp_config "" --default "" branch.myr18.merge &&
+	test_cmp_config "" --default "" branch.myr18.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked remote branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr19 local/main &&
-	test "z$(git config branch.myr19.remote)" = z &&
-	test "z$(git config branch.myr19.merge)" = z &&
-	test "z$(git config branch.myr19.rebase)" = z
+	test_cmp_config "" --default "" branch.myr19.remote &&
+	test_cmp_config "" --default "" branch.myr19.merge &&
+	test_cmp_config "" --default "" branch.myr19.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked remote branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	fetch_if_remote_ref_missing main local &&
 	git branch --no-track myr20 local/main &&
-	test "z$(git config branch.myr20.remote)" = z &&
-	test "z$(git config branch.myr20.merge)" = z &&
-	test "z$(git config branch.myr20.rebase)" = z
+	test_cmp_config "" --default "" branch.myr20.remote &&
+	test_cmp_config "" --default "" branch.myr20.merge &&
+	test_cmp_config "" --default "" branch.myr20.rebase
 '
 
 test_expect_success 'autosetuprebase always on detached HEAD' '
-	git config branch.autosetupmerge always &&
+	test_config branch.autosetupmerge always &&
 	test_when_finished git checkout main &&
 	git checkout HEAD^0 &&
 	git branch my11 &&
-	test -z "$(git config branch.my11.remote)" &&
-	test -z "$(git config branch.my11.merge)"
+	test_cmp_config "" --default "" branch.my11.remote &&
+	test_cmp_config "" --default "" branch.my11.rebase
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (bad value)' '
-	git config branch.autosetuprebase garbage &&
+	test_config branch.autosetuprebase garbage &&
 	test_must_fail git branch
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (no value)' '
-	git config --unset branch.autosetuprebase &&
+	test_unconfig branch.autosetuprebase &&
 	echo "[branch] autosetuprebase" >>.git/config &&
 	test_must_fail git branch &&
 	git config --unset branch.autosetuprebase

@@ -1277,7 +1253,7 @@ test_expect_success 'detect misconfigured autosetuprebase (no value)' '
 
 test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '
 	git checkout my9 &&
-	git config --unset branch.my8.merge &&
+	test_unconfig branch.my8.merge &&
 	test_must_fail git branch -d my8
 '

@@ -1285,7 +1261,7 @@ test_expect_success 'attempt to delete a branch merged to its base' '
 	# we are on my9 which is the initial commit; traditionally
 	# we would not have allowed deleting my8 that is not merged
 	# to my9, but it is set to track main that already has my8
-	git config branch.my8.merge refs/heads/main &&
+	test_config branch.my8.merge refs/heads/main &&
 	git branch -d my8
 '

@@ -1397,8 +1373,8 @@ test_expect_success 'tracking with unexpected .fetch refspec' '
 		git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
 		git fetch c &&
 		git branch --track local/a/main remotes/a/main &&
-		test "$(git config branch.local/a/main.remote)" = "c" &&
-		test "$(git config branch.local/a/main.merge)" = "refs/remotes/a/main" &&
+		test_cmp_config "c" branch.local/a/main.remote &&
+		test_cmp_config "refs/remotes/a/main" branch.local/a/main.merge &&
 		git rev-parse --verify a >expect &&
 		git rev-parse --verify local/a/main >actual &&
 		test_cmp expect actual

base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a

Signature · 1 line

gitgitgadget

Ævar Arnfjörð Bjarmason

2022-03-21 13:58 UTC #1 lore raw

t/t3200-branch.sh export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME · line 26

Quoted 8 lines · Tao Klerks via GitGitGadget

t/t3200-branch.sh +8 -0

+fetch_if_remote_ref_missing () {
+	# this is an anti-pattern: swallows segfault
+	#git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+	# this is slightly slower, up to 1s out of 6s on this set of tests:
+	git fetch "$2"
+	# this doesn't work
+	#test_might_fail git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+}

Moving the context around a bit, as this refers to this code above:

Quoted 11 lines

I'm submitting this as RFC because I have a couple of significant doubts:
1. Does it make sense to do this? I believe it's a good idea to keep things "clean" so that newcomers more easily do the right thing than the wrong thing, but on the other hand, I've definitely read that we have a "don't change things unnecessarily" bias somewhere.
2. What's the right pattern for the "(git show-ref -q refs/remotes/local/main || git fetch local)" fetch-avoidance optimization? Removing it adds a second to test runtimes, but Ævar warned it hides segfaults

So first, 6s? Is this on Windows? I tried running this v.s. master:

    $ git hyperfine -L rev origin/master,HEAD~,HEAD -s 'make' '(cd t && ./t3200-branch.sh)'
    Benchmark 1: (cd t && ./t3200-branch.sh)' in 'origin/master
      Time (mean ± σ):      1.887 s ±  0.095 s    [User: 1.534 s, System: 0.514 s]
      Range (min … max):    1.826 s …  2.117 s    10 runs

    Benchmark 2: (cd t && ./t3200-branch.sh)' in 'HEAD~
      Time (mean ± σ):      2.132 s ±  0.013 s    [User: 1.742 s, System: 0.561 s]
      Range (min … max):    2.120 s …  2.166 s    10 runs
    
    Benchmark 3: (cd t && ./t3200-branch.sh)' in 'HEAD
      Time (mean ± σ):      1.944 s ±  0.005 s    [User: 1.620 s, System: 0.495 s]
      Range (min … max):    1.938 s …  1.953 s    10 runs
    
    Summary
      '(cd t && ./t3200-branch.sh)' in 'origin/master' ran
        1.03 ± 0.05 times faster than '(cd t && ./t3200-branch.sh)' in 'HEAD'
        1.13 ± 0.06 times faster than '(cd t && ./t3200-branch.sh)' in 'HEAD~'

The HEAD~ there is your patch here, and HEAD is my fix-up. I.e.:

	diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
	index 9bd621ed97e..7f7b3b28581 100755
	--- a/t/t3200-branch.sh
	+++ b/t/t3200-branch.sh
	@@ -17,12 +17,14 @@ test_set_remote () {
	 }
	 
	 fetch_if_remote_ref_missing () {
	-	# this is an anti-pattern: swallows segfault
	-	#git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
	-	# this is slightly slower, up to 1s out of 6s on this set of tests:
	-	git fetch "$2"
	-	# this doesn't work
	-	#test_might_fail git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
	+	test_when_finished "rm -f ref" &&
	+	test_might_fail git rev-parse -q --verify "refs/remotes/$2/$1" >ref
	+	if ! test -s ref
	+	then
	+		# Purely an optimization, makes the test run ~10%
	+		# faster.
	+		git fetch "$2"
	+	fi
	 }
	 
	 test_expect_success 'prepare a trivial repository' '

That's a safe way to do it that won't hide segfaults.

In *general* it's a bit painful to convert some of these, because we really should refactor out the whole bit after "exit_code=$?" in test_must_fail in test-lib-functions.sh into a utility function. I.e. have the ability to run an arbitrary command, and then after-the-fact ask if its exit code was OK.

If you'd like to refactor that that would be most welcome, and it *would* help to convert some of these...

But in this case we can just use "rev-parse -q --verify", or rather, nothing :)

I.e. my bias would be to just not try to optimize this, i.e. just convert the users to the equivalent of a:

    git fetch "$2"

I.e. it's also useful to see that we behave correctly in the noop case, and as there's no behavior difference it's a marginally more useful test as a result.

And if you are trying to optimize this on Windows as I suspect I think it's better to not do it. ~5s is the time it takes it just to get out of bed in the morning as far as our test runtimes are concerned.

The real underlying issue is presumably its the shelling we'll do in "git fetch", which we can eventually fix, and then make it approximately the cost of the rev-parse when run locally...

Tao Klerks

2022-03-22 19:23 UTC #2 lore raw

Quoted 27 lines · Ævar Arnfjörð Bjarmason

t/t3200-branch.sh export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME · line 26
Quoted 8 lines · Tao Klerks via GitGitGadget
t/t3200-branch.sh +8 -0
+fetch_if_remote_ref_missing () {
+     # this is an anti-pattern: swallows segfault
+     #git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+     # this is slightly slower, up to 1s out of 6s on this set of tests:
+     git fetch "$2"
+     # this doesn't work
+     #test_might_fail git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
+}
Moving the context around a bit, as this refers to this code above:
Quoted 11 lines
I'm submitting this as RFC because I have a couple of significant doubts:
1. Does it make sense to do this? I believe it's a good idea to keep things "clean" so that newcomers more easily do the right thing than the wrong thing, but on the other hand, I've definitely read that we have a "don't change things unnecessarily" bias somewhere.
2. What's the right pattern for the "(git show-ref -q refs/remotes/local/main || git fetch local)" fetch-avoidance optimization? Removing it adds a second to test runtimes, but Ævar warned it hides segfaults

Quoted 1 line

So first, 6s? Is this on Windows?

Eh, kind of. It's Ubuntu running under a WSL2 VM, which in my experience so far runs *almost* as fast as bare-metal - certainly with none of the per-process or per-disk-access overheads of Windows.

It looks like my hardware is a little more "vintage" than yours, and more importantly during my initial testing I had some significant overhead and variability from VS Code's server trying to track file changes.

Quoted 7 lines

I tried running this v.s. master:

    $ git hyperfine -L rev origin/master,HEAD~,HEAD -s 'make' '(cd t && ./t3200-branch.sh)'
    Benchmark 1: (cd t && ./t3200-branch.sh)' in 'origin/master
      Time (mean ± σ):      1.887 s ±  0.095 s    [User: 1.534 s, System: 0.514 s]
      Range (min … max):    1.826 s …  2.117 s    10 runs

Quoted 13 lines

    Benchmark 2: (cd t && ./t3200-branch.sh)' in 'HEAD~
      Time (mean ± σ):      2.132 s ±  0.013 s    [User: 1.742 s, System: 0.561 s]
      Range (min … max):    2.120 s …  2.166 s    10 runs

    Benchmark 3: (cd t && ./t3200-branch.sh)' in 'HEAD
      Time (mean ± σ):      1.944 s ±  0.005 s    [User: 1.620 s, System: 0.495 s]
      Range (min … max):    1.938 s …  1.953 s    10 runs

    Summary
      '(cd t && ./t3200-branch.sh)' in 'origin/master' ran
        1.03 ± 0.05 times faster than '(cd t && ./t3200-branch.sh)' in 'HEAD'
        1.13 ± 0.06 times faster than '(cd t && ./t3200-branch.sh)' in 'HEAD~'

When applying this more rigorous testing approach (without your git-hyperfine setup, which I haven't understood yet), without VSCode in the way, I get slower but similar outcomes:

~/git/t$ git checkout cleanup-t3200-tests 2>/dev/null && hyperfine './t3200-branch.sh' && git checkout cleanup-t3200-tests~ 2>/dev/null && hyperfine './t3200-branch.sh' && git checkout cleanup-t3200-tests~2 2>/dev/null && hyperfine './t3200-branch.sh'

Benchmark 1: ./t3200-branch.sh
Time (mean ± σ): 3.372 s ± 0.030 s [User: 2.945 s, System: 0.825 s]
Range (min … max): 3.336 s … 3.417 s 10 runs

Benchmark 1: ./t3200-branch.sh
Time (mean ± σ): 3.630 s ± 0.032 s [User: 3.134 s, System: 0.898 s]
Range (min … max): 3.592 s … 3.668 s 10 runs

Benchmark 1: ./t3200-branch.sh
Time (mean ± σ): 3.097 s ± 0.055 s [User: 2.741 s, System: 0.730 s]
Range (min … max): 3.018 s … 3.216 s 10 runs

Upshot: some of my other changes had improved performance by 10%, the unconditional git fetch had worsened performance by 20%, and your change fixed the latter.

Quoted 3 lines

That's a safe way to do it that won't hide segfaults.

Thx!

Quoted 8 lines

In *general* it's a bit painful to convert some of these, because we really should refactor out the whole bit after "exit_code=$?" in test_must_fail in test-lib-functions.sh into a utility function. I.e. have the ability to run an arbitrary command, and then after-the-fact ask if its exit code was OK.
If you'd like to refactor that that would be most welcome, and it *would* help to convert some of these...

I'm interested, but this looks like it would require bash-fu far beyond my level.

Quoted 4 lines

But in this case we can just use "rev-parse -q --verify", or rather, nothing :)

Quoted 8 lines

I.e. my bias would be to just not try to optimize this, i.e. just convert the users to the equivalent of a:
    git fetch "$2"
I.e. it's also useful to see that we behave correctly in the noop case, and as there's no behavior difference it's a marginally more useful test as a result.

I will happily buy this argument; I also like that the simple "git fetch" call is inherently clearer/more legible than any alternative.

Quoted 8 lines

And if you are trying to optimize this on Windows as I suspect I think it's better to not do it. ~5s is the time it takes it just to get out of bed in the morning as far as our test runtimes are concerned.
The real underlying issue is presumably its the shelling we'll do in "git fetch", which we can eventually fix, and then make it approximately the cost of the rev-parse when run locally...

Makes sense, but not the case. I was just being oversensitive I guess.

Tao Klerks via GitGitGadget

author

2022-03-23 00:24 UTC #3 lore raw

PATCH v2 1/1 t3200: fix antipatterns in existing branch tests

In reply to Tao Klerks via GitGitGadget

From: Tao Klerks <[email protected]>

Fix issues in t3200 branch tests that, if copied, might catch new contributors out:

Use test_config to show that config state is not being intentionally left to spill over into other tests.

Use test_cmp_config instead of git config in subshells, so that git's error code is not lost if/when an unexpected error occurs.

Use output redirection and later content checking instead of subshells, so that git's error code is not lost if/when an unexpected error occurs.

Eliminate local-fetch-avoiding optimization as it is error-prone (it is easy to check the wrong thing), hides segfaults, and yields only a marginal performance improvement given the fetch is local.

Introduce local helper test_set_remote to simplify the common local pattern of setting up a remote via config.

Signed-off-by Tao Klerks <[email protected]>

---
t3200: fix antipatterns in existing branch tests

This is a cleanup of the branch tests following a case where I was adding some, and did substantially the wrong thing by following existing examples.

I believe this third version addresses all my concerns and Ævar's suggestions.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1182%2FTaoK%2Fcleanup-t3200-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1182/TaoK/cleanup-t3200-tests-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1182

Range-diff vs v1:

1: 983254bc2eb ! 1: e4addb910f7 t3200: fix antipatterns in existing branch tests

     @@ Commit message
          subshells, so that git's error code is not lost if/when an
          unexpected error occurs.
      
     -    Try to eliminate local-fetch-avoiding optimization as it is
     -    error-prone (it is easy to check the wrong thing), hides segfaults,
     -    and yields only a marginal performance improvement anyway.
     +    Eliminate local-fetch-avoiding optimization as it is error-prone (it
     +    is easy to check the wrong thing), hides segfaults, and yields only
     +    a marginal performance improvement given the fetch is local.
      
          Introduce local helper test_set_remote to simplify the common local
          pattern of setting up a remote via config.
     @@ t/t3200-branch.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      +	test_config "remote.$1.url" "$2" &&
      +	test_config "remote.$1.fetch" "${3:-"refs/heads/*:refs/remotes/$1/*"}"
      +}
     -+
     -+fetch_if_remote_ref_missing () {
     -+	# this is an anti-pattern: swallows segfault
     -+	#git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
     -+	# this is slightly slower, up to 1s out of 6s on this set of tests:
     -+	git fetch "$2"
     -+	# this doesn't work
     -+	#test_might_fail git show-ref -q "refs/remotes/$2/$1" || git fetch "$2"
     -+}
      +
       test_expect_success 'prepare a trivial repository' '
       	echo Hello >A &&
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track my1 local/main &&
      -	test $(git config branch.my1.remote) = local &&
      -	test $(git config branch.my1.merge) = refs/heads/main
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	git config remote.local.fetch refs/heads/main:refs/remotes/local/main &&
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . refs/heads/main:refs/remotes/local/main &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track my4 local/main &&
      -	test $(git config branch.my4.remote) = local &&
      -	test $(git config branch.my4.merge) = refs/heads/main
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      -	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
      +	test_config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
       	test_must_fail git branch --track my5 local/main &&
       	test_must_fail git config branch.my5.remote &&
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetupmerge true &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch my3 local/main &&
      -	test $(git config branch.my3.remote) = local &&
      -	test $(git config branch.my3.merge) = refs/heads/main
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetupmerge true &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track my2 local/main &&
      -	git config branch.autosetupmerge false &&
      -	! test "$(git config branch.my2.remote)" = local &&
     @@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .g
      -	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      -	(git show-ref -q refs/remotes/local/o/o || git fetch local) &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing o/o local &&
     ++	git fetch local &&
       	git branch --track my7 local/o/o &&
      -	test "$(git config branch.my7.remote)" = local &&
      -	test "$(git config branch.my7.merge)" = refs/heads/o/o
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/o || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase local &&
     -+	fetch_if_remote_ref_missing o local &&
     ++	git fetch local &&
       	git branch mybase &&
       	git branch --track myr1 mybase &&
      -	test "$(git config branch.myr1.remote)" = . &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/o || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase always &&
     -+	fetch_if_remote_ref_missing o local &&
     ++	git fetch local &&
       	git branch mybase2 &&
       	git branch --track myr2 mybase &&
      -	test "$(git config branch.myr2.remote)" = . &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/o || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase remote &&
     -+	fetch_if_remote_ref_missing o local &&
     ++	git fetch local &&
       	git branch mybase3 &&
       	git branch --track myr3 mybase2 &&
      -	test "$(git config branch.myr3.remote)" = . &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/o || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase never &&
     -+	fetch_if_remote_ref_missing o local &&
     ++	git fetch local &&
       	git branch mybase4 &&
       	git branch --track myr4 mybase2 &&
      -	test "$(git config branch.myr4.remote)" = . &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase local &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track myr5 local/main &&
      -	test "$(git config branch.myr5.remote)" = local &&
      -	test "$(git config branch.myr5.merge)" = refs/heads/main &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase never &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track myr6 local/main &&
      -	test "$(git config branch.myr6.remote)" = local &&
      -	test "$(git config branch.myr6.merge)" = refs/heads/main &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase remote &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track myr7 local/main &&
      -	test "$(git config branch.myr7.remote)" = local &&
      -	test "$(git config branch.myr7.merge)" = refs/heads/main &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
      +	test_config branch.autosetuprebase remote &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track myr8 local/main &&
      -	test "$(git config branch.myr8.remote)" = local &&
      -	test "$(git config branch.myr8.merge)" = refs/heads/main &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_unconfig branch.autosetuprebase &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --track myr9 local/main &&
      -	test "$(git config branch.myr9.remote)" = local &&
      -	test "$(git config branch.myr9.merge)" = refs/heads/main &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      -	(git show-ref -q refs/remotes/local/o || git fetch local) &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing o local &&
     ++	git fetch local &&
       	git branch mybase10 &&
       	git branch --track myr10 mybase2 &&
      -	test "$(git config branch.myr10.remote)" = . &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr11 mybase2 &&
      -	test "z$(git config branch.myr11.remote)" = z &&
      -	test "z$(git config branch.myr11.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr12 local/main &&
      -	test "z$(git config branch.myr12.remote)" = z &&
      -	test "z$(git config branch.myr12.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase never &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr13 mybase2 &&
      -	test "z$(git config branch.myr13.remote)" = z &&
      -	test "z$(git config branch.myr13.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase local &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr14 mybase2 &&
      -	test "z$(git config branch.myr14.remote)" = z &&
      -	test "z$(git config branch.myr14.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase remote &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr15 mybase2 &&
      -	test "z$(git config branch.myr15.remote)" = z &&
      -	test "z$(git config branch.myr15.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase always &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr16 mybase2 &&
      -	test "z$(git config branch.myr16.remote)" = z &&
      -	test "z$(git config branch.myr16.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase never &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr17 local/main &&
      -	test "z$(git config branch.myr17.remote)" = z &&
      -	test "z$(git config branch.myr17.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase local &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr18 local/main &&
      -	test "z$(git config branch.myr18.remote)" = z &&
      -	test "z$(git config branch.myr18.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase remote &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr19 local/main &&
      -	test "z$(git config branch.myr19.remote)" = z &&
      -	test "z$(git config branch.myr19.merge)" = z &&
     @@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
      -	(git show-ref -q refs/remotes/local/main || git fetch local) &&
      +	test_config branch.autosetuprebase always &&
      +	test_set_remote local . &&
     -+	fetch_if_remote_ref_missing main local &&
     ++	git fetch local &&
       	git branch --no-track myr20 local/main &&
      -	test "z$(git config branch.myr20.remote)" = z &&
      -	test "z$(git config branch.myr20.merge)" = z &&

 t/t3200-branch.sh | 439 +++++++++++++++++++++-------------------------
 1 file changed, 203 insertions(+), 236 deletions(-)

t/t3200-branch.sh +203 ~~-236~~

index 1bc3795847d..30292d89f2d 100755
@@ -11,6 +11,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+test_set_remote () {
+	test_config "remote.$1.url" "$2" &&
+	test_config "remote.$1.fetch" "${3:-"refs/heads/*:refs/remotes/$1/*"}"
+}
+
 test_expect_success 'prepare a trivial repository' '
 	echo Hello >A &&
 	git update-index --add A &&

@@ -372,11 +377,9 @@ EOF
 '
 
 test_expect_success 'git branch with column.*' '
-	git config column.ui column &&
-	git config column.branch "dense" &&
+	test_config column.ui column &&
+	test_config column.branch "dense" &&
 	COLUMNS=80 git branch >actual &&
-	git config --unset column.branch &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c   bam   foo   l   * main   n     o/p   r
   abc     bar   j/k   m/m   mb     o/o   q     topic

@@ -389,9 +392,8 @@ test_expect_success 'git branch --column -v should fail' '
 '
 
 test_expect_success 'git branch -v with column.ui ignored' '
-	git config column.ui column &&
+	test_config column.ui column &&
 	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
   abc

@@ -435,7 +437,7 @@ test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
 '
 
 test_expect_success 'config information was renamed, too' '
-	test $(git config branch.s.dummy) = Hello &&
+	test_cmp_config Hello branch.s.dummy &&
 	test_must_fail git config branch.s/s.dummy
 '

@@ -493,63 +495,57 @@ test_expect_success 'git branch --copy dumps usage' '
 test_expect_success 'git branch -c d e should work' '
 	git branch --create-reflog d &&
 	git reflog exists refs/heads/d &&
-	git config branch.d.dummy Hello &&
+	test_config branch.d.dummy Hello &&
 	git branch -c d e &&
 	git reflog exists refs/heads/d &&
 	git reflog exists refs/heads/e &&
-	echo Hello >expect &&
-	git config branch.e.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.d.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.e.dummy &&
+	test_cmp_config Hello branch.d.dummy
 '
 
 test_expect_success 'git branch --copy is a synonym for -c' '
 	git branch --create-reflog copy &&
 	git reflog exists refs/heads/copy &&
-	git config branch.copy.dummy Hello &&
+	test_config branch.copy.dummy Hello &&
 	git branch --copy copy copy-to &&
 	git reflog exists refs/heads/copy &&
 	git reflog exists refs/heads/copy-to &&
-	echo Hello >expect &&
-	git config branch.copy.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.copy-to.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.copy.dummy &&
+	test_cmp_config Hello branch.copy-to.dummy
 '
 
 test_expect_success 'git branch -c ee ef should copy ee to create branch ef' '
 	git checkout -b ee &&
 	git reflog exists refs/heads/ee &&
-	git config branch.ee.dummy Hello &&
+	test_config branch.ee.dummy Hello &&
 	git branch -c ee ef &&
 	git reflog exists refs/heads/ee &&
 	git reflog exists refs/heads/ef &&
-	test $(git config branch.ee.dummy) = Hello &&
-	test $(git config branch.ef.dummy) = Hello &&
-	test $(git rev-parse --abbrev-ref HEAD) = ee
+	test_cmp_config Hello branch.ee.dummy &&
+	test_cmp_config Hello branch.ef.dummy &&
+	echo ee >expect &&
+	git rev-parse --abbrev-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch -c f/f g/g should work' '
 	git branch --create-reflog f/f &&
 	git reflog exists refs/heads/f/f &&
-	git config branch.f/f.dummy Hello &&
+	test_config branch.f/f.dummy Hello &&
 	git branch -c f/f g/g &&
 	git reflog exists refs/heads/f/f &&
 	git reflog exists refs/heads/g/g &&
-	test $(git config branch.f/f.dummy) = Hello &&
-	test $(git config branch.g/g.dummy) = Hello
+	test_cmp_config Hello branch.f/f.dummy &&
+	test_cmp_config Hello branch.g/g.dummy
 '
 
 test_expect_success 'git branch -c m2 m2 should work' '
 	git branch --create-reflog m2 &&
 	git reflog exists refs/heads/m2 &&
-	git config branch.m2.dummy Hello &&
+	test_config branch.m2.dummy Hello &&
 	git branch -c m2 m2 &&
 	git reflog exists refs/heads/m2 &&
-	test $(git config branch.m2.dummy) = Hello
+	test_cmp_config Hello branch.m2.dummy
 '
 
 test_expect_success 'git branch -c zz zz/zz should fail' '

@@ -619,15 +615,15 @@ test_expect_success 'git branch -C main5 main5 should work when main is checked
 test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
 	git branch --create-reflog cd &&
 	git reflog exists refs/heads/cd &&
-	git config branch.cd.dummy CD &&
+	test_config branch.cd.dummy CD &&
 	git branch --create-reflog ab &&
 	git reflog exists refs/heads/ab &&
-	git config branch.ab.dummy AB &&
+	test_config branch.ab.dummy AB &&
 	git branch -C ab cd &&
 	git reflog exists refs/heads/ab &&
 	git reflog exists refs/heads/cd &&
-	test $(git config branch.ab.dummy) = AB &&
-	test $(git config branch.cd.dummy) = AB
+	test_cmp_config AB branch.ab.dummy &&
+	test_cmp_config AB branch.cd.dummy
 '
 
 test_expect_success 'git branch -c correctly copies multiple config sections' '

@@ -761,75 +757,67 @@ test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
 '
 
 test_expect_success 'test tracking setup via --track' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track my1 local/main &&
-	test $(git config branch.my1.remote) = local &&
-	test $(git config branch.my1.merge) = refs/heads/main
+	test_cmp_config local branch.my1.remote &&
+	test_cmp_config refs/heads/main branch.my1.merge
 '
 
 test_expect_success 'test tracking setup (non-wildcard, matching)' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/main:refs/remotes/local/main &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . refs/heads/main:refs/remotes/local/main &&
+	git fetch local &&
 	git branch --track my4 local/main &&
-	test $(git config branch.my4.remote) = local &&
-	test $(git config branch.my4.merge) = refs/heads/main
+	test_cmp_config local branch.my4.remote &&
+	test_cmp_config refs/heads/main branch.my4.merge
 '
 
 test_expect_success 'tracking setup fails on non-matching refspec' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
-	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
+	test_set_remote local . &&
+	git fetch local &&
+	test_config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	test_must_fail git branch --track my5 local/main &&
 	test_must_fail git config branch.my5.remote &&
 	test_must_fail git config branch.my5.merge
 '
 
 test_expect_success 'test tracking setup via config' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch my3 local/main &&
-	test $(git config branch.my3.remote) = local &&
-	test $(git config branch.my3.merge) = refs/heads/main
+	test_cmp_config local branch.my3.remote &&
+	test_cmp_config refs/heads/main branch.my3.merge
 '
 
 test_expect_success 'test overriding tracking setup via --no-track' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track my2 local/main &&
-	git config branch.autosetupmerge false &&
-	! test "$(git config branch.my2.remote)" = local &&
-	! test "$(git config branch.my2.merge)" = refs/heads/main
+	! test_cmp_config local branch.my2.remote &&
+	! test_cmp_config refs/heads/main branch.my2.merge
 '
 
 test_expect_success 'no tracking without .fetch entries' '
-	git config branch.autosetupmerge true &&
+	test_config branch.autosetupmerge true &&
 	git branch my6 s &&
-	git config branch.autosetupmerge false &&
-	test -z "$(git config branch.my6.remote)" &&
-	test -z "$(git config branch.my6.merge)"
+	test_cmp_config "" --default "" branch.my6.remote &&
+	test_cmp_config "" --default "" branch.my6.merge
 '
 
 test_expect_success 'test tracking setup via --track but deeper' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o/o || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track my7 local/o/o &&
-	test "$(git config branch.my7.remote)" = local &&
-	test "$(git config branch.my7.merge)" = refs/heads/o/o
+	test_cmp_config local branch.my7.remote &&
+	test_cmp_config refs/heads/o/o branch.my7.merge
 '
 
 test_expect_success 'test deleting branch deletes branch config' '
 	git branch -d my7 &&
-	test -z "$(git config branch.my7.remote)" &&
-	test -z "$(git config branch.my7.merge)"
+	test_cmp_config "" --default "" branch.my7.remote &&
+	test_cmp_config "" --default "" branch.my7.merge
 '
 
 test_expect_success 'test deleting branch without config' '

@@ -850,14 +838,15 @@ test_expect_success 'deleting currently checked out branch fails' '
 
 test_expect_success 'test --track without .fetch entries' '
 	git branch --track my8 &&
-	test "$(git config branch.my8.remote)" &&
-	test "$(git config branch.my8.merge)"
+	git config branch.my8.remote >out &&
+	test -s out &&
+	git config branch.my8.merge >out &&
+	test -s out
 '
 
 test_expect_success 'branch from non-branch HEAD w/autosetupmerge=always' '
-	git config branch.autosetupmerge always &&
-	git branch my9 HEAD^ &&
-	git config branch.autosetupmerge false
+	test_config branch.autosetupmerge always &&
+	git branch my9 HEAD^
 '
 
 test_expect_success 'branch from non-branch HEAD w/--track causes failure' '

@@ -913,16 +902,16 @@ test_expect_success 'use --set-upstream-to modify HEAD' '
 	test_config branch.main.merge foo &&
 	git branch my12 &&
 	git branch --set-upstream-to my12 &&
-	test "$(git config branch.main.remote)" = "." &&
-	test "$(git config branch.main.merge)" = "refs/heads/my12"
+	test_cmp_config "." branch.main.remote &&
+	test_cmp_config "refs/heads/my12" branch.main.merge
 '
 
 test_expect_success 'use --set-upstream-to modify a particular branch' '
 	git branch my13 &&
 	git branch --set-upstream-to main my13 &&
 	test_when_finished "git branch --unset-upstream my13" &&
-	test "$(git config branch.my13.remote)" = "." &&
-	test "$(git config branch.my13.merge)" = "refs/heads/main"
+	test_cmp_config "." branch.my13.remote &&
+	test_cmp_config "refs/heads/main" branch.my13.merge
 '
 
 test_expect_success '--unset-upstream should fail if given a non-existent branch' '

@@ -1003,273 +992,251 @@ test_expect_success 'git checkout -b g/h/i -l should create a branch and a log'
 
 test_expect_success 'checkout -b makes reflog by default' '
 	git checkout main &&
-	git config --unset core.logAllRefUpdates &&
+	test_unconfig core.logAllRefUpdates &&
 	git checkout -b alpha &&
 	git rev-parse --verify alpha@{0}
 '
 
 test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
-	git config core.logAllRefUpdates false &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -b beta &&
 	test_must_fail git rev-parse --verify beta@{0}
 '
 
 test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -lb gamma &&
-	git config --unset core.logAllRefUpdates &&
 	git rev-parse --verify gamma@{0}
 '
 
 test_expect_success 'avoid ambiguous track' '
-	git config branch.autosetupmerge true &&
-	git config remote.ambi1.url lalala &&
-	git config remote.ambi1.fetch refs/heads/lalala:refs/heads/main &&
-	git config remote.ambi2.url lilili &&
-	git config remote.ambi2.fetch refs/heads/lilili:refs/heads/main &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote ambi1 lalala refs/heads/lalala:refs/heads/main &&
+	test_set_remote ambi2 lilili refs/heads/lilili:refs/heads/main &&
 	test_must_fail git branch all1 main &&
-	test -z "$(git config branch.all1.merge)"
+	test_cmp_config "" --default "" branch.all1.merge
 '
 
 test_expect_success 'autosetuprebase local on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	git fetch local &&
 	git branch mybase &&
 	git branch --track myr1 mybase &&
-	test "$(git config branch.myr1.remote)" = . &&
-	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr1.rebase)" = true
+	test_cmp_config . branch.myr1.remote &&
+	test_cmp_config refs/heads/mybase branch.myr1.merge &&
+	test_cmp_config true branch.myr1.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase always &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase always &&
+	git fetch local &&
 	git branch mybase2 &&
 	git branch --track myr2 mybase &&
-	test "$(git config branch.myr2.remote)" = . &&
-	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr2.rebase)" = true
+	test_cmp_config . branch.myr2.remote &&
+	test_cmp_config refs/heads/mybase branch.myr2.merge &&
+	test_cmp_config true branch.myr2.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch mybase3 &&
 	git branch --track myr3 mybase2 &&
-	test "$(git config branch.myr3.remote)" = . &&
-	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr3.rebase)" = true
+	test_cmp_config . branch.myr3.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr3.merge &&
+	! test_cmp_config true branch.myr3.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	git fetch local &&
 	git branch mybase4 &&
 	git branch --track myr4 mybase2 &&
-	test "$(git config branch.myr4.remote)" = . &&
-	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr4.rebase)" = true
+	test_cmp_config . branch.myr4.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr4.merge &&
+	! test_cmp_config true branch.myr4.rebase
 '
 
 test_expect_success 'autosetuprebase local on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	git fetch local &&
 	git branch --track myr5 local/main &&
-	test "$(git config branch.myr5.remote)" = local &&
-	test "$(git config branch.myr5.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr5.rebase)" = true
+	test_cmp_config local branch.myr5.remote &&
+	test_cmp_config refs/heads/main branch.myr5.merge &&
+	! test_cmp_config true branch.myr5.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	git fetch local &&
 	git branch --track myr6 local/main &&
-	test "$(git config branch.myr6.remote)" = local &&
-	test "$(git config branch.myr6.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr6.rebase)" = true
+	test_cmp_config local branch.myr6.remote &&
+	test_cmp_config refs/heads/main branch.myr6.merge &&
+	! test_cmp_config true branch.myr6.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch --track myr7 local/main &&
-	test "$(git config branch.myr7.remote)" = local &&
-	test "$(git config branch.myr7.merge)" = refs/heads/main &&
-	test "$(git config branch.myr7.rebase)" = true
+	test_cmp_config local branch.myr7.remote &&
+	test_cmp_config refs/heads/main branch.myr7.merge &&
+	test_cmp_config true branch.myr7.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch --track myr8 local/main &&
-	test "$(git config branch.myr8.remote)" = local &&
-	test "$(git config branch.myr8.merge)" = refs/heads/main &&
-	test "$(git config branch.myr8.rebase)" = true
+	test_cmp_config local branch.myr8.remote &&
+	test_cmp_config refs/heads/main branch.myr8.merge &&
+	test_cmp_config true branch.myr8.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
-	git config --unset branch.autosetuprebase &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_unconfig branch.autosetuprebase &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track myr9 local/main &&
-	test "$(git config branch.myr9.remote)" = local &&
-	test "$(git config branch.myr9.merge)" = refs/heads/main &&
-	test "z$(git config branch.myr9.rebase)" = z
+	test_cmp_config local branch.myr9.remote &&
+	test_cmp_config refs/heads/main branch.myr9.merge &&
+	test_cmp_config "" --default "" branch.myr9.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch mybase10 &&
 	git branch --track myr10 mybase2 &&
-	test "$(git config branch.myr10.remote)" = . &&
-	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
-	test "z$(git config branch.myr10.rebase)" = z
+	test_cmp_config . branch.myr10.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr10.merge &&
+	test_cmp_config "" --default "" branch.myr10.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr11 mybase2 &&
-	test "z$(git config branch.myr11.remote)" = z &&
-	test "z$(git config branch.myr11.merge)" = z &&
-	test "z$(git config branch.myr11.rebase)" = z
+	test_cmp_config "" --default "" branch.myr11.remote &&
+	test_cmp_config "" --default "" branch.myr11.merge &&
+	test_cmp_config "" --default "" branch.myr11.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr12 local/main &&
-	test "z$(git config branch.myr12.remote)" = z &&
-	test "z$(git config branch.myr12.merge)" = z &&
-	test "z$(git config branch.myr12.rebase)" = z
+	test_cmp_config "" --default "" branch.myr12.remote &&
+	test_cmp_config "" --default "" branch.myr12.merge &&
+	test_cmp_config "" --default "" branch.myr12.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked local branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr13 mybase2 &&
-	test "z$(git config branch.myr13.remote)" = z &&
-	test "z$(git config branch.myr13.merge)" = z &&
-	test "z$(git config branch.myr13.rebase)" = z
+	test_cmp_config "" --default "" branch.myr13.remote &&
+	test_cmp_config "" --default "" branch.myr13.merge &&
+	test_cmp_config "" --default "" branch.myr13.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked local branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr14 mybase2 &&
-	test "z$(git config branch.myr14.remote)" = z &&
-	test "z$(git config branch.myr14.merge)" = z &&
-	test "z$(git config branch.myr14.rebase)" = z
+	test_cmp_config "" --default "" branch.myr14.remote &&
+	test_cmp_config "" --default "" branch.myr14.merge &&
+	test_cmp_config "" --default "" branch.myr14.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked local branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr15 mybase2 &&
-	test "z$(git config branch.myr15.remote)" = z &&
-	test "z$(git config branch.myr15.merge)" = z &&
-	test "z$(git config branch.myr15.rebase)" = z
+	test_cmp_config "" --default "" branch.myr15.remote &&
+	test_cmp_config "" --default "" branch.myr15.merge &&
+	test_cmp_config "" --default "" branch.myr15.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked local branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr16 mybase2 &&
-	test "z$(git config branch.myr16.remote)" = z &&
-	test "z$(git config branch.myr16.merge)" = z &&
-	test "z$(git config branch.myr16.rebase)" = z
+	test_cmp_config "" --default "" branch.myr16.remote &&
+	test_cmp_config "" --default "" branch.myr16.merge &&
+	test_cmp_config "" --default "" branch.myr16.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked remote branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr17 local/main &&
-	test "z$(git config branch.myr17.remote)" = z &&
-	test "z$(git config branch.myr17.merge)" = z &&
-	test "z$(git config branch.myr17.rebase)" = z
+	test_cmp_config "" --default "" branch.myr17.remote &&
+	test_cmp_config "" --default "" branch.myr17.merge &&
+	test_cmp_config "" --default "" branch.myr17.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked remote branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr18 local/main &&
-	test "z$(git config branch.myr18.remote)" = z &&
-	test "z$(git config branch.myr18.merge)" = z &&
-	test "z$(git config branch.myr18.rebase)" = z
+	test_cmp_config "" --default "" branch.myr18.remote &&
+	test_cmp_config "" --default "" branch.myr18.merge &&
+	test_cmp_config "" --default "" branch.myr18.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked remote branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr19 local/main &&
-	test "z$(git config branch.myr19.remote)" = z &&
-	test "z$(git config branch.myr19.merge)" = z &&
-	test "z$(git config branch.myr19.rebase)" = z
+	test_cmp_config "" --default "" branch.myr19.remote &&
+	test_cmp_config "" --default "" branch.myr19.merge &&
+	test_cmp_config "" --default "" branch.myr19.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked remote branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr20 local/main &&
-	test "z$(git config branch.myr20.remote)" = z &&
-	test "z$(git config branch.myr20.merge)" = z &&
-	test "z$(git config branch.myr20.rebase)" = z
+	test_cmp_config "" --default "" branch.myr20.remote &&
+	test_cmp_config "" --default "" branch.myr20.merge &&
+	test_cmp_config "" --default "" branch.myr20.rebase
 '
 
 test_expect_success 'autosetuprebase always on detached HEAD' '
-	git config branch.autosetupmerge always &&
+	test_config branch.autosetupmerge always &&
 	test_when_finished git checkout main &&
 	git checkout HEAD^0 &&
 	git branch my11 &&
-	test -z "$(git config branch.my11.remote)" &&
-	test -z "$(git config branch.my11.merge)"
+	test_cmp_config "" --default "" branch.my11.remote &&
+	test_cmp_config "" --default "" branch.my11.rebase
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (bad value)' '
-	git config branch.autosetuprebase garbage &&
+	test_config branch.autosetuprebase garbage &&
 	test_must_fail git branch
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (no value)' '
-	git config --unset branch.autosetuprebase &&
+	test_unconfig branch.autosetuprebase &&
 	echo "[branch] autosetuprebase" >>.git/config &&
 	test_must_fail git branch &&
 	git config --unset branch.autosetuprebase

@@ -1277,7 +1244,7 @@ test_expect_success 'detect misconfigured autosetuprebase (no value)' '
 
 test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '
 	git checkout my9 &&
-	git config --unset branch.my8.merge &&
+	test_unconfig branch.my8.merge &&
 	test_must_fail git branch -d my8
 '

@@ -1285,7 +1252,7 @@ test_expect_success 'attempt to delete a branch merged to its base' '
 	# we are on my9 which is the initial commit; traditionally
 	# we would not have allowed deleting my8 that is not merged
 	# to my9, but it is set to track main that already has my8
-	git config branch.my8.merge refs/heads/main &&
+	test_config branch.my8.merge refs/heads/main &&
 	git branch -d my8
 '

@@ -1397,8 +1364,8 @@ test_expect_success 'tracking with unexpected .fetch refspec' '
 		git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
 		git fetch c &&
 		git branch --track local/a/main remotes/a/main &&
-		test "$(git config branch.local/a/main.remote)" = "c" &&
-		test "$(git config branch.local/a/main.merge)" = "refs/remotes/a/main" &&
+		test_cmp_config "c" branch.local/a/main.remote &&
+		test_cmp_config "refs/remotes/a/main" branch.local/a/main.merge &&
 		git rev-parse --verify a >expect &&
 		git rev-parse --verify local/a/main >actual &&
 		test_cmp expect actual

base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a

Signature · 1 line

gitgitgadget

Tao Klerks via GitGitGadget

author

2022-04-30 18:43 UTC #4 lore raw

PATCH v3 1/1 t3200: fix antipatterns in existing branch tests

From: Tao Klerks <[email protected]>

Fix issues in t3200 branch tests that, if copied, might catch new contributors out:

Use test_config to show that config state is not being intentionally left to spill over into other tests.

Use test_cmp_config instead of git config in subshells, so that git's error code is not lost if/when an unexpected error occurs.

Use output redirection and later content checking instead of subshells, so that git's error code is not lost if/when an unexpected error occurs.

Eliminate local-fetch-avoiding optimization as it is error-prone (it is easy to check the wrong thing), hides segfaults, and yields only a marginal performance improvement given the fetch is local.

Introduce local helper test_set_remote to simplify the common local pattern of setting up a remote via config.

Signed-off-by Tao Klerks <[email protected]>

---
t3200: fix antipatterns in existing branch tests

This is a cleanup of the branch tests following a case where I was adding some, and did substantially the wrong thing by following existing examples.

Changes in V3:

* rebased onto recent master (with some conflict resolution)

Resending for rebase and also because there have been no comments in a long time.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1182%2FTaoK%2Fcleanup-t3200-tests-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1182/TaoK/cleanup-t3200-tests-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1182

Range-diff vs v2:

1: e4addb910f7 ! 1: d49a230952e t3200: fix antipatterns in existing branch tests
@@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
git rev-parse --verify gamma@{0}
'

- test_expect_success 'avoid ambiguous track' '
+ test_expect_success 'avoid ambiguous track and advise' '
- git config branch.autosetupmerge true &&
- git config remote.ambi1.url lalala &&
- git config remote.ambi1.fetch refs/heads/lalala:refs/heads/main &&
@@ t/t3200-branch.sh: test_expect_success 'git checkout -b g/h/i -l should create a
+ test_config branch.autosetupmerge true &&
+ test_set_remote ambi1 lalala refs/heads/lalala:refs/heads/main &&
+ test_set_remote ambi2 lilili refs/heads/lilili:refs/heads/main &&
- test_must_fail git branch all1 main &&
+ cat <<-EOF >expected &&
+ fatal: not tracking: ambiguous information for ref '\''refs/heads/main'\''
+ hint: There are multiple remotes whose fetch refspecs map to the remote
+@@ t/t3200-branch.sh: test_expect_success 'avoid ambiguous track and advise' '
+ EOF
+ test_must_fail git branch all1 main 2>actual &&
+ test_cmp expected actual &&
- test -z "$(git config branch.all1.merge)"
+ test_cmp_config "" --default "" branch.all1.merge
'

 t/t3200-branch.sh | 439 +++++++++++++++++++++-------------------------
 1 file changed, 203 insertions(+), 236 deletions(-)

t/t3200-branch.sh +203 ~~-236~~

index e12db593615..88cbc866ab6 100755
@@ -11,6 +11,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+test_set_remote () {
+	test_config "remote.$1.url" "$2" &&
+	test_config "remote.$1.fetch" "${3:-"refs/heads/*:refs/remotes/$1/*"}"
+}
+
 test_expect_success 'prepare a trivial repository' '
 	echo Hello >A &&
 	git update-index --add A &&

@@ -389,11 +394,9 @@ EOF
 '
 
 test_expect_success 'git branch with column.*' '
-	git config column.ui column &&
-	git config column.branch "dense" &&
+	test_config column.ui column &&
+	test_config column.branch "dense" &&
 	COLUMNS=80 git branch >actual &&
-	git config --unset column.branch &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c   bam   foo   l   * main   n     o/p   r
   abc     bar   j/k   m/m   mb     o/o   q     topic

@@ -406,9 +409,8 @@ test_expect_success 'git branch --column -v should fail' '
 '
 
 test_expect_success 'git branch -v with column.ui ignored' '
-	git config column.ui column &&
+	test_config column.ui column &&
 	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
-	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
   abc

@@ -452,7 +454,7 @@ test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
 '
 
 test_expect_success 'config information was renamed, too' '
-	test $(git config branch.s.dummy) = Hello &&
+	test_cmp_config Hello branch.s.dummy &&
 	test_must_fail git config branch.s/s.dummy
 '

@@ -510,63 +512,57 @@ test_expect_success 'git branch --copy dumps usage' '
 test_expect_success 'git branch -c d e should work' '
 	git branch --create-reflog d &&
 	git reflog exists refs/heads/d &&
-	git config branch.d.dummy Hello &&
+	test_config branch.d.dummy Hello &&
 	git branch -c d e &&
 	git reflog exists refs/heads/d &&
 	git reflog exists refs/heads/e &&
-	echo Hello >expect &&
-	git config branch.e.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.d.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.e.dummy &&
+	test_cmp_config Hello branch.d.dummy
 '
 
 test_expect_success 'git branch --copy is a synonym for -c' '
 	git branch --create-reflog copy &&
 	git reflog exists refs/heads/copy &&
-	git config branch.copy.dummy Hello &&
+	test_config branch.copy.dummy Hello &&
 	git branch --copy copy copy-to &&
 	git reflog exists refs/heads/copy &&
 	git reflog exists refs/heads/copy-to &&
-	echo Hello >expect &&
-	git config branch.copy.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.copy-to.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.copy.dummy &&
+	test_cmp_config Hello branch.copy-to.dummy
 '
 
 test_expect_success 'git branch -c ee ef should copy ee to create branch ef' '
 	git checkout -b ee &&
 	git reflog exists refs/heads/ee &&
-	git config branch.ee.dummy Hello &&
+	test_config branch.ee.dummy Hello &&
 	git branch -c ee ef &&
 	git reflog exists refs/heads/ee &&
 	git reflog exists refs/heads/ef &&
-	test $(git config branch.ee.dummy) = Hello &&
-	test $(git config branch.ef.dummy) = Hello &&
-	test $(git rev-parse --abbrev-ref HEAD) = ee
+	test_cmp_config Hello branch.ee.dummy &&
+	test_cmp_config Hello branch.ef.dummy &&
+	echo ee >expect &&
+	git rev-parse --abbrev-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch -c f/f g/g should work' '
 	git branch --create-reflog f/f &&
 	git reflog exists refs/heads/f/f &&
-	git config branch.f/f.dummy Hello &&
+	test_config branch.f/f.dummy Hello &&
 	git branch -c f/f g/g &&
 	git reflog exists refs/heads/f/f &&
 	git reflog exists refs/heads/g/g &&
-	test $(git config branch.f/f.dummy) = Hello &&
-	test $(git config branch.g/g.dummy) = Hello
+	test_cmp_config Hello branch.f/f.dummy &&
+	test_cmp_config Hello branch.g/g.dummy
 '
 
 test_expect_success 'git branch -c m2 m2 should work' '
 	git branch --create-reflog m2 &&
 	git reflog exists refs/heads/m2 &&
-	git config branch.m2.dummy Hello &&
+	test_config branch.m2.dummy Hello &&
 	git branch -c m2 m2 &&
 	git reflog exists refs/heads/m2 &&
-	test $(git config branch.m2.dummy) = Hello
+	test_cmp_config Hello branch.m2.dummy
 '
 
 test_expect_success 'git branch -c zz zz/zz should fail' '

@@ -636,15 +632,15 @@ test_expect_success 'git branch -C main5 main5 should work when main is checked
 test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
 	git branch --create-reflog cd &&
 	git reflog exists refs/heads/cd &&
-	git config branch.cd.dummy CD &&
+	test_config branch.cd.dummy CD &&
 	git branch --create-reflog ab &&
 	git reflog exists refs/heads/ab &&
-	git config branch.ab.dummy AB &&
+	test_config branch.ab.dummy AB &&
 	git branch -C ab cd &&
 	git reflog exists refs/heads/ab &&
 	git reflog exists refs/heads/cd &&
-	test $(git config branch.ab.dummy) = AB &&
-	test $(git config branch.cd.dummy) = AB
+	test_cmp_config AB branch.ab.dummy &&
+	test_cmp_config AB branch.cd.dummy
 '
 
 test_expect_success 'git branch -c correctly copies multiple config sections' '

@@ -778,75 +774,67 @@ test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
 '
 
 test_expect_success 'test tracking setup via --track' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track my1 local/main &&
-	test $(git config branch.my1.remote) = local &&
-	test $(git config branch.my1.merge) = refs/heads/main
+	test_cmp_config local branch.my1.remote &&
+	test_cmp_config refs/heads/main branch.my1.merge
 '
 
 test_expect_success 'test tracking setup (non-wildcard, matching)' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/main:refs/remotes/local/main &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . refs/heads/main:refs/remotes/local/main &&
+	git fetch local &&
 	git branch --track my4 local/main &&
-	test $(git config branch.my4.remote) = local &&
-	test $(git config branch.my4.merge) = refs/heads/main
+	test_cmp_config local branch.my4.remote &&
+	test_cmp_config refs/heads/main branch.my4.merge
 '
 
 test_expect_success 'tracking setup fails on non-matching refspec' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
-	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
+	test_set_remote local . &&
+	git fetch local &&
+	test_config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	test_must_fail git branch --track my5 local/main &&
 	test_must_fail git config branch.my5.remote &&
 	test_must_fail git config branch.my5.merge
 '
 
 test_expect_success 'test tracking setup via config' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch my3 local/main &&
-	test $(git config branch.my3.remote) = local &&
-	test $(git config branch.my3.merge) = refs/heads/main
+	test_cmp_config local branch.my3.remote &&
+	test_cmp_config refs/heads/main branch.my3.merge
 '
 
 test_expect_success 'test overriding tracking setup via --no-track' '
-	git config branch.autosetupmerge true &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track my2 local/main &&
-	git config branch.autosetupmerge false &&
-	! test "$(git config branch.my2.remote)" = local &&
-	! test "$(git config branch.my2.merge)" = refs/heads/main
+	! test_cmp_config local branch.my2.remote &&
+	! test_cmp_config refs/heads/main branch.my2.merge
 '
 
 test_expect_success 'no tracking without .fetch entries' '
-	git config branch.autosetupmerge true &&
+	test_config branch.autosetupmerge true &&
 	git branch my6 s &&
-	git config branch.autosetupmerge false &&
-	test -z "$(git config branch.my6.remote)" &&
-	test -z "$(git config branch.my6.merge)"
+	test_cmp_config "" --default "" branch.my6.remote &&
+	test_cmp_config "" --default "" branch.my6.merge
 '
 
 test_expect_success 'test tracking setup via --track but deeper' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o/o || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track my7 local/o/o &&
-	test "$(git config branch.my7.remote)" = local &&
-	test "$(git config branch.my7.merge)" = refs/heads/o/o
+	test_cmp_config local branch.my7.remote &&
+	test_cmp_config refs/heads/o/o branch.my7.merge
 '
 
 test_expect_success 'test deleting branch deletes branch config' '
 	git branch -d my7 &&
-	test -z "$(git config branch.my7.remote)" &&
-	test -z "$(git config branch.my7.merge)"
+	test_cmp_config "" --default "" branch.my7.remote &&
+	test_cmp_config "" --default "" branch.my7.merge
 '
 
 test_expect_success 'test deleting branch without config' '

@@ -867,14 +855,15 @@ test_expect_success 'deleting currently checked out branch fails' '
 
 test_expect_success 'test --track without .fetch entries' '
 	git branch --track my8 &&
-	test "$(git config branch.my8.remote)" &&
-	test "$(git config branch.my8.merge)"
+	git config branch.my8.remote >out &&
+	test -s out &&
+	git config branch.my8.merge >out &&
+	test -s out
 '
 
 test_expect_success 'branch from non-branch HEAD w/autosetupmerge=always' '
-	git config branch.autosetupmerge always &&
-	git branch my9 HEAD^ &&
-	git config branch.autosetupmerge false
+	test_config branch.autosetupmerge always &&
+	git branch my9 HEAD^
 '
 
 test_expect_success 'branch from non-branch HEAD w/--track causes failure' '

@@ -930,16 +919,16 @@ test_expect_success 'use --set-upstream-to modify HEAD' '
 	test_config branch.main.merge foo &&
 	git branch my12 &&
 	git branch --set-upstream-to my12 &&
-	test "$(git config branch.main.remote)" = "." &&
-	test "$(git config branch.main.merge)" = "refs/heads/my12"
+	test_cmp_config "." branch.main.remote &&
+	test_cmp_config "refs/heads/my12" branch.main.merge
 '
 
 test_expect_success 'use --set-upstream-to modify a particular branch' '
 	git branch my13 &&
 	git branch --set-upstream-to main my13 &&
 	test_when_finished "git branch --unset-upstream my13" &&
-	test "$(git config branch.my13.remote)" = "." &&
-	test "$(git config branch.my13.merge)" = "refs/heads/main"
+	test_cmp_config "." branch.my13.remote &&
+	test_cmp_config "refs/heads/main" branch.my13.merge
 '
 
 test_expect_success '--unset-upstream should fail if given a non-existent branch' '

@@ -1020,31 +1009,29 @@ test_expect_success 'git checkout -b g/h/i -l should create a branch and a log'
 
 test_expect_success 'checkout -b makes reflog by default' '
 	git checkout main &&
-	git config --unset core.logAllRefUpdates &&
+	test_unconfig core.logAllRefUpdates &&
 	git checkout -b alpha &&
 	git rev-parse --verify alpha@{0}
 '
 
 test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
-	git config core.logAllRefUpdates false &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -b beta &&
 	test_must_fail git rev-parse --verify beta@{0}
 '
 
 test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
+	test_config core.logAllRefUpdates false &&
 	git checkout -lb gamma &&
-	git config --unset core.logAllRefUpdates &&
 	git rev-parse --verify gamma@{0}
 '
 
 test_expect_success 'avoid ambiguous track and advise' '
-	git config branch.autosetupmerge true &&
-	git config remote.ambi1.url lalala &&
-	git config remote.ambi1.fetch refs/heads/lalala:refs/heads/main &&
-	git config remote.ambi2.url lilili &&
-	git config remote.ambi2.fetch refs/heads/lilili:refs/heads/main &&
+	test_config branch.autosetupmerge true &&
+	test_set_remote ambi1 lalala refs/heads/lalala:refs/heads/main &&
+	test_set_remote ambi2 lilili refs/heads/lilili:refs/heads/main &&
 	cat <<-EOF >expected &&
 	fatal: not tracking: ambiguous information for ref '\''refs/heads/main'\''
 	hint: There are multiple remotes whose fetch refspecs map to the remote

@@ -1060,247 +1047,227 @@ test_expect_success 'avoid ambiguous track and advise' '
 	EOF
 	test_must_fail git branch all1 main 2>actual &&
 	test_cmp expected actual &&
-	test -z "$(git config branch.all1.merge)"
+	test_cmp_config "" --default "" branch.all1.merge
 '
 
 test_expect_success 'autosetuprebase local on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	git fetch local &&
 	git branch mybase &&
 	git branch --track myr1 mybase &&
-	test "$(git config branch.myr1.remote)" = . &&
-	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr1.rebase)" = true
+	test_cmp_config . branch.myr1.remote &&
+	test_cmp_config refs/heads/mybase branch.myr1.merge &&
+	test_cmp_config true branch.myr1.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase always &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase always &&
+	git fetch local &&
 	git branch mybase2 &&
 	git branch --track myr2 mybase &&
-	test "$(git config branch.myr2.remote)" = . &&
-	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr2.rebase)" = true
+	test_cmp_config . branch.myr2.remote &&
+	test_cmp_config refs/heads/mybase branch.myr2.merge &&
+	test_cmp_config true branch.myr2.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch mybase3 &&
 	git branch --track myr3 mybase2 &&
-	test "$(git config branch.myr3.remote)" = . &&
-	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr3.rebase)" = true
+	test_cmp_config . branch.myr3.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr3.merge &&
+	! test_cmp_config true branch.myr3.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	git fetch local &&
 	git branch mybase4 &&
 	git branch --track myr4 mybase2 &&
-	test "$(git config branch.myr4.remote)" = . &&
-	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr4.rebase)" = true
+	test_cmp_config . branch.myr4.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr4.merge &&
+	! test_cmp_config true branch.myr4.rebase
 '
 
 test_expect_success 'autosetuprebase local on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase local &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase local &&
+	git fetch local &&
 	git branch --track myr5 local/main &&
-	test "$(git config branch.myr5.remote)" = local &&
-	test "$(git config branch.myr5.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr5.rebase)" = true
+	test_cmp_config local branch.myr5.remote &&
+	test_cmp_config refs/heads/main branch.myr5.merge &&
+	! test_cmp_config true branch.myr5.rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase never &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase never &&
+	git fetch local &&
 	git branch --track myr6 local/main &&
-	test "$(git config branch.myr6.remote)" = local &&
-	test "$(git config branch.myr6.merge)" = refs/heads/main &&
-	! test "$(git config branch.myr6.rebase)" = true
+	test_cmp_config local branch.myr6.remote &&
+	test_cmp_config refs/heads/main branch.myr6.merge &&
+	! test_cmp_config true branch.myr6.rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch --track myr7 local/main &&
-	test "$(git config branch.myr7.remote)" = local &&
-	test "$(git config branch.myr7.merge)" = refs/heads/main &&
-	test "$(git config branch.myr7.rebase)" = true
+	test_cmp_config local branch.myr7.remote &&
+	test_cmp_config refs/heads/main branch.myr7.merge &&
+	test_cmp_config true branch.myr7.rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	git config branch.autosetuprebase remote &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	test_config branch.autosetuprebase remote &&
+	git fetch local &&
 	git branch --track myr8 local/main &&
-	test "$(git config branch.myr8.remote)" = local &&
-	test "$(git config branch.myr8.merge)" = refs/heads/main &&
-	test "$(git config branch.myr8.rebase)" = true
+	test_cmp_config local branch.myr8.remote &&
+	test_cmp_config refs/heads/main branch.myr8.merge &&
+	test_cmp_config true branch.myr8.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
-	git config --unset branch.autosetuprebase &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_unconfig branch.autosetuprebase &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --track myr9 local/main &&
-	test "$(git config branch.myr9.remote)" = local &&
-	test "$(git config branch.myr9.merge)" = refs/heads/main &&
-	test "z$(git config branch.myr9.rebase)" = z
+	test_cmp_config local branch.myr9.remote &&
+	test_cmp_config refs/heads/main branch.myr9.merge &&
+	test_cmp_config "" --default "" branch.myr9.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/o || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch mybase10 &&
 	git branch --track myr10 mybase2 &&
-	test "$(git config branch.myr10.remote)" = . &&
-	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
-	test "z$(git config branch.myr10.rebase)" = z
+	test_cmp_config . branch.myr10.remote &&
+	test_cmp_config refs/heads/mybase2 branch.myr10.merge &&
+	test_cmp_config "" --default "" branch.myr10.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr11 mybase2 &&
-	test "z$(git config branch.myr11.remote)" = z &&
-	test "z$(git config branch.myr11.merge)" = z &&
-	test "z$(git config branch.myr11.rebase)" = z
+	test_cmp_config "" --default "" branch.myr11.remote &&
+	test_cmp_config "" --default "" branch.myr11.merge &&
+	test_cmp_config "" --default "" branch.myr11.rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr12 local/main &&
-	test "z$(git config branch.myr12.remote)" = z &&
-	test "z$(git config branch.myr12.merge)" = z &&
-	test "z$(git config branch.myr12.rebase)" = z
+	test_cmp_config "" --default "" branch.myr12.remote &&
+	test_cmp_config "" --default "" branch.myr12.merge &&
+	test_cmp_config "" --default "" branch.myr12.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked local branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr13 mybase2 &&
-	test "z$(git config branch.myr13.remote)" = z &&
-	test "z$(git config branch.myr13.merge)" = z &&
-	test "z$(git config branch.myr13.rebase)" = z
+	test_cmp_config "" --default "" branch.myr13.remote &&
+	test_cmp_config "" --default "" branch.myr13.merge &&
+	test_cmp_config "" --default "" branch.myr13.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked local branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr14 mybase2 &&
-	test "z$(git config branch.myr14.remote)" = z &&
-	test "z$(git config branch.myr14.merge)" = z &&
-	test "z$(git config branch.myr14.rebase)" = z
+	test_cmp_config "" --default "" branch.myr14.remote &&
+	test_cmp_config "" --default "" branch.myr14.merge &&
+	test_cmp_config "" --default "" branch.myr14.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked local branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr15 mybase2 &&
-	test "z$(git config branch.myr15.remote)" = z &&
-	test "z$(git config branch.myr15.merge)" = z &&
-	test "z$(git config branch.myr15.rebase)" = z
+	test_cmp_config "" --default "" branch.myr15.remote &&
+	test_cmp_config "" --default "" branch.myr15.merge &&
+	test_cmp_config "" --default "" branch.myr15.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked local branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr16 mybase2 &&
-	test "z$(git config branch.myr16.remote)" = z &&
-	test "z$(git config branch.myr16.merge)" = z &&
-	test "z$(git config branch.myr16.rebase)" = z
+	test_cmp_config "" --default "" branch.myr16.remote &&
+	test_cmp_config "" --default "" branch.myr16.merge &&
+	test_cmp_config "" --default "" branch.myr16.rebase
 '
 
 test_expect_success 'autosetuprebase never on an untracked remote branch' '
-	git config branch.autosetuprebase never &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase never &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr17 local/main &&
-	test "z$(git config branch.myr17.remote)" = z &&
-	test "z$(git config branch.myr17.merge)" = z &&
-	test "z$(git config branch.myr17.rebase)" = z
+	test_cmp_config "" --default "" branch.myr17.remote &&
+	test_cmp_config "" --default "" branch.myr17.merge &&
+	test_cmp_config "" --default "" branch.myr17.rebase
 '
 
 test_expect_success 'autosetuprebase local on an untracked remote branch' '
-	git config branch.autosetuprebase local &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase local &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr18 local/main &&
-	test "z$(git config branch.myr18.remote)" = z &&
-	test "z$(git config branch.myr18.merge)" = z &&
-	test "z$(git config branch.myr18.rebase)" = z
+	test_cmp_config "" --default "" branch.myr18.remote &&
+	test_cmp_config "" --default "" branch.myr18.merge &&
+	test_cmp_config "" --default "" branch.myr18.rebase
 '
 
 test_expect_success 'autosetuprebase remote on an untracked remote branch' '
-	git config branch.autosetuprebase remote &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase remote &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr19 local/main &&
-	test "z$(git config branch.myr19.remote)" = z &&
-	test "z$(git config branch.myr19.merge)" = z &&
-	test "z$(git config branch.myr19.rebase)" = z
+	test_cmp_config "" --default "" branch.myr19.remote &&
+	test_cmp_config "" --default "" branch.myr19.merge &&
+	test_cmp_config "" --default "" branch.myr19.rebase
 '
 
 test_expect_success 'autosetuprebase always on an untracked remote branch' '
-	git config branch.autosetuprebase always &&
-	git config remote.local.url . &&
-	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
-	(git show-ref -q refs/remotes/local/main || git fetch local) &&
+	test_config branch.autosetuprebase always &&
+	test_set_remote local . &&
+	git fetch local &&
 	git branch --no-track myr20 local/main &&
-	test "z$(git config branch.myr20.remote)" = z &&
-	test "z$(git config branch.myr20.merge)" = z &&
-	test "z$(git config branch.myr20.rebase)" = z
+	test_cmp_config "" --default "" branch.myr20.remote &&
+	test_cmp_config "" --default "" branch.myr20.merge &&
+	test_cmp_config "" --default "" branch.myr20.rebase
 '
 
 test_expect_success 'autosetuprebase always on detached HEAD' '
-	git config branch.autosetupmerge always &&
+	test_config branch.autosetupmerge always &&
 	test_when_finished git checkout main &&
 	git checkout HEAD^0 &&
 	git branch my11 &&
-	test -z "$(git config branch.my11.remote)" &&
-	test -z "$(git config branch.my11.merge)"
+	test_cmp_config "" --default "" branch.my11.remote &&
+	test_cmp_config "" --default "" branch.my11.rebase
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (bad value)' '
-	git config branch.autosetuprebase garbage &&
+	test_config branch.autosetuprebase garbage &&
 	test_must_fail git branch
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (no value)' '
-	git config --unset branch.autosetuprebase &&
+	test_unconfig branch.autosetuprebase &&
 	echo "[branch] autosetuprebase" >>.git/config &&
 	test_must_fail git branch &&
 	git config --unset branch.autosetuprebase

@@ -1308,7 +1275,7 @@ test_expect_success 'detect misconfigured autosetuprebase (no value)' '
 
 test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '
 	git checkout my9 &&
-	git config --unset branch.my8.merge &&
+	test_unconfig branch.my8.merge &&
 	test_must_fail git branch -d my8
 '

@@ -1316,7 +1283,7 @@ test_expect_success 'attempt to delete a branch merged to its base' '
 	# we are on my9 which is the initial commit; traditionally
 	# we would not have allowed deleting my8 that is not merged
 	# to my9, but it is set to track main that already has my8
-	git config branch.my8.merge refs/heads/main &&
+	test_config branch.my8.merge refs/heads/main &&
 	git branch -d my8
 '

@@ -1428,8 +1395,8 @@ test_expect_success 'tracking with unexpected .fetch refspec' '
 		git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
 		git fetch c &&
 		git branch --track local/a/main remotes/a/main &&
-		test "$(git config branch.local/a/main.remote)" = "c" &&
-		test "$(git config branch.local/a/main.merge)" = "refs/remotes/a/main" &&
+		test_cmp_config "c" branch.local/a/main.remote &&
+		test_cmp_config "refs/remotes/a/main" branch.local/a/main.merge &&
 		git rev-parse --verify a >expect &&
 		git rev-parse --verify local/a/main >actual &&
 		test_cmp expect actual

base-commit: 0f828332d5ac36fc63b7d8202652efa152809856

Signature · 1 line

gitgitgadget

Junio C Hamano

2022-05-04 18:07 UTC #5 lore raw

Quoted 7 lines · Tao Klerks via GitGitGadget

t/t3200-branch.sh +5 -0

+test_set_remote () {
+	test_config "remote.$1.url" "$2" &&
+	test_config "remote.$1.fetch" "${3:-"refs/heads/*:refs/remotes/$1/*"}"
+}
+

t/t3200-branch.sh EOF · line 396

Quoted 10 lines

t/t3200-branch.sh +2 -4

@@ -389,11 +394,9 @@ EOF
 '
 
 test_expect_success 'git branch with column.*' '
-	git config column.ui column &&
-	git config column.branch "dense" &&
+	test_config column.ui column &&
+	test_config column.branch "dense" &&
 	COLUMNS=80 git branch >actual &&
-	git config --unset column.branch &&
-	git config --unset column.ui &&

OK these are easy to verify, as we can clearly see that the intention is for these two settings to affect no later tests.

t/t3200-branch.sh test_expect_success 'git branch --column -v should fail' ' · line 411

Quoted 8 lines

t/t3200-branch.sh +1 -2

@@ -406,9 +409,8 @@ test_expect_success 'git branch --column -v should fail' '
 '
 
 test_expect_success 'git branch -v with column.ui ignored' '
-	git config column.ui column &&
+	test_config column.ui column &&
 	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
-	git config --unset column.ui &&

Likewise.

t/t3200-branch.sh test_expect_success 'git branch -m s/s s should work when s/t is deleted' ' · line 459

Quoted 8 lines

t/t3200-branch.sh +1 -1

@@ -452,7 +454,7 @@ test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
 '
 
 test_expect_success 'config information was renamed, too' '
-	test $(git config branch.s.dummy) = Hello &&
+	test_cmp_config Hello branch.s.dummy &&
 	test_must_fail git config branch.s/s.dummy
 '

OK.

Quoted 8 lines

t/t3200-branch.sh +1 -1

@@ -510,63 +512,57 @@ test_expect_success 'git branch --copy dumps usage' '
 test_expect_success 'git branch -c d e should work' '
 	git branch --create-reflog d &&
 	git reflog exists refs/heads/d &&
-	git config branch.d.dummy Hello &&
+	test_config branch.d.dummy Hello &&
 	git branch -c d e &&
 	git reflog exists refs/heads/d &&

t/t3200-branch.sh test_expect_success 'git branch --copy dumps usage' ' · line 521

Quoted 10 lines

t/t3200-branch.sh +2 -6

 	git reflog exists refs/heads/e &&
-	echo Hello >expect &&
-	git config branch.e.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.d.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.e.dummy &&
+	test_cmp_config Hello branch.d.dummy
 '

This test used to leave both branch.d.dummy and branch.e.dummy behind for later tests. Now with this patch, we clean branch.d.dummy because we use test_config, but branch.e.dummy that was copied by successful "git branch -c" will still be left.

- It is unforunate that it is impossible to verify that the change in behaviour for branch.d.dummy is correct. Without checking all the remainder of the test (and no, grepping for branch.d.dummy is not "checking all the remainder"---a later "branch -c d x" would have created brnach.x.dummy in the original, but with this patch, it would not), which is time consuming, that is.

I trust you made sure that branch.d.dummy is never used after this test is done---it would have been good to explain it either in the proposed log message or after three-dash that you did check and how to save reviewer bandwidth.

- Are you deliberatly leaving branch.e.dummy uncleaned, or is it a mere oversight?

Quoted 7 lines

t/t3200-branch.sh +1 -1

 test_expect_success 'git branch --copy is a synonym for -c' '
 	git branch --create-reflog copy &&
 	git reflog exists refs/heads/copy &&
-	git config branch.copy.dummy Hello &&
+	test_config branch.copy.dummy Hello &&
 	git branch --copy copy copy-to &&
 	git reflog exists refs/heads/copy &&

t/t3200-branch.sh test_expect_success 'git branch --copy dumps usage' ' · line 532

Quoted 10 lines

t/t3200-branch.sh +2 -6

 	git reflog exists refs/heads/copy-to &&
-	echo Hello >expect &&
-	git config branch.copy.dummy >actual &&
-	test_cmp expect actual &&
-	echo Hello >expect &&
-	git config branch.copy-to.dummy >actual &&
-	test_cmp expect actual
+	test_cmp_config Hello branch.copy.dummy &&
+	test_cmp_config Hello branch.copy-to.dummy
 '

The same comment for branch.copy.dummy and branch.copy-to.dummy applies.

I'll stop here for now. Thanks for starting this clean-up.

Tao Klerks

2022-05-12 05:12 UTC #6 lore raw

Quoted 35 lines · Junio C Hamano

Quoted 8 lines · Tao Klerks via GitGitGadget
t/t3200-branch.sh +1 -1
@@ -510,63 +512,57 @@ test_expect_success 'git branch --copy dumps usage' '
 test_expect_success 'git branch -c d e should work' '
      git branch --create-reflog d &&
      git reflog exists refs/heads/d &&
-     git config branch.d.dummy Hello &&
+     test_config branch.d.dummy Hello &&
      git branch -c d e &&
      git reflog exists refs/heads/d &&
t/t3200-branch.sh test_expect_success 'git branch --copy dumps usage' ' · line 521
Quoted 10 lines
t/t3200-branch.sh +2 -6
      git reflog exists refs/heads/e &&
-     echo Hello >expect &&
-     git config branch.e.dummy >actual &&
-     test_cmp expect actual &&
-     echo Hello >expect &&
-     git config branch.d.dummy >actual &&
-     test_cmp expect actual
+     test_cmp_config Hello branch.e.dummy &&
+     test_cmp_config Hello branch.d.dummy
 '
This test used to leave both branch.d.dummy and branch.e.dummy behind for later tests. Now with this patch, we clean branch.d.dummy because we use test_config, but branch.e.dummy that was copied by successful "git branch -c" will still be left.
- It is unforunate that it is impossible to verify that the change in behaviour for branch.d.dummy is correct. Without checking all the remainder of the test (and no, grepping for branch.d.dummy is not "checking all the remainder"---a later "branch -c d x" would have created brnach.x.dummy in the original, but with this patch, it would not), which is time consuming, that is.

Quoted 4 lines

I trust you made sure that branch.d.dummy is never used after this test is done---it would have been good to explain it either in the proposed log message or after three-dash that you did check and how to save reviewer bandwidth.

I will add a comment, and will (first) more diligently check for dependencies on things now-removed.

My strategy was, frankly, a little more haphazard: eyeball the intent, look for references in the following couple tests, and otherwise assume that if the tests still pass, it means they didn't unexpectedly depend on this.

Fwiw, the not-trivial-but-still-straightforward approach I'm using is to search case-sensitively for the word "d" in this example. a, b, c and d are all bad because they are common english words or flags used throughout this test, but other letters and combinations are easier to check in this reasonably-simple way.

Quoted 4 lines

- Are you deliberatly leaving branch.e.dummy uncleaned, or is it a mere oversight?

Somewhere in-between. My intent with these changes was to use the right helpers/patterns, but I did not aspire to making sure nothing leaks between tests at all.

Generally speaking, none of the tests I've seen (in this file or others) clean up / delete the *branches* they create - presumably because there's no significant need - the existence of branches rarely or never has side-effects unless explicitly referenced. Config, on the other hand, is a good thing to clean up by default, because it is much more likely to interfere with later tests.

Something like configuring "branch.x.dummy" (or the more meaningful "branch.x.merge") straddles those 2 worlds - I would use test_config to be consistent if setting one manually, but it's still branch-specific stuff so I wouldn't be diligent about removing it if it was created as a side-effect of another command like "git branch -c" - just like I'm not removing the new branch itself.

That said, now that you've called my attention to it, I'll look for leaky branch configs :)

I will also add a note clarifying "intend to use the right patterns/helpers, but not necessarily to eliminate state leaks between tests" in the commit message.

Quoted 18 lines

Quoted 7 lines

t/t3200-branch.sh +1 -1

 test_expect_success 'git branch --copy is a synonym for -c' '
      git branch --create-reflog copy &&
      git reflog exists refs/heads/copy &&
-     git config branch.copy.dummy Hello &&
+     test_config branch.copy.dummy Hello &&
      git branch --copy copy copy-to &&
      git reflog exists refs/heads/copy &&

t/t3200-branch.sh test_expect_success 'git branch --copy dumps usage' ' · line 532

Quoted 10 lines

t/t3200-branch.sh +2 -6

      git reflog exists refs/heads/copy-to &&
-     echo Hello >expect &&
-     git config branch.copy.dummy >actual &&
-     test_cmp expect actual &&
-     echo Hello >expect &&
-     git config branch.copy-to.dummy >actual &&
-     test_cmp expect actual
+     test_cmp_config Hello branch.copy.dummy &&
+     test_cmp_config Hello branch.copy-to.dummy
 '

Quoted 4 lines

The same comment for branch.copy.dummy and branch.copy-to.dummy applies.
I'll stop here for now. Thanks for starting this clean-up.

Thank you!