Re: [PATCH v2 06/13] t5538: add tests to confirm deltas in shallow pushes

[Taylor Blau <me@ttaylorr.com>]

On Mon, Mar 24, 2025 at 03:22:42PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> It can be notoriously difficult to detect if delta bases are being
> computed properly during 'git push'. Construct an example where it will
> make a kilobyte worth of difference when a delta base is not found. We
> can then use the progress indicators to distinguish between bytes and
> KiB depending on whether the delta base is found and used.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  t/t5538-push-shallow.sh | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
> index e91fcc173e8..11b85cca9e8 100755
> --- a/t/t5538-push-shallow.sh
> +++ b/t/t5538-push-shallow.sh
> @@ -123,4 +123,38 @@ EOF
>  	git cat-file blob $(echo 1|git hash-object --stdin) >/dev/null
>  	)
>  '
> +
> +test_expect_success 'push new commit from shallow clone has correct object count' '
> +	git init origin &&
> +	test_commit -C origin a &&
> +	test_commit -C origin b &&
> +
> +	git clone --depth=1 "file://$(pwd)/origin" client &&
> +	git -C client checkout -b topic &&
> +	git -C client commit --allow-empty -m "empty" &&
> +	GIT_PROGRESS_DELAY=0 git -C client push --progress origin topic 2>err &&
> +	test_grep "Enumerating objects: 1, done." err

Why is a full 'git push' necessary here? Could you instead directly
invoke pack-objects as git push / send-pack does? That test_grep to
assert on the size of the pack seems very fragile to me.

> +
> +test_expect_success 'push new commit from shallow clone has good deltas' '
> +	git init base &&
> +	test_seq 1 999 >base/a &&
> +	test_commit -C base initial &&
> +	git -C base add a &&
> +	git -C base commit -m "big a" &&

I don't think it really matters, but you may want to write a test_tick
here before committing.

> +
> +	git clone --depth=1 "file://$(pwd)/base" deltas &&
> +	git -C deltas checkout -b deltas &&
> +	test_seq 1 1000 >deltas/a &&
> +	git -C deltas commit -a -m "bigger a" &&
> +	GIT_TRACE2_PERF="$(pwd)/trace.txt" \
> +	GIT_PROGRESS_DELAY=0 git -C deltas push --progress origin deltas 2>err &&

Same note here.

> +
> +	test_grep "Enumerating objects: 5, done" err &&
> +
> +	# If the delta base is found, then this message uses "bytes".
> +	# If the delta base is not found, then this message uses "KiB".
> +	test_grep "Writing objects: .* bytes" err

If we had the raw pack that was generated, could you use verify-pack
instead to check that the desired delta/base relationship was
discovered?

Thanks,
Taylor