merge-base: add tests for --is-ancestor
PATCH 1/1 merge-base: add tests for --is-ancestor
From: Nikolaus Schuetz <[email protected]>
git merge-base --is-ancestor A B is used a lot in scripts but has no tests. Add some to t6010 covering its exit codes: 0 when A is an ancestor of B, 1 when it is not, and 128 (not 1) when given a bad argument. Also check that --is-ancestor and --all can't be combined, and that the resulting error names both options.
---
merge-base: add tests for --is-ancestor
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2186
t/t6010-merge-base.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
t/t6010-merge-base.sh +34 -0
@@ -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 + ) +' + +test_expect_success '--is-ancestor parent and child' ' + git -C is-ancestor merge-base --is-ancestor one two && + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one +' + +test_expect_success '--is-ancestor self' ' + git -C is-ancestor merge-base --is-ancestor two two +' + +test_expect_success '--is-ancestor diverged commits' ' + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two +' + +test_expect_success '--is-ancestor exit 128 non-existent commit' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit && + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one +' + +test_expect_success '--is-ancestor and --all cannot be used together' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err && + test_grep "options .--is-ancestor. and .--all. cannot be used together" err +' + test_done
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
Signature · 1 line
gitgitgadget
Quoted 10 lines · Nikolaus Schuetz via GitGitGadget
From: Nikolaus Schuetz <[email protected]>
git merge-base --is-ancestor A Bis used a lot in scripts but has no tests. Add some to t6010 covering its exit codes: 0 when A is an ancestor of B, 1 when it is not, and 128 (not 1) when given a bad argument. Also check that --is-ancestor and --all can't be combined, and that the resulting error names both options.Signed-off-by Nikolaus Schuetz <[email protected]>---
Quoted 8 lines
merge-base: add tests for --is-ancestorPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2186t/t6010-merge-base.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
Thanks, will queue.
Quoted 46 lines
t/t6010-merge-base.sh +34
-0@@ -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 + ) +' + +test_expect_success '--is-ancestor parent and child' ' + git -C is-ancestor merge-base --is-ancestor one two && + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one +' + +test_expect_success '--is-ancestor self' ' + git -C is-ancestor merge-base --is-ancestor two two +' + +test_expect_success '--is-ancestor diverged commits' ' + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two +' + +test_expect_success '--is-ancestor exit 128 non-existent commit' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit && + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one +' + +test_expect_success '--is-ancestor and --all cannot be used together' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err && + test_grep "options .--is-ancestor. and .--all. cannot be used together" err +' + test_donebase-commit:
f60db8d575adb79761d363e026fb49bddf330c73
Hi Nikolaus
Quoted 7 lines · Nikolaus Schuetz via GitGitGadget
From: Nikolaus Schuetz <[email protected]>
git merge-base --is-ancestor A Bis used a lot in scripts but has no tests. Add some to t6010 covering its exit codes: 0 when A is an ancestor of B, 1 when it is not, and 128 (not 1) when given a bad argument. Also check that --is-ancestor and --all can't be combined, and that the resulting error names both options.
Thanks for adding some tests for this option
Quoted 8 lines
t/t6010-merge-base.sh +0
-0@@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' ' test_cmp expected actual '
t/t6010-merge-base.sh test_expect_success 'merge-base --octopus --all for complex tree' ' · line 317
Quoted 10 lines
t/t6010-merge-base.sh +10
-0+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? 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
Phillip
Quoted 26 lines
t/t6010-merge-base.sh +23
-0+test_expect_success '--is-ancestor parent and child' ' + git -C is-ancestor merge-base --is-ancestor one two && + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one +' + +test_expect_success '--is-ancestor self' ' + git -C is-ancestor merge-base --is-ancestor two two +' + +test_expect_success '--is-ancestor diverged commits' ' + test_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two +' + +test_expect_success '--is-ancestor exit 128 non-existent commit' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit && + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one +' + +test_expect_success '--is-ancestor and --all cannot be used together' ' + test_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err && + test_grep "options .--is-ancestor. and .--all. cannot be used together" err +' + test_donebase-commit:
f60db8d575adb79761d363e026fb49bddf330c73
Quoted 10 lines · Phillip Wood
Thanks for adding some tests for this option
Quoted 8 lines
t/t6010-merge-base.sh +0
-0@@ -305,4 +305,38 @@ test_expect_success 'merge-base --octopus --all for complex tree' ' test_cmp expected actual '
Quoted 13 lines
t/t6010-merge-base.sh test_expect_success 'merge-base --octopus --all for complex tree' ' · line 317
Quoted 10 lines
t/t6010-merge-base.sh +10
-0+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.
Quoted 3 lines
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.
PATCH v2 1/1 merge-base: add tests for --is-ancestor
In reply to Nikolaus Schuetz via GitGitGadgetFrom: Nikolaus Schuetz <[email protected]>
git merge-base --is-ancestor A B is used a lot in scripts but has no tests. Add some to t6010 covering its exit codes: 0 when A is an ancestor of B, 1 when it is not, and 128 (not 1) when given a bad argument. Also check that --is-ancestor and --all can't be combined, and that the resulting error names both options.
---
merge-base: add tests for --is-ancestor
Changes since v1, per review:
* Reuse the E---D---C---B---A history and the G/H merges from the "set up G and H" test instead of creating a separate repository.
* Add a test that --is-ancestor requires exactly two commits: too few is a usage error (exit 129), while more than two is rejected with "--is-ancestor takes exactly two commits" (exit 128).
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2186%2Fnikolauspschuetz%2Ft6010-test-is-ancestor-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2186/nikolauspschuetz/t6010-test-is-ancestor-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2186
Range-diff vs v1:
1: e512e35788 ! 1: 319bf89d25 merge-base: add tests for --is-ancestor
@@ t/t6010-merge-base.sh: test_expect_success 'merge-base --octopus --all for compl
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
-+ )
++test_expect_success '--is-ancestor with an ancestor and a descendant' '
++ git merge-base --is-ancestor $E $A &&
++ test_expect_code 1 git merge-base --is-ancestor $A $E
+'
+
-+test_expect_success '--is-ancestor parent and child' '
-+ git -C is-ancestor merge-base --is-ancestor one two &&
-+ test_expect_code 1 git -C is-ancestor merge-base --is-ancestor two one
++test_expect_success '--is-ancestor treats a commit as its own ancestor' '
++ git merge-base --is-ancestor $A $A
+'
+
-+test_expect_success '--is-ancestor self' '
-+ git -C is-ancestor merge-base --is-ancestor two two
++test_expect_success '--is-ancestor with diverged commits' '
++ test_expect_code 1 git merge-base --is-ancestor $G $H &&
++ test_expect_code 1 git merge-base --is-ancestor $H $G
+'
+
-+test_expect_success '--is-ancestor diverged commits' '
-+ test_expect_code 1 git -C is-ancestor merge-base --is-ancestor three two
++test_expect_success '--is-ancestor exits 128 on a bad commit' '
++ test_expect_code 128 git merge-base --is-ancestor $A no-such-commit &&
++ test_expect_code 128 git merge-base --is-ancestor no-such-commit $A
+'
+
-+test_expect_success '--is-ancestor exit 128 non-existent commit' '
-+ test_expect_code 128 git -C is-ancestor merge-base --is-ancestor one no-such-commit &&
-+ test_expect_code 128 git -C is-ancestor merge-base --is-ancestor no-such-commit one
++test_expect_success '--is-ancestor requires exactly two commits' '
++ test_expect_code 129 git merge-base --is-ancestor &&
++ test_expect_code 129 git merge-base --is-ancestor $A &&
++ test_expect_code 128 git merge-base --is-ancestor $E $A $B 2>err &&
++ test_grep ".--is-ancestor takes exactly two commits" err
+'
+
+test_expect_success '--is-ancestor and --all cannot be used together' '
-+ test_expect_code 128 git -C is-ancestor merge-base --is-ancestor --all one two 2>err &&
++ test_expect_code 128 git merge-base --is-ancestor --all $E $A 2>err &&
+ test_grep "options .--is-ancestor. and .--all. cannot be used together" err
+'
+t/t6010-merge-base.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
t/t6010-merge-base.sh +31 -0
@@ -305,4 +305,35 @@ test_expect_success 'merge-base --octopus --all for complex tree' ' test_cmp expected actual ' +test_expect_success '--is-ancestor with an ancestor and a descendant' ' + git merge-base --is-ancestor $E $A && + test_expect_code 1 git merge-base --is-ancestor $A $E +' + +test_expect_success '--is-ancestor treats a commit as its own ancestor' ' + git merge-base --is-ancestor $A $A +' + +test_expect_success '--is-ancestor with diverged commits' ' + test_expect_code 1 git merge-base --is-ancestor $G $H && + test_expect_code 1 git merge-base --is-ancestor $H $G +' + +test_expect_success '--is-ancestor exits 128 on a bad commit' ' + test_expect_code 128 git merge-base --is-ancestor $A no-such-commit && + test_expect_code 128 git merge-base --is-ancestor no-such-commit $A +' + +test_expect_success '--is-ancestor requires exactly two commits' ' + test_expect_code 129 git merge-base --is-ancestor && + test_expect_code 129 git merge-base --is-ancestor $A && + test_expect_code 128 git merge-base --is-ancestor $E $A $B 2>err && + test_grep ".--is-ancestor takes exactly two commits" err +' + +test_expect_success '--is-ancestor and --all cannot be used together' ' + test_expect_code 128 git merge-base --is-ancestor --all $E $A 2>err && + test_grep "options .--is-ancestor. and .--all. cannot be used together" err +' + test_done
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
Signature · 1 line
gitgitgadget
Quoted 4 lines · Nikolaus Schuetz via GitGitGadget
Changes since v1, per review:
* Reuse the E---D---C---B---A history and the G/H merges from the "set up G and H" test instead of creating a separate repository.
Looks good.
Quoted 3 lines
* Add a test that --is-ancestor requires exactly two commits: too few is a usage error (exit 129), while more than two is rejected with "--is-ancestor takes exactly two commits" (exit 128).
OK.
Will queue. Thanks.