Re: [PATCH v7 04/10] t6099, t6600: add side-exhaustion regression tests

On Thu, Aug 6, 2026 at 4:04 AM Kristofer Karlsson via GitGitGadget
<[email protected]> wrote:
>
> From: Kristofer Karlsson <[email protected]>
>
> Add t6099 to test the case where multiple merge-base candidates exist
> and one is an ancestor of another. This exercises the side-exhaustion
> optimization in paint_down_to_common together with the
> remove_redundant safety net in get_merge_bases_many_0.
>
> Add a mixed finite/INFINITY test to t6600 where one tip is outside
> the commit-graph (INFINITY generation) and the other is inside.
> This exercises the region transition: the walk starts in the
> INFINITY region where side-exhaustion is disabled, then crosses
> into the finite region where it can fire.

Junio already commented on the second paragraph not following your
earlier split.

> Signed-off-by: Kristofer Karlsson <[email protected]>
> ---
>  t/meson.build                         |  1 +
>  t/t6099-merge-base-side-exhaustion.sh | 82 +++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>  create mode 100755 t/t6099-merge-base-side-exhaustion.sh
>
> diff --git a/t/meson.build b/t/meson.build
> index a25f37d2f5..655c94f860 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -795,6 +795,7 @@ integration_tests = [
>    't6041-bisect-submodule.sh',
>    't6050-replace.sh',
>    't6060-merge-index.sh',
> +  't6099-merge-base-side-exhaustion.sh',
>    't6100-rev-list-in-order.sh',
>    't6101-rev-parse-parents.sh',
>    't6102-rev-list-unexpected-objects.sh',
> diff --git a/t/t6099-merge-base-side-exhaustion.sh b/t/t6099-merge-base-side-exhaustion.sh
> new file mode 100755
> index 0000000000..4f1e0d50ef
> --- /dev/null
> +++ b/t/t6099-merge-base-side-exhaustion.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description='merge-base with ancestor among merge-base candidates
> +
> +Test that merge-base --all correctly handles cases where
> +multiple merge-base candidates exist and one is an ancestor
> +of another. The side-exhaustion optimization in
> +paint_down_to_common may exit before STALE propagation
> +removes the ancestor, but remove_redundant catches it.
> +
> +Graph shape (parents are below children):
> +
> +   A ----------- X
> +   |\           /|
> +   | B---------/ |
> +   | |           |
> +   e2 \         f2
> +   |   |         |
> +   e1 d1        f1
> +    \  |        /
> +     \ |       /
> +      \|      /
> +       C
> +
> +A and X are the two tips.
> +B and C are both reachable from A and X.
> +B reaches C through d1.
> +Only B should appear in merge-base --all output.

Was this graph created in an editor using a variable width font?  In a
fixed width font, it makes one assume that C is not an ancestor of X,
but instead that C and f1 will likely eventually converge on common
history.  One might need to know what your original variable width
font was in order to see it right.  The description below if very
helpful, but could we replace the graph with:

   A ----- X
   |\     /|
   | B---/ |
   |  \    |
   e2  \   f2
   |   |   |
   e1  d1  f1
    \  |  /
     \ | /
      \|/
       C

?

> +'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ancestor merge-base candidate' '
> +       test_commit C &&
> +
> +       git checkout -b d-chain HEAD &&
> +       test_commit d1 &&
> +       test_commit B &&
> +
> +       git checkout -b e-path C &&
> +       test_commit e1 &&
> +       test_commit e2 &&
> +
> +       git checkout -b f-path C &&
> +       test_commit f1 &&
> +       test_commit f2 &&
> +
> +       git checkout -b branch-A e-path &&
> +       test_merge A B &&
> +
> +       git checkout -b branch-X f-path &&
> +       test_merge X B &&
> +
> +       git commit-graph write --reachable
> +'
> +
> +test_expect_success 'merge-base --all excludes ancestor candidate' '
> +       git rev-parse B >expected &&
> +       git merge-base --all A X >actual &&
> +       test_cmp expected actual
> +'
> +
> +test_expect_success 'merge-base (single) finds shallowest' '
> +       git rev-parse B >expected &&
> +       git merge-base A X >actual &&
> +       test_cmp expected actual
> +'
> +
> +# Without commit-graph: generation numbers are INFINITY,
> +# side-exhaustion optimization does not fire.

This comment made me think the test would run with trace2 regions
printing in order to verify which codepath it went through.  That
might make it a stronger test (if coupled with trace2 regions above to
ensure the above code is testing something different than what below
does), but certainly not worth a re-roll.

> +test_expect_success 'merge-base --all without commit-graph' '
> +       rm -f .git/objects/info/commit-graph &&
> +       git rev-parse B >expected &&
> +       git merge-base --all A X >actual &&
> +       test_cmp expected actual
> +'
> +
> +test_done

I like the tests added here; look good to me.