From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 26 Jun 2026 14:31:09 GMT
Subject: Re: [PATCH v3 4/8] commit-reach: add trace2 instrumentation to paint_down_to_common()
Message-ID: <a74d3114-7d7f-469a-b181-60853bb82864@gmail.com>
In-Reply-To: <2592264cda543c96c4479bb4ba6368c0121e4207.1782479286.git.gitgitgadget@gmail.com>

On 6/26/2026 9:08 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>

>  run_all_modes () {
> -	test_when_finished rm -rf .git/objects/info/commit-graph &&
> -	"$@" <input >actual &&
> -	test_cmp expect actual &&
> -	cp commit-graph-full .git/objects/info/commit-graph &&
> -	"$@" <input >actual &&
> -	test_cmp expect actual &&
> -	cp commit-graph-half .git/objects/info/commit-graph &&
> -	"$@" <input >actual &&
> -	test_cmp expect actual &&
> -	cp commit-graph-no-gdat .git/objects/info/commit-graph &&
> -	"$@" <input >actual &&
> -	test_cmp expect actual
> +	graph=.git/objects/info/commit-graph &&
> +	test_when_finished rm -rf "$graph" "${graph}s" &&
> +	rm -f trace-mode-*.txt &&
> +
> +	for mode in none full half no-gdat
> +	do
> +		rm -rf "$graph" "${graph}s" &&
> +		cp "commit-graph-${mode}" "$graph" 2>/dev/null ||
> +		true &&
> +		GIT_TRACE2_EVENT="$(pwd)/trace-mode-${mode}.txt" \
> +			"$@" <input >actual &&
> +		test_cmp expect actual || return 1
> +	done
>  }

Thank you for putting these traces into this helper AND for
making it cleaner at the same time!

> +test_paint_down_steps () {
> +	for mode in none full half no-gdat
> +	do
> +		test_trace2_data paint_down_to_common steps "$1" \
> +			<"trace-mode-${mode}.txt" || return 1
> +		shift
> +	done
> +}
> +
>  test_expect_success 'ref_newer:miss' '
>  	cat >input <<-\EOF &&
>  	A:commit-5-7
> @@ -209,7 +219,8 @@ test_expect_success 'in_merge_bases_many:self' '
>  	X:commit-6-8
>  	EOF
>  	echo "in_merge_bases_many(A,X):1" >expect &&
> -	test_all_modes in_merge_bases_many
> +	test_all_modes in_merge_bases_many &&
> +	test_paint_down_steps 45 2 25 3
>  '

oooh that's clean. Thanks!

Way to over-achieve here. Thanks for going the extra mile with
this patch.

Thanks,
-Stolee