{"thread":{"id":"66065","subject":"[PATCH] merge-base: add tests for --is-ancestor","startedAt":"2026-07-25T17:00:30Z","lastAt":"2026-07-30T16:02:53Z","messageCount":6,"participants":["Nikolaus Schuetz via GitGitGadget","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"548972","messageId":"pull.2186.git.1784998828879.gitgitgadget@gmail.com","threadId":"66065","inReplyTo":null,"subject":"[PATCH] merge-base: add tests for --is-ancestor","fromName":"Nikolaus Schuetz via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-25T17:00:28Z","receivedAt":"2026-07-25T17:00:30Z","isPatch":true,"body":"From: Nikolaus Schuetz \n\n`git merge-base --is-ancestor A B` is used a lot in scripts but has no\ntests. Add some to t6010 covering its exit codes: 0 when A is an\nancestor of B, 1 when it is not, and 128 (not 1) when given a bad\nargument. Also check that --is-ancestor and --all can't be combined,\nand that the resulting error names both options.\n\nSigned-off-by: Nikolaus Schuetz \n---\n merge-base: add tests for --is-ancestor\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2186\n\n t/t6010-merge-base.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex 44c726ea39..d28d9dab2c 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'setup --is-ancestor' '\n+\tgit init is-ancestor &&\n+\t(\n+\t\tcd is-ancestor &&\n+\t\ttest_commit one &&\n+\t\ttest_commit two &&\n+\t\tgit checkout -b side one &&\n+\t\ttest_commit three\n+\t)\n+'\n+\n+test_expect_success '--is-ancestor parent and child' '\n+\tgit -C is-ancestor merge-base --is-ancestor one two &&\n+\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one\n+'\n+\n+test_expect_success '--is-ancestor self' '\n+\tgit -C is-ancestor merge-base --is-ancestor two two\n+'\n+\n+test_expect_success '--is-ancestor diverged commits' '\n+\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two\n+'\n+\n+test_expect_success '--is-ancestor exit 128 non-existent commit' '\n+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit &&\n+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one\n+'\n+\n+test_expect_success '--is-ancestor and --all cannot be used together' '\n+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err &&\n+\ttest_grep \"options .--is-ancestor. and .--all. cannot be used together\" err\n+'\n+\n test_done\n\nbase-commit: f60db8d575adb79761d363e026fb49bddf330c73\n-- \ngitgitgadget\n"},{"id":"549177","messageId":"xmqq1pcmd93w.fsf@gitster.g","threadId":"66065","inReplyTo":"pull.2186.git.1784998828879.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-base: add tests for --is-ancestor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-28T22:48:03Z","receivedAt":"2026-07-28T22:48:06Z","isPatch":true,"body":"\"Nikolaus Schuetz via GitGitGadget\" writes:\n\n> From: Nikolaus Schuetz \n>\n> `git merge-base --is-ancestor A B` is used a lot in scripts but has no\n> tests. Add some to t6010 covering its exit codes: 0 when A is an\n> ancestor of B, 1 when it is not, and 128 (not 1) when given a bad\n> argument. Also check that --is-ancestor and --all can't be combined,\n> and that the resulting error names both options.\n>\n> Signed-off-by: Nikolaus Schuetz \n> ---\n> merge-base: add tests for --is-ancestor\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/2186\n>\n> t/t6010-merge-base.sh | 34 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 34 insertions(+)\n\nThanks, will queue.\n\n>\n> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n> index 44c726ea39..d28d9dab2c 100755\n> --- a/t/t6010-merge-base.sh\n> +++ b/t/t6010-merge-base.sh\n> @@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'setup --is-ancestor' '\n> +\tgit init is-ancestor &&\n> +\t(\n> +\t\tcd is-ancestor &&\n> +\t\ttest_commit one &&\n> +\t\ttest_commit two &&\n> +\t\tgit checkout -b side one &&\n> +\t\ttest_commit three\n> +\t)\n> +'\n> +\n> +test_expect_success '--is-ancestor parent and child' '\n> +\tgit -C is-ancestor merge-base --is-ancestor one two &&\n> +\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one\n> +'\n> +\n> +test_expect_success '--is-ancestor self' '\n> +\tgit -C is-ancestor merge-base --is-ancestor two two\n> +'\n> +\n> +test_expect_success '--is-ancestor diverged commits' '\n> +\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two\n> +'\n> +\n> +test_expect_success '--is-ancestor exit 128 non-existent commit' '\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit &&\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one\n> +'\n> +\n> +test_expect_success '--is-ancestor and --all cannot be used together' '\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err &&\n> +\ttest_grep \"options .--is-ancestor. and .--all. cannot be used together\" err\n> +'\n> +\n> test_done\n>\n> base-commit: f60db8d575adb79761d363e026fb49bddf330c73\n"},{"id":"549188","messageId":"9a47d529-6195-435b-90a6-e511856f128e@gmail.com","threadId":"66065","inReplyTo":"pull.2186.git.1784998828879.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-base: add tests for --is-ancestor","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-29T09:39:11Z","receivedAt":"2026-07-29T09:39:18Z","isPatch":true,"body":"Hi Nikolaus\n\nOn 25/07/2026 18:00, Nikolaus Schuetz via GitGitGadget wrote:\n> From: Nikolaus Schuetz \n> \n> `git merge-base --is-ancestor A B` is used a lot in scripts but has no\n> tests. Add some to t6010 covering its exit codes: 0 when A is an\n> ancestor of B, 1 when it is not, and 128 (not 1) when given a bad\n> argument. Also check that --is-ancestor and --all can't be combined,\n> and that the resulting error names both options.\n\nThanks for adding some tests for this option\n\n> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n> index 44c726ea39..d28d9dab2c 100755\n> --- a/t/t6010-merge-base.sh\n> +++ b/t/t6010-merge-base.sh\n> @@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'setup --is-ancestor' '\n> +\tgit init is-ancestor &&\n> +\t(\n> +\t\tcd is-ancestor &&\n> +\t\ttest_commit one &&\n> +\t\ttest_commit two &&\n> +\t\tgit checkout -b side one &&\n> +\t\ttest_commit three\n> +\t)\n> +'\n\nDo we really need to create a new repository? None of the existing tests \ndo that - can't we just use the commits created by the first test? \nHaving said that, the tests below look good. We could perhaps have a \ntest to check that it fails with one commit or three commits but what \nyou have here is clearly a very useful improvement in our test coverage.\n\nThanks\n\nPhillip\n\n> +test_expect_success '--is-ancestor parent and child' '\n> +\tgit -C is-ancestor merge-base --is-ancestor one two &&\n> +\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one\n> +'\n> +\n> +test_expect_success '--is-ancestor self' '\n> +\tgit -C is-ancestor merge-base --is-ancestor two two\n> +'\n> +\n> +test_expect_success '--is-ancestor diverged commits' '\n> +\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two\n> +'\n> +\n> +test_expect_success '--is-ancestor exit 128 non-existent commit' '\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit &&\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one\n> +'\n> +\n> +test_expect_success '--is-ancestor and --all cannot be used together' '\n> +\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err &&\n> +\ttest_grep \"options .--is-ancestor. and .--all. cannot be used together\" err\n> +'\n> +\n> test_done\n> \n> base-commit: f60db8d575adb79761d363e026fb49bddf330c73\n\n"},{"id":"549215","messageId":"xmqqo6fpak2k.fsf@gitster.g","threadId":"66065","inReplyTo":"9a47d529-6195-435b-90a6-e511856f128e@gmail.com","subject":"Re: [PATCH] merge-base: add tests for --is-ancestor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-29T15:31:47Z","receivedAt":"2026-07-29T15:31:50Z","isPatch":true,"body":"Phillip Wood writes:\n\n> Thanks for adding some tests for this option\n>\n>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n>> index 44c726ea39..d28d9dab2c 100755\n>> --- a/t/t6010-merge-base.sh\n>> +++ b/t/t6010-merge-base.sh\n>> @@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' '\n>> \ttest_cmp expected actual\n>> '\n>> \n>> +test_expect_success 'setup --is-ancestor' '\n>> +\tgit init is-ancestor &&\n>> +\t(\n>> +\t\tcd is-ancestor &&\n>> +\t\ttest_commit one &&\n>> +\t\ttest_commit two &&\n>> +\t\tgit checkout -b side one &&\n>> +\t\ttest_commit three\n>> +\t)\n>> +'\n>\n> Do we really need to create a new repository? None of the existing tests \n> do that - can't we just use the commits created by the first test? \n\nGood point. It is often a lot more work to do so when writing these\nnew tests, and I suspect that is the reason why this new repository\nwas created, but I do not know whether, in the long run, it is\ncheaper and more maintainable to reuse the test history that has been\nused by all existing tests above. The next developer who needs to\ntweak the first test has 5 (the number of tests below) extra\nconstraints if we go that route, but a separate setup means the two\nsets of tests are more isolated.\n\n> Having said that, the tests below look good. We could perhaps have a \n> test to check that it fails with one commit or three commits but what \n> you have here is clearly a very useful improvement in our test coverage.\n\nThanks.\n"},{"id":"549261","messageId":"pull.2186.v2.git.1785392350660.gitgitgadget@gmail.com","threadId":"66065","inReplyTo":"pull.2186.git.1784998828879.gitgitgadget@gmail.com","subject":"[PATCH v2] merge-base: add tests for --is-ancestor","fromName":"Nikolaus Schuetz via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-30T06:19:09Z","receivedAt":"2026-07-30T06:19:17Z","isPatch":true,"body":"From: Nikolaus Schuetz \n\n`git merge-base --is-ancestor A B` is used a lot in scripts but has no\ntests. Add some to t6010 covering its exit codes: 0 when A is an\nancestor of B, 1 when it is not, and 128 (not 1) when given a bad\nargument. Also check that --is-ancestor and --all can't be combined,\nand that the resulting error names both options.\n\nSigned-off-by: Nikolaus Schuetz \n---\n merge-base: add tests for --is-ancestor\n \n Changes since v1, per review:\n \n * Reuse the E---D---C---B---A history and the G/H merges from the \"set\n up G and H\" test instead of creating a separate repository.\n * Add a test that --is-ancestor requires exactly two commits: too few\n is a usage error (exit 129), while more than two is rejected with\n \"--is-ancestor takes exactly two commits\" (exit 128).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2186\n\nRange-diff vs v1:\n\n 1: e512e35788 ! 1: 319bf89d25 merge-base: add tests for --is-ancestor\n @@ t/t6010-merge-base.sh: test_expect_success 'merge-base --octopus --all for compl\n \ttest_cmp expected actual\n '\n \n -+test_expect_success 'setup --is-ancestor' '\n -+\tgit init is-ancestor &&\n -+\t(\n -+\t\tcd is-ancestor &&\n -+\t\ttest_commit one &&\n -+\t\ttest_commit two &&\n -+\t\tgit checkout -b side one &&\n -+\t\ttest_commit three\n -+\t)\n ++test_expect_success '--is-ancestor with an ancestor and a descendant' '\n ++\tgit merge-base --is-ancestor $E $A &&\n ++\ttest_expect_code 1 git merge-base --is-ancestor $A $E\n +'\n +\n -+test_expect_success '--is-ancestor parent and child' '\n -+\tgit -C is-ancestor merge-base --is-ancestor one two &&\n -+\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one\n ++test_expect_success '--is-ancestor treats a commit as its own ancestor' '\n ++\tgit merge-base --is-ancestor $A $A\n +'\n +\n -+test_expect_success '--is-ancestor self' '\n -+\tgit -C is-ancestor merge-base --is-ancestor two two\n ++test_expect_success '--is-ancestor with diverged commits' '\n ++\ttest_expect_code 1 git merge-base --is-ancestor $G $H &&\n ++\ttest_expect_code 1 git merge-base --is-ancestor $H $G\n +'\n +\n -+test_expect_success '--is-ancestor diverged commits' '\n -+\ttest_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two\n ++test_expect_success '--is-ancestor exits 128 on a bad commit' '\n ++\ttest_expect_code 128 git merge-base --is-ancestor $A no-such-commit &&\n ++\ttest_expect_code 128 git merge-base --is-ancestor no-such-commit $A\n +'\n +\n -+test_expect_success '--is-ancestor exit 128 non-existent commit' '\n -+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit &&\n -+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one\n ++test_expect_success '--is-ancestor requires exactly two commits' '\n ++\ttest_expect_code 129 git merge-base --is-ancestor &&\n ++\ttest_expect_code 129 git merge-base --is-ancestor $A &&\n ++\ttest_expect_code 128 git merge-base --is-ancestor $E $A $B 2>err &&\n ++\ttest_grep \".--is-ancestor takes exactly two commits\" err\n +'\n +\n +test_expect_success '--is-ancestor and --all cannot be used together' '\n -+\ttest_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err &&\n ++\ttest_expect_code 128 git merge-base --is-ancestor --all $E $A 2>err &&\n +\ttest_grep \"options .--is-ancestor. and .--all. cannot be used together\" err\n +'\n +\n\n\n t/t6010-merge-base.sh | 31 +++++++++++++++++++++++++++++++\n 1 file changed, 31 insertions(+)\n\ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex 44c726ea39..21f09a678f 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -305,4 +305,35 @@ test_expect_success 'merge-base --octopus --all for complex tree' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success '--is-ancestor with an ancestor and a descendant' '\n+\tgit merge-base --is-ancestor $E $A &&\n+\ttest_expect_code 1 git merge-base --is-ancestor $A $E\n+'\n+\n+test_expect_success '--is-ancestor treats a commit as its own ancestor' '\n+\tgit merge-base --is-ancestor $A $A\n+'\n+\n+test_expect_success '--is-ancestor with diverged commits' '\n+\ttest_expect_code 1 git merge-base --is-ancestor $G $H &&\n+\ttest_expect_code 1 git merge-base --is-ancestor $H $G\n+'\n+\n+test_expect_success '--is-ancestor exits 128 on a bad commit' '\n+\ttest_expect_code 128 git merge-base --is-ancestor $A no-such-commit &&\n+\ttest_expect_code 128 git merge-base --is-ancestor no-such-commit $A\n+'\n+\n+test_expect_success '--is-ancestor requires exactly two commits' '\n+\ttest_expect_code 129 git merge-base --is-ancestor &&\n+\ttest_expect_code 129 git merge-base --is-ancestor $A &&\n+\ttest_expect_code 128 git merge-base --is-ancestor $E $A $B 2>err &&\n+\ttest_grep \".--is-ancestor takes exactly two commits\" err\n+'\n+\n+test_expect_success '--is-ancestor and --all cannot be used together' '\n+\ttest_expect_code 128 git merge-base --is-ancestor --all $E $A 2>err &&\n+\ttest_grep \"options .--is-ancestor. and .--all. cannot be used together\" err\n+'\n+\n test_done\n\nbase-commit: f60db8d575adb79761d363e026fb49bddf330c73\n-- \ngitgitgadget\n"},{"id":"549309","messageId":"xmqq8q6slb2t.fsf@gitster.g","threadId":"66065","inReplyTo":"pull.2186.v2.git.1785392350660.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] merge-base: add tests for --is-ancestor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T16:02:50Z","receivedAt":"2026-07-30T16:02:53Z","isPatch":true,"body":"\"Nikolaus Schuetz via GitGitGadget\" writes:\n\n> Changes since v1, per review:\n> \n> * Reuse the E---D---C---B---A history and the G/H merges from the \"set\n> up G and H\" test instead of creating a separate repository.\n\nLooks good.\n\n> * Add a test that --is-ancestor requires exactly two commits: too few\n> is a usage error (exit 129), while more than two is rejected with\n> \"--is-ancestor takes exactly two commits\" (exit 128).\n\nOK.\n\nWill queue. Thanks.\n"}]}