Re: [PATCH v2 4/4] receive.denyCurrentBranch: respect all worktrees

[Junio C Hamano <gitster@pobox.com>]

"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
> index c89483fdba2..6270fb7b576 100755
> --- a/t/t5509-fetch-push-namespaces.sh
> +++ b/t/t5509-fetch-push-namespaces.sh
> @@ -152,7 +152,7 @@ test_expect_success 'clone chooses correct HEAD (v2)' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure 'denyCurrentBranch and unborn branch with ref namespace' '
> +test_expect_success 'denyCurrentBranch and unborn branch with ref namespace' '
>  	cd original &&
>  	git init unborn &&
>  	git remote add unborn-namespaced "ext::git --namespace=namespace %s unborn" &&
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index c81ca360ac4..49982b0fd90 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1712,4 +1712,15 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
>  	)
>  '
>  
> +test_expect_success 'denyCurrentBranch and worktrees' '
> +	git worktree add new-wt &&
> +	git clone . cloned &&
> +	test_commit -C cloned first &&
> +	test_config receive.denyCurrentBranch refuse &&
> +	test_must_fail git -C cloned push origin HEAD:new-wt &&
> +	test_config receive.denyCurrentBranch updateInstead &&
> +	git -C cloned push origin HEAD:new-wt &&
> +	test_must_fail git -C cloned push --delete origin new-wt
> +'
> +
>  test_done

This adds one new test and also flips a test that was added in a
separate step that expected a failure to expect success, which looks
a bit strange.

For a series this small, having a test that demonstrates that the
updated code works as expected together with the fix to the code in
a single patch is easier to manage.  After applying a single
test+fix patch, you can easily apply the same patch except for the
test part in reverse on top, if you need to see in what way the code
without the change breaks by running the test.

On a truly large fix, sometimes it may make sense to add a failing
test and nothing else and then a separate step that changes the code
and flips the expectation of the test from failure->success, but I
think a change this size is easier to handle without such an artificial
split.

Thanks.