[PATCH] t3200: clean side effect of git checkout --orphan

[PATCH] t3200: clean side effect of git checkout --orphan

[Aaron Lipman]

The "refuse --edit-description on unborn branch for now" test in t3200
switches to an orphan branch, causing subsequent git commands
referencing HEAD to fail. Avoid this side-effect by switching back to
master after that test finishes.

This has gone undetected, as the next effected test expects failure -
but it currently fails for the wrong reason.

Verbose output of the next test referencing HEAD,
"--merged is incompatible with --no-merged":

  fatal: malformed object name HEAD

Which this commit corrects to:

  error: option `no-merged' is incompatible with --merged

Signed-off-by: Aaron Lipman <alipman88@gmail.com>
---
We might considered updating "--merged is incompatible with --no-merged"
tests to not only test for failure, but check the contents of the error
message to ensure they fail for the correct reason, e.g.:

test_expect_success '--merged is incompatible with --no-merged' '
  test_must_fail git branch --merged HEAD --no-merged HEAD 2>error &&
  test_i18ngrep "is incompatible with --merged" error
'

However, I intend to submit a patch enabling ref-filter to accept
multiple merged/no-merged filters, which would make said updates
irrelevant.

 t/t3200-branch.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4c0734157b..028c88d1b2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1287,6 +1287,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '
 '
 
 test_expect_success 'refuse --edit-description on unborn branch for now' '
+	test_when_finished "git checkout master" &&
 	write_script editor <<-\EOF &&
 		echo "New contents" >"$1"
 	EOF
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] t3200: clean side effect of git checkout --orphan

[Eric Sunshine]

On Sat, Aug 29, 2020 at 6:57 PM Aaron Lipman <alipman88@gmail.com> wrote:
> The "refuse --edit-description on unborn branch for now" test in t3200
> switches to an orphan branch, causing subsequent git commands
> referencing HEAD to fail. Avoid this side-effect by switching back to
> master after that test finishes.
>
> This has gone undetected, as the next effected test expects failure -
> but it currently fails for the wrong reason.

s/effected/affected

In fact, the three tests following the orphan test all expect failure
(though I didn't check if they also fail for the wrong reason), and
the following test which doesn't expect failure has an explicit "git
checkout master" early on, which explains why it was not impacted by
this problem.

> Verbose output of the next test referencing HEAD,
> "--merged is incompatible with --no-merged":
>
>   fatal: malformed object name HEAD
>
> Which this commit corrects to:
>
>   error: option `no-merged' is incompatible with --merged
>
> Signed-off-by: Aaron Lipman <alipman88@gmail.com>

Description and actual fix make perfect sense.

[PATCH v2] t3200: clean side effect of git checkout --orphan

[Aaron Lipman]

The "refuse --edit-description on unborn branch for now" test in t3200
switches to an orphan branch, causing subsequent git commands
referencing HEAD to fail. Avoid this side-effect by switching back to
master after the test finishes.

This has gone undetected, as the next affected test expects failure -
but it currently fails for the wrong reason.

Verbose output of the next test referencing HEAD,
"--merged is incompatible with --no-merged":

  fatal: malformed object name HEAD

Which this commit corrects to:

  error: option `no-merged' is incompatible with --merged

Signed-off-by: Aaron Lipman <alipman88@gmail.com>
---
Whoops, fixed the effected/affected error.
Thanks for catching that, Eric!

 t/t3200-branch.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4c0734157b..028c88d1b2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1287,6 +1287,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '
 '
 
 test_expect_success 'refuse --edit-description on unborn branch for now' '
+	test_when_finished "git checkout master" &&
 	write_script editor <<-\EOF &&
 		echo "New contents" >"$1"
 	EOF
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] t3200: clean side effect of git checkout --orphan

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sat, Aug 29, 2020 at 6:57 PM Aaron Lipman <alipman88@gmail.com> wrote:
>> The "refuse --edit-description on unborn branch for now" test in t3200
>> switches to an orphan branch, causing subsequent git commands
>> referencing HEAD to fail. Avoid this side-effect by switching back to
>> master after that test finishes.
>>
>> This has gone undetected, as the next effected test expects failure -
>> but it currently fails for the wrong reason.
>
> s/effected/affected
>
> In fact, the three tests following the orphan test all expect failure
> (though I didn't check if they also fail for the wrong reason), and
> the following test which doesn't expect failure has an explicit "git
> checkout master" early on, which explains why it was not impacted by
> this problem.
>
>> Verbose output of the next test referencing HEAD,
>> "--merged is incompatible with --no-merged":
>>
>>   fatal: malformed object name HEAD
>>
>> Which this commit corrects to:
>>
>>   error: option `no-merged' is incompatible with --merged
>>
>> Signed-off-by: Aaron Lipman <alipman88@gmail.com>
>
> Description and actual fix make perfect sense.

Yeah, looks good.  Of course, the affected test can be made more
defensive to protect the precondition it relies on from getting
broken by other tests (i.e. if it refers to HEAD, it should make
sure it is on an actual commit).   Each test cleaning up after
itself is a good discipline to have, but what is "clean" is quite
subjective and depends on each test piece in the script X-<.

Thanks, will queue.