{"thread":{"id":"34815","subject":"[PATCH 0/4] t: branch: fixes and cleanups","startedAt":"2013-08-31T04:31:47Z","lastAt":"2013-09-08T05:01:11Z","messageCount":14,"participants":["Felipe Contreras","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"226399","messageId":"1377923511-20787-1-git-send-email-felipe.contreras@gmail.com","threadId":"34815","inReplyTo":null,"subject":"[PATCH 0/4] t: branch: fixes and cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-31T04:31:47Z","receivedAt":"2013-08-31T04:31:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras (4):\n  t: branch: trivial style fix\n  t: branch: fix typo\n  t: branch: fix broken && chains\n  t: branch: improve test rollback\n\n t/t3200-branch.sh | 82 +++++++++++++++++++++++++++----------------------------\n 1 file changed, 41 insertions(+), 41 deletions(-)\n\n-- \n1.8.4-fc\n"},{"id":"226400","messageId":"1377923511-20787-2-git-send-email-felipe.contreras@gmail.com","threadId":"34815","inReplyTo":"1377923511-20787-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/4] t: branch: trivial style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-31T04:31:48Z","receivedAt":"2013-08-31T04:31:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t3200-branch.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 44ec6a4..ea548f9 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -14,7 +14,8 @@ test_expect_success 'prepare a trivial repository' '\n \techo World >>A &&\n \tgit update-index --add A &&\n \tgit commit -m \"Second commit.\" &&\n-\tHEAD=$(git rev-parse --verify HEAD)'\n+\tHEAD=$(git rev-parse --verify HEAD)\n+'\n \n test_expect_success 'git branch --help should not have created a bogus branch' '\n \ttest_might_fail git branch --help </dev/null >/dev/null 2>/dev/null &&\n-- \n1.8.4-fc\n"},{"id":"226401","messageId":"1377923511-20787-3-git-send-email-felipe.contreras@gmail.com","threadId":"34815","inReplyTo":"1377923511-20787-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/4] t: branch: fix typo","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-31T04:31:49Z","receivedAt":"2013-08-31T04:31:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t3200-branch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex ea548f9..3134652 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -351,7 +351,7 @@ test_expect_success 'test overriding tracking setup via --no-track' '\n test_expect_success 'no tracking without .fetch entries' '\n \tgit config branch.autosetupmerge true &&\n \tgit branch my6 s &&\n-\tgit config branch.automsetupmerge false &&\n+\tgit config branch.autosetupmerge false &&\n \ttest -z \"$(git config branch.my6.remote)\" &&\n \ttest -z \"$(git config branch.my6.merge)\"\n '\n-- \n1.8.4-fc\n"},{"id":"226403","messageId":"1377923511-20787-4-git-send-email-felipe.contreras@gmail.com","threadId":"34815","inReplyTo":"1377923511-20787-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/4] t: branch: fix broken && chains","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-31T04:31:50Z","receivedAt":"2013-08-31T04:31:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t3200-branch.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 3134652..d85306f 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -425,14 +425,14 @@ test_expect_success '--set-upstream-to fails on a non-ref' '\n test_expect_success 'use --set-upstream-to modify HEAD' '\n \ttest_config branch.master.remote foo &&\n \ttest_config branch.master.merge foo &&\n-\tgit branch my12\n+\tgit branch my12 &&\n \tgit branch --set-upstream-to my12 &&\n \ttest \"$(git config branch.master.remote)\" = \".\" &&\n \ttest \"$(git config branch.master.merge)\" = \"refs/heads/my12\"\n '\n \n test_expect_success 'use --set-upstream-to modify a particular branch' '\n-\tgit branch my13\n+\tgit branch my13 &&\n \tgit branch --set-upstream-to master my13 &&\n \ttest \"$(git config branch.my13.remote)\" = \".\" &&\n \ttest \"$(git config branch.my13.merge)\" = \"refs/heads/master\"\n@@ -443,7 +443,7 @@ test_expect_success '--unset-upstream should fail if given a non-existent branch\n '\n \n test_expect_success 'test --unset-upstream on HEAD' '\n-\tgit branch my14\n+\tgit branch my14 &&\n \ttest_config branch.master.remote foo &&\n \ttest_config branch.master.merge foo &&\n \tgit branch --set-upstream-to my14 &&\n@@ -465,7 +465,7 @@ test_expect_success '--unset-upstream should fail on detached HEAD' '\n '\n \n test_expect_success 'test --unset-upstream on a particular branch' '\n-\tgit branch my15\n+\tgit branch my15 &&\n \tgit branch --set-upstream-to master my14 &&\n \tgit branch --unset-upstream my14 &&\n \ttest_must_fail git config branch.my14.remote &&\n-- \n1.8.4-fc\n"},{"id":"226402","messageId":"1377923511-20787-5-git-send-email-felipe.contreras@gmail.com","threadId":"34815","inReplyTo":"1377923511-20787-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 4/4] t: branch: improve test rollback","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-08-31T04:31:51Z","receivedAt":"2013-08-31T04:31:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"After every test the environment should be as close as to how it was\nbefore as possible.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t3200-branch.sh | 71 +++++++++++++++++++++++++++----------------------------\n 1 file changed, 35 insertions(+), 36 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex d85306f..3d4f634 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -328,7 +328,7 @@ test_expect_success 'tracking setup fails on non-matching refspec' '\n '\n \n test_expect_success 'test tracking setup via config' '\n-\tgit config branch.autosetupmerge true &&\n+\ttest_config branch.autosetupmerge true &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -338,20 +338,18 @@ test_expect_success 'test tracking setup via config' '\n '\n \n test_expect_success 'test overriding tracking setup via --no-track' '\n-\tgit config branch.autosetupmerge true &&\n+\ttest_config branch.autosetupmerge true &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n \tgit branch --no-track my2 local/master &&\n-\tgit config branch.autosetupmerge false &&\n \t! test \"$(git config branch.my2.remote)\" = local &&\n \t! test \"$(git config branch.my2.merge)\" = refs/heads/master\n '\n \n test_expect_success 'no tracking without .fetch entries' '\n-\tgit config branch.autosetupmerge true &&\n+\ttest_config branch.autosetupmerge true &&\n \tgit branch my6 s &&\n-\tgit config branch.autosetupmerge false &&\n \ttest -z \"$(git config branch.my6.remote)\" &&\n \ttest -z \"$(git config branch.my6.merge)\"\n '\n@@ -386,9 +384,8 @@ test_expect_success 'test --track without .fetch entries' '\n '\n \n test_expect_success 'branch from non-branch HEAD w/autosetupmerge=always' '\n-\tgit config branch.autosetupmerge always &&\n-\tgit branch my9 HEAD^ &&\n-\tgit config branch.autosetupmerge false\n+\ttest_config branch.autosetupmerge always &&\n+\tgit branch my9 HEAD^\n '\n \n test_expect_success 'branch from non-branch HEAD w/--track causes failure' '\n@@ -405,9 +402,9 @@ test_expect_success '--set-upstream-to fails on multiple branches' '\n '\n \n test_expect_success '--set-upstream-to fails on detached HEAD' '\n+\ttest_when_finished \"git checkout -\" &&\n \tgit checkout HEAD^{} &&\n-\ttest_must_fail git branch --set-upstream-to master &&\n-\tgit checkout -\n+\ttest_must_fail git branch --set-upstream-to master\n '\n \n test_expect_success '--set-upstream-to fails on a missing dst branch' '\n@@ -459,9 +456,9 @@ test_expect_success '--unset-upstream should fail on multiple branches' '\n '\n \n test_expect_success '--unset-upstream should fail on detached HEAD' '\n+\ttest_when_finished \"git checkout -\" &&\n \tgit checkout HEAD^{} &&\n-\ttest_must_fail git branch --unset-upstream &&\n-\tgit checkout -\n+\ttest_must_fail git branch --unset-upstream\n '\n \n test_expect_success 'test --unset-upstream on a particular branch' '\n@@ -540,7 +537,8 @@ test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates\n '\n \n test_expect_success 'avoid ambiguous track' '\n-\tgit config branch.autosetupmerge true &&\n+\ttest_when_finished \"git remote rm ambi1 && git remote rm ambi2\" &&\n+\ttest_config branch.autosetupmerge true &&\n \tgit config remote.ambi1.url lalala &&\n \tgit config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&\n \tgit config remote.ambi2.url lilili &&\n@@ -552,7 +550,7 @@ test_expect_success 'avoid ambiguous track' '\n test_expect_success 'autosetuprebase local on a tracked local branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase local &&\n+\ttest_config branch.autosetuprebase local &&\n \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n \tgit branch mybase &&\n \tgit branch --track myr1 mybase &&\n@@ -564,7 +562,7 @@ test_expect_success 'autosetuprebase local on a tracked local branch' '\n test_expect_success 'autosetuprebase always on a tracked local branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase always &&\n+\ttest_config branch.autosetuprebase always &&\n \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n \tgit branch mybase2 &&\n \tgit branch --track myr2 mybase &&\n@@ -576,7 +574,7 @@ test_expect_success 'autosetuprebase always on a tracked local branch' '\n test_expect_success 'autosetuprebase remote on a tracked local branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase remote &&\n+\ttest_config branch.autosetuprebase remote &&\n \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n \tgit branch mybase3 &&\n \tgit branch --track myr3 mybase2 &&\n@@ -588,7 +586,7 @@ test_expect_success 'autosetuprebase remote on a tracked local branch' '\n test_expect_success 'autosetuprebase never on a tracked local branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase never &&\n+\ttest_config branch.autosetuprebase never &&\n \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n \tgit branch mybase4 &&\n \tgit branch --track myr4 mybase2 &&\n@@ -600,7 +598,7 @@ test_expect_success 'autosetuprebase never on a tracked local branch' '\n test_expect_success 'autosetuprebase local on a tracked remote branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase local &&\n+\ttest_config branch.autosetuprebase local &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n \tgit branch --track myr5 local/master &&\n \ttest \"$(git config branch.myr5.remote)\" = local &&\n@@ -611,7 +609,7 @@ test_expect_success 'autosetuprebase local on a tracked remote branch' '\n test_expect_success 'autosetuprebase never on a tracked remote branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase never &&\n+\ttest_config branch.autosetuprebase never &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n \tgit branch --track myr6 local/master &&\n \ttest \"$(git config branch.myr6.remote)\" = local &&\n@@ -622,7 +620,7 @@ test_expect_success 'autosetuprebase never on a tracked remote branch' '\n test_expect_success 'autosetuprebase remote on a tracked remote branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase remote &&\n+\ttest_config branch.autosetuprebase remote &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n \tgit branch --track myr7 local/master &&\n \ttest \"$(git config branch.myr7.remote)\" = local &&\n@@ -633,7 +631,7 @@ test_expect_success 'autosetuprebase remote on a tracked remote branch' '\n test_expect_success 'autosetuprebase always on a tracked remote branch' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n-\tgit config branch.autosetuprebase remote &&\n+\ttest_config branch.autosetuprebase remote &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n \tgit branch --track myr8 local/master &&\n \ttest \"$(git config branch.myr8.remote)\" = local &&\n@@ -642,7 +640,7 @@ test_expect_success 'autosetuprebase always on a tracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '\n-\tgit config --unset branch.autosetuprebase &&\n+\ttest_unconfig branch.autosetuprebase &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -684,7 +682,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase never on an untracked local branch' '\n-\tgit config branch.autosetuprebase never &&\n+\ttest_config branch.autosetuprebase never &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -695,7 +693,7 @@ test_expect_success 'autosetuprebase never on an untracked local branch' '\n '\n \n test_expect_success 'autosetuprebase local on an untracked local branch' '\n-\tgit config branch.autosetuprebase local &&\n+\ttest_config branch.autosetuprebase local &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -706,7 +704,7 @@ test_expect_success 'autosetuprebase local on an untracked local branch' '\n '\n \n test_expect_success 'autosetuprebase remote on an untracked local branch' '\n-\tgit config branch.autosetuprebase remote &&\n+\ttest_config branch.autosetuprebase remote &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -717,7 +715,7 @@ test_expect_success 'autosetuprebase remote on an untracked local branch' '\n '\n \n test_expect_success 'autosetuprebase always on an untracked local branch' '\n-\tgit config branch.autosetuprebase always &&\n+\ttest_config branch.autosetuprebase always &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -728,7 +726,7 @@ test_expect_success 'autosetuprebase always on an untracked local branch' '\n '\n \n test_expect_success 'autosetuprebase never on an untracked remote branch' '\n-\tgit config branch.autosetuprebase never &&\n+\ttest_config branch.autosetuprebase never &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -739,7 +737,7 @@ test_expect_success 'autosetuprebase never on an untracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase local on an untracked remote branch' '\n-\tgit config branch.autosetuprebase local &&\n+\ttest_config branch.autosetuprebase local &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -750,7 +748,7 @@ test_expect_success 'autosetuprebase local on an untracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase remote on an untracked remote branch' '\n-\tgit config branch.autosetuprebase remote &&\n+\ttest_config branch.autosetuprebase remote &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -761,7 +759,7 @@ test_expect_success 'autosetuprebase remote on an untracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase always on an untracked remote branch' '\n-\tgit config branch.autosetuprebase always &&\n+\ttest_config branch.autosetuprebase always &&\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n@@ -772,8 +770,8 @@ test_expect_success 'autosetuprebase always on an untracked remote branch' '\n '\n \n test_expect_success 'autosetuprebase always on detached HEAD' '\n-\tgit config branch.autosetupmerge always &&\n-\ttest_when_finished git checkout master &&\n+\ttest_when_finished \"git checkout -\" &&\n+\ttest_config branch.autosetupmerge always &&\n \tgit checkout HEAD^0 &&\n \tgit branch my11 &&\n \ttest -z \"$(git config branch.my11.remote)\" &&\n@@ -781,15 +779,15 @@ test_expect_success 'autosetuprebase always on detached HEAD' '\n '\n \n test_expect_success 'detect misconfigured autosetuprebase (bad value)' '\n-\tgit config branch.autosetuprebase garbage &&\n+\ttest_config branch.autosetuprebase garbage &&\n \ttest_must_fail git branch\n '\n \n test_expect_success 'detect misconfigured autosetuprebase (no value)' '\n-\tgit config --unset branch.autosetuprebase &&\n+\ttest_when_finished \"test_unconfig branch.autosetuprebase\" &&\n+\ttest_unconfig branch.autosetuprebase &&\n \techo \"[branch] autosetuprebase\" >>.git/config &&\n-\ttest_must_fail git branch &&\n-\tgit config --unset branch.autosetuprebase\n+\ttest_must_fail git branch\n '\n \n test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '\n@@ -856,6 +854,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n '\n \n test_expect_success 'refuse --edit-description on unborn branch for now' '\n+\ttest_when_finished \"git checkout -\" &&\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n-- \n1.8.4-fc\n"},{"id":"226655","messageId":"xmqq8uzdsopq.fsf@gitster.dls.corp.google.com","threadId":"34815","inReplyTo":"1377923511-20787-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/4] t: branch: trivial style fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-03T19:13:37Z","receivedAt":"2013-09-03T19:13:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The first three are trivially correct and good changes. I still have\nto read the last one, but wanted to give a Thanks before doing so,\nwhich would take more time.\n"},{"id":"226658","messageId":"xmqqwqmxr9a7.fsf@gitster.dls.corp.google.com","threadId":"34815","inReplyTo":"1377923511-20787-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-03T19:32:16Z","receivedAt":"2013-09-03T19:32:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> After every test the environment should be as close as to how it was\n> before as possible.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n\nVery good in general; a few points (not \"the patch breaks things\",\nbut more like \"tests after the patch are still depending on the\nprevious state\") below, though.\n\n>  t/t3200-branch.sh | 71 +++++++++++++++++++++++++++----------------------------\n>  1 file changed, 35 insertions(+), 36 deletions(-)\n>\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index d85306f..3d4f634 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -328,7 +328,7 @@ test_expect_success 'tracking setup fails on non-matching refspec' '\n>  '\n>  \n>  test_expect_success 'test tracking setup via config' '\n> -\tgit config branch.autosetupmerge true &&\n> +\ttest_config branch.autosetupmerge true &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -338,20 +338,18 @@ test_expect_success 'test tracking setup via config' '\n>  '\n>  \n>  test_expect_success 'test overriding tracking setup via --no-track' '\n> -\tgit config branch.autosetupmerge true &&\n> +\ttest_config branch.autosetupmerge true &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n>  \tgit branch --no-track my2 local/master &&\n> -\tgit config branch.autosetupmerge false &&\n>  \t! test \"$(git config branch.my2.remote)\" = local &&\n>  \t! test \"$(git config branch.my2.merge)\" = refs/heads/master\n>  '\n>  \n>  test_expect_success 'no tracking without .fetch entries' '\n> -\tgit config branch.autosetupmerge true &&\n> +\ttest_config branch.autosetupmerge true &&\n>  \tgit branch my6 s &&\n> -\tgit config branch.autosetupmerge false &&\n>  \ttest -z \"$(git config branch.my6.remote)\" &&\n>  \ttest -z \"$(git config branch.my6.merge)\"\n>  '\n\nThe four tests after this one used to expect to start with\nbranch.autosetupmerge explicitly set to false, but this change\nleaves the variable unset---as a side effect we start testing a\ndifferent thing.  As long as we do not introduce a bug that allows\nan exlicit branch.autosetupmerge=false defeat --track on the command\nline, this side effect does not hurt, but I think the 'test tracking\nsetup via --track but deeper' was the only test that checks this\ninteraction between 'false in config, --track on the command line'\ncombination.\n\n> @@ -386,9 +384,8 @@ test_expect_success 'test --track without .fetch entries' '\n>  '\n>  \n>  test_expect_success 'branch from non-branch HEAD w/autosetupmerge=always' '\n> -\tgit config branch.autosetupmerge always &&\n> -\tgit branch my9 HEAD^ &&\n> -\tgit config branch.autosetupmerge false\n> +\ttest_config branch.autosetupmerge always &&\n> +\tgit branch my9 HEAD^\n>  '\n\nLikewise for three subsequent tests but what they test are not\nprimarily whether tracking is done, so it does not matter as much as\nthe previous.\n\n>  test_expect_success 'branch from non-branch HEAD w/--track causes failure' '\n> @@ -405,9 +402,9 @@ test_expect_success '--set-upstream-to fails on multiple branches' '\n>  '\n>  \n>  test_expect_success '--set-upstream-to fails on detached HEAD' '\n> +\ttest_when_finished \"git checkout -\" &&\n>  \tgit checkout HEAD^{} &&\n> -\ttest_must_fail git branch --set-upstream-to master &&\n> -\tgit checkout -\n> +\ttest_must_fail git branch --set-upstream-to master\n>  '\n>  \n>  test_expect_success '--set-upstream-to fails on a missing dst branch' '\n> @@ -459,9 +456,9 @@ test_expect_success '--unset-upstream should fail on multiple branches' '\n>  '\n>  \n>  test_expect_success '--unset-upstream should fail on detached HEAD' '\n> +\ttest_when_finished \"git checkout -\" &&\n>  \tgit checkout HEAD^{} &&\n> -\ttest_must_fail git branch --unset-upstream &&\n> -\tgit checkout -\n> +\ttest_must_fail git branch --unset-upstream\n>  '\n>  \n>  test_expect_success 'test --unset-upstream on a particular branch' '\n> @@ -540,7 +537,8 @@ test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates\n>  '\n>  \n>  test_expect_success 'avoid ambiguous track' '\n> -\tgit config branch.autosetupmerge true &&\n> +\ttest_when_finished \"git remote rm ambi1 && git remote rm ambi2\" &&\n> +\ttest_config branch.autosetupmerge true &&\n>  \tgit config remote.ambi1.url lalala &&\n>  \tgit config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&\n>  \tgit config remote.ambi2.url lilili &&\n> @@ -552,7 +550,7 @@ test_expect_success 'avoid ambiguous track' '\n>  test_expect_success 'autosetuprebase local on a tracked local branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase local &&\n> +\ttest_config branch.autosetuprebase local &&\n>  \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n>  \tgit branch mybase &&\n>  \tgit branch --track myr1 mybase &&\n> @@ -564,7 +562,7 @@ test_expect_success 'autosetuprebase local on a tracked local branch' '\n>  test_expect_success 'autosetuprebase always on a tracked local branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase always &&\n> +\ttest_config branch.autosetuprebase always &&\n>  \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n>  \tgit branch mybase2 &&\n>  \tgit branch --track myr2 mybase &&\n> @@ -576,7 +574,7 @@ test_expect_success 'autosetuprebase always on a tracked local branch' '\n>  test_expect_success 'autosetuprebase remote on a tracked local branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase remote &&\n> +\ttest_config branch.autosetuprebase remote &&\n>  \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n>  \tgit branch mybase3 &&\n>  \tgit branch --track myr3 mybase2 &&\n> @@ -588,7 +586,7 @@ test_expect_success 'autosetuprebase remote on a tracked local branch' '\n>  test_expect_success 'autosetuprebase never on a tracked local branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase never &&\n> +\ttest_config branch.autosetuprebase never &&\n>  \t(git show-ref -q refs/remotes/local/o || git fetch local) &&\n>  \tgit branch mybase4 &&\n>  \tgit branch --track myr4 mybase2 &&\n> @@ -600,7 +598,7 @@ test_expect_success 'autosetuprebase never on a tracked local branch' '\n>  test_expect_success 'autosetuprebase local on a tracked remote branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase local &&\n> +\ttest_config branch.autosetuprebase local &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n>  \tgit branch --track myr5 local/master &&\n>  \ttest \"$(git config branch.myr5.remote)\" = local &&\n> @@ -611,7 +609,7 @@ test_expect_success 'autosetuprebase local on a tracked remote branch' '\n>  test_expect_success 'autosetuprebase never on a tracked remote branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase never &&\n> +\ttest_config branch.autosetuprebase never &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n>  \tgit branch --track myr6 local/master &&\n>  \ttest \"$(git config branch.myr6.remote)\" = local &&\n> @@ -622,7 +620,7 @@ test_expect_success 'autosetuprebase never on a tracked remote branch' '\n>  test_expect_success 'autosetuprebase remote on a tracked remote branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase remote &&\n> +\ttest_config branch.autosetuprebase remote &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n>  \tgit branch --track myr7 local/master &&\n>  \ttest \"$(git config branch.myr7.remote)\" = local &&\n> @@ -633,7 +631,7 @@ test_expect_success 'autosetuprebase remote on a tracked remote branch' '\n>  test_expect_success 'autosetuprebase always on a tracked remote branch' '\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n> -\tgit config branch.autosetuprebase remote &&\n> +\ttest_config branch.autosetuprebase remote &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n>  \tgit branch --track myr8 local/master &&\n>  \ttest \"$(git config branch.myr8.remote)\" = local &&\n> @@ -642,7 +640,7 @@ test_expect_success 'autosetuprebase always on a tracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '\n> -\tgit config --unset branch.autosetuprebase &&\n> +\ttest_unconfig branch.autosetuprebase &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -684,7 +682,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase never on an untracked local branch' '\n> -\tgit config branch.autosetuprebase never &&\n> +\ttest_config branch.autosetuprebase never &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -695,7 +693,7 @@ test_expect_success 'autosetuprebase never on an untracked local branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase local on an untracked local branch' '\n> -\tgit config branch.autosetuprebase local &&\n> +\ttest_config branch.autosetuprebase local &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -706,7 +704,7 @@ test_expect_success 'autosetuprebase local on an untracked local branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase remote on an untracked local branch' '\n> -\tgit config branch.autosetuprebase remote &&\n> +\ttest_config branch.autosetuprebase remote &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -717,7 +715,7 @@ test_expect_success 'autosetuprebase remote on an untracked local branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase always on an untracked local branch' '\n> -\tgit config branch.autosetuprebase always &&\n> +\ttest_config branch.autosetuprebase always &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -728,7 +726,7 @@ test_expect_success 'autosetuprebase always on an untracked local branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase never on an untracked remote branch' '\n> -\tgit config branch.autosetuprebase never &&\n> +\ttest_config branch.autosetuprebase never &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -739,7 +737,7 @@ test_expect_success 'autosetuprebase never on an untracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase local on an untracked remote branch' '\n> -\tgit config branch.autosetuprebase local &&\n> +\ttest_config branch.autosetuprebase local &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -750,7 +748,7 @@ test_expect_success 'autosetuprebase local on an untracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase remote on an untracked remote branch' '\n> -\tgit config branch.autosetuprebase remote &&\n> +\ttest_config branch.autosetuprebase remote &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -761,7 +759,7 @@ test_expect_success 'autosetuprebase remote on an untracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase always on an untracked remote branch' '\n> -\tgit config branch.autosetuprebase always &&\n> +\ttest_config branch.autosetuprebase always &&\n>  \tgit config remote.local.url . &&\n>  \tgit config remote.local.fetch refs/heads/*:refs/remotes/local/* &&\n>  \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\n> @@ -772,8 +770,8 @@ test_expect_success 'autosetuprebase always on an untracked remote branch' '\n>  '\n>  \n>  test_expect_success 'autosetuprebase always on detached HEAD' '\n> -\tgit config branch.autosetupmerge always &&\n> -\ttest_when_finished git checkout master &&\n> +\ttest_when_finished \"git checkout -\" &&\n\nI think the explicit 'master' was better.  We are not in the\nbusiness of checking @{-1} completion here, and depending on the\noutcome of the \"git checkout\" in the test, \"checkout -\" will take us\nto a different place, no?\n\n> +\ttest_config branch.autosetupmerge always &&\n\nThis used to be propagated down, but now branch.autosetupmerge is\nkept unset after this.  The tests after this one do not seem to\nnewly create branches that need auto-setup-merge, so I think this\nchange is very good.\n\n>  \tgit checkout HEAD^0 &&\n>  \tgit branch my11 &&\n>  \ttest -z \"$(git config branch.my11.remote)\" &&\n> @@ -781,15 +779,15 @@ test_expect_success 'autosetuprebase always on detached HEAD' '\n>  '\n>  \n>  test_expect_success 'detect misconfigured autosetuprebase (bad value)' '\n> -\tgit config branch.autosetuprebase garbage &&\n> +\ttest_config branch.autosetuprebase garbage &&\n>  \ttest_must_fail git branch\n>  '\n>  \n>  test_expect_success 'detect misconfigured autosetuprebase (no value)' '\n> -\tgit config --unset branch.autosetuprebase &&\n> +\ttest_when_finished \"test_unconfig branch.autosetuprebase\" &&\n> +\ttest_unconfig branch.autosetuprebase &&\n>  \techo \"[branch] autosetuprebase\" >>.git/config &&\n> -\ttest_must_fail git branch &&\n> -\tgit config --unset branch.autosetuprebase\n> +\ttest_must_fail git branch\n>  '\n>  \n>  test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '\n> @@ -856,6 +854,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n>  '\n>  \n>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n> +\ttest_when_finished \"git checkout -\" &&\n\nI am not sure if this is a good change.  Depending on the outcome of\nthe \"git checkout\" in the test (it may succeed and set @{-1} to the\nbranch we were on when we entered the test, or it may fail and leave\n@{-1} to the branch before we were on when we entered the test),\nthis will take us to a different place, no?\n\n>  \twrite_script editor <<-\\EOF &&\n>  \t\techo \"New contents\" >\"$1\"\n>  \tEOF\n"},{"id":"226685","messageId":"CAMP44s2BU86zZ_KE78BPYpsjBZJ7Mj0MJqM4Lj28AKxYfeLVDQ@mail.gmail.com","threadId":"34815","inReplyTo":"xmqqwqmxr9a7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-03T22:10:25Z","receivedAt":"2013-09-03T22:10:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n>> +     test_when_finished \"git checkout -\" &&\n>\n> I am not sure if this is a good change.  Depending on the outcome of\n> the \"git checkout\" in the test (it may succeed and set @{-1} to the\n> branch we were on when we entered the test, or it may fail and leave\n> @{-1} to the branch before we were on when we entered the test),\n> this will take us to a different place, no?\n\nIt is better than leaving Git in an unborn branch. Many tests could\nnot care which is the current branch, as long as HEAD does points to a\nbranch, and that is the default, the tests should be messing with the\ndefault.\n\n-- \nFelipe Contreras\n"},{"id":"226689","messageId":"xmqqtxi1o6kf.fsf@gitster.dls.corp.google.com","threadId":"34815","inReplyTo":"CAMP44s2BU86zZ_KE78BPYpsjBZJ7Mj0MJqM4Lj28AKxYfeLVDQ@mail.gmail.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-03T22:59:12Z","receivedAt":"2013-09-03T22:59:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n>>> +     test_when_finished \"git checkout -\" &&\n>>\n>> I am not sure if this is a good change.  Depending on the outcome of\n>> the \"git checkout\" in the test (it may succeed and set @{-1} to the\n>> branch we were on when we entered the test, or it may fail and leave\n>> @{-1} to the branch before we were on when we entered the test),\n>> this will take us to a different place, no?\n>\n> It is better than leaving Git in an unborn branch.\n\nTrue, but it falls short of \"After every test the environment should\nbe as close as to how it was before as possible.\", doesn't it?\n\nI think the fix is just to use the name of a concrete branch,\ninstead of \"-\", which could be far quickly done than making excuses\nlike that.\n"},{"id":"226692","messageId":"CAMP44s1Q6YjVnVz3OnT=cE+ozUddBxFzM1r_WT5QoR7sP6-9Sg@mail.gmail.com","threadId":"34815","inReplyTo":"xmqqtxi1o6kf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-03T23:03:09Z","receivedAt":"2013-09-03T23:03:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>>>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n>>>> +     test_when_finished \"git checkout -\" &&\n>>>\n>>> I am not sure if this is a good change.  Depending on the outcome of\n>>> the \"git checkout\" in the test (it may succeed and set @{-1} to the\n>>> branch we were on when we entered the test, or it may fail and leave\n>>> @{-1} to the branch before we were on when we entered the test),\n>>> this will take us to a different place, no?\n>>\n>> It is better than leaving Git in an unborn branch.\n>\n> True, but it falls short of \"After every test the environment should\n> be as close as to how it was before as possible.\", doesn't it?\n\nNo it doesn't. Closer is closer.\n\n> I think the fix is just to use the name of a concrete branch,\n> instead of \"-\", which could be far quickly done than making excuses\n> like that.\n\nThe fix is to write a test_checkout that would save the previous HEAD\nand restore it after the test, or something similar. But I'm not going\nto write that, and I bet nobody else is, so unless you want the tests\nto remain in the crappy state they are, \"git checkout -\" is the best\nsensible option.\n\n-- \nFelipe Contreras\n"},{"id":"226743","messageId":"xmqq61ugo67v.fsf@gitster.dls.corp.google.com","threadId":"34815","inReplyTo":"CAMP44s1Q6YjVnVz3OnT=cE+ozUddBxFzM1r_WT5QoR7sP6-9Sg@mail.gmail.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-04T17:19:00Z","receivedAt":"2013-09-04T17:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>>>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n>>>>> +     test_when_finished \"git checkout -\" &&\n>>>>\n>>>> I am not sure if this is a good change.  Depending on the outcome of\n>>>> the \"git checkout\" in the test (it may succeed and set @{-1} to the\n>>>> branch we were on when we entered the test, or it may fail and leave\n>>>> @{-1} to the branch before we were on when we entered the test),\n>>>> this will take us to a different place, no?\n>>>\n>>> It is better than leaving Git in an unborn branch.\n>>\n>> True, but it falls short of \"After every test the environment should\n>> be as close as to how it was before as possible.\", doesn't it?\n>\n> No it doesn't. Closer is closer.\n\nCloser is not \"as close as ... possible\".\n\nI think the fix is just to use the name of a concrete branch we want\nthe next test to start on, instead of \"-\", which could be far\nquickly done than making excuses like that.\n"},{"id":"227054","messageId":"CAMP44s2PFVAKRUkEyN8BAH8C5UUMn21ou9hNkcOXaCsXscywHQ@mail.gmail.com","threadId":"34815","inReplyTo":"xmqq61ugo67v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-08T03:02:59Z","receivedAt":"2013-09-08T03:02:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Sep 4, 2013 at 12:19 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>>\n>>>>>>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n>>>>>> +     test_when_finished \"git checkout -\" &&\n>>>>>\n>>>>> I am not sure if this is a good change.  Depending on the outcome of\n>>>>> the \"git checkout\" in the test (it may succeed and set @{-1} to the\n>>>>> branch we were on when we entered the test, or it may fail and leave\n>>>>> @{-1} to the branch before we were on when we entered the test),\n>>>>> this will take us to a different place, no?\n>>>>\n>>>> It is better than leaving Git in an unborn branch.\n>>>\n>>> True, but it falls short of \"After every test the environment should\n>>> be as close as to how it was before as possible.\", doesn't it?\n>>\n>> No it doesn't. Closer is closer.\n>\n> Closer is not \"as close as ... possible\".\n\nYes it is, because there are no better alternatives within our\nability, capacity, or realization.\n\nhttp://www.merriam-webster.com/dictionary/possible\n\n> I think the fix is just to use the name of a concrete branch we want\n> the next test to start on, instead of \"-\", which could be far\n> quickly done than making excuses like that.\n\nNo, what if that test is skipped for some reason? Ideally each test\ncase should do 'git checkout $branch_i_need', not rely on the previous\ntests to checkout the branch they need, but that requires too much\nwork to fix.\n\n'git checkout -' works perfectly fine, and it's the closest we have to\nmy suggested test_checkout() which I think would be ideal, but I'm not\ngoing to work on.\n\n-- \nFelipe Contreras\n"},{"id":"227068","messageId":"20130908045606.GF14019@sigill.intra.peff.net","threadId":"34815","inReplyTo":"CAMP44s2PFVAKRUkEyN8BAH8C5UUMn21ou9hNkcOXaCsXscywHQ@mail.gmail.com","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-08T04:56:06Z","receivedAt":"2013-09-08T04:56:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 07, 2013 at 10:02:59PM -0500, Felipe Contreras wrote:\n\n> 'git checkout -' works perfectly fine, and it's the closest we have to\n> my suggested test_checkout() which I think would be ideal, but I'm not\n> going to work on.\n\nI do not think it works perfectly fine if the checkout itself fails. For\nexample, after your patch, one of the tests reads:\n\n  test_when_finished \"git checkout -\" &&\n  git checkout HEAD^{} &&\n  test_must_fail git branch --set-upstream-to master\n\nThe cleanup is undoing what happened on the second line. So if we fail\non the third line, it does what we want. If we fail on the second line,\nthen what branch are switching to?\n\nI think you would at least want to reverse the order as:\n\n  git checkout HEAD^{} &&\n  test_when_finished \"git checkout -\" &&\n  test_must_fail git branch --set-upstream-to master\n\nso that you know you are actually going to the pre-test state.\n\n-Peff\n"},{"id":"227069","messageId":"CAMP44s3PPrtr2ham2LWeeGu3tV+2qBd2Xhv1X5RCntbubUvFuw@mail.gmail.com","threadId":"34815","inReplyTo":"20130908045606.GF14019@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] t: branch: improve test rollback","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-08T05:01:11Z","receivedAt":"2013-09-08T05:01:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Sep 7, 2013 at 11:56 PM, Jeff King <peff@peff.net> wrote:\n> On Sat, Sep 07, 2013 at 10:02:59PM -0500, Felipe Contreras wrote:\n>\n>> 'git checkout -' works perfectly fine, and it's the closest we have to\n>> my suggested test_checkout() which I think would be ideal, but I'm not\n>> going to work on.\n>\n> I do not think it works perfectly fine if the checkout itself fails. For\n> example, after your patch, one of the tests reads:\n>\n>   test_when_finished \"git checkout -\" &&\n>   git checkout HEAD^{} &&\n>   test_must_fail git branch --set-upstream-to master\n>\n> The cleanup is undoing what happened on the second line. So if we fail\n> on the third line, it does what we want. If we fail on the second line,\n> then what branch are switching to?\n>\n> I think you would at least want to reverse the order as:\n>\n>   git checkout HEAD^{} &&\n>   test_when_finished \"git checkout -\" &&\n>   test_must_fail git branch --set-upstream-to master\n>\n> so that you know you are actually going to the pre-test state.\n\nYeah, that would be better and easily achievable.\n\n-- \nFelipe Contreras\n"}]}