Re: [PATCH] merge-base: add tests for --is-ancestor
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 29, 2026, 15:31 UTC
- Message-ID
- <xmqqo6fpak2k.fsf@gitster.g>
- In-Reply-To
- <9a47d529-6195-435b-90a6-e511856f128e@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 23 quoted lines
> Thanks for adding some tests for this option > >> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh >> index 44c726ea39..d28d9dab2c 100755 >> --- a/t/t6010-merge-base.sh >> +++ b/t/t6010-merge-base.sh >> @@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' ' >> test_cmp expected actual >> ' >> >> +test_expect_success 'setup --is-ancestor' ' >> + git init is-ancestor && >> + ( >> + cd is-ancestor && >> + test_commit one && >> + test_commit two && >> + git checkout -b side one && >> + test_commit three >> + ) >> +' > > Do we really need to create a new repository? None of the existing tests > do that - can't we just use the commits created by the first test?
Good point. It is often a lot more work to do so when writing these new tests, and I suspect that is the reason why this new repository was created, but I do not know whether, in the long run, it is cheaper and more maintainable to reuse the test history that has been used by all existing tests above. The next developer who needs to tweak the first test has 5 (the number of tests below) extra constraints if we go that route, but a separate setup means the two sets of tests are more isolated.
> Having said that, the tests below look good. We could perhaps have a > test to check that it fails with one commit or three commits but what > you have here is clearly a very useful improvement in our test coverage.
Thanks.