Re: [PATCH v2 4/4] receive.denyCurrentBranch: respect all worktrees
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 23, 2020, 06:18 UTC
- Message-ID
- <xmqqftf1su6n.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <61e5f75a6f9a8579271870f6b8b95021055a96ad.1582410908.git.gitgitgadget@gmail.com>
"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 33 quoted lines
> 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.