Skip to contentSkip to conversation

t1010: add basic tests for mktree --batch

PATCH Superseded Roger Koranteng via GitGitGadget 2 participants 3 messages Oct 20 – Nov 10

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t1010: add basic tests for mktree --batch

From: Roger Koranteng <[email protected]>

The only "mktree --batch" test in t1010 is the one checking that it survives a concurrent repack. Nothing covers basic batch mode, which is how the leak fixed in 6272f3bd17 (mktree: plug per-tree leak in --batch mode, 2026-08-29) went unnoticed.

Add a few tests for it. Two trees in one batch should get the same IDs as running plain "mktree" on each, a trailing blank line should not add an extra tree, -z input should work, and a tree that names a missing object should make mktree die after printing the trees before it. While at it, check that a blank line is rejected without --batch.

One test feeds two blank lines in a row and expects an empty tree in between. That is what mktree does today, but the documentation says trees are separated by "a single blank line", so I am not sure it is intended. The test records the current behaviour.

---
t1010: add basic tests for mktree --batch

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2270%2Frogerkorantenng%2Fmktree-batch-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2270/rogerkorantenng/mktree-batch-tests-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2270

 t/t1010-mktree.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
t/t1010-mktree.sh +73 -0
index cecba55d45..42f72acabc 100755
@@ -117,4 +117,77 @@ test_expect_success PIPE 'mktree --batch survives a concurrent repack retiring a
 	)
 '
 
+test_expect_success 'mktree --batch writes one tree per input block' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch ignores a trailing blank line' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub &&
+		echo
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch: two blank lines give an empty tree' '
+	{
+		cat top &&
+		echo &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	{
+		cat tree &&
+		git mktree </dev/null &&
+		cat tree.withsub
+	} >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree rejects a blank line without --batch' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	test_must_fail git mktree <input 2>err &&
+	test_grep "blank line only valid in batch mode" err
+'
+
+test_expect_success 'mktree --batch -z reads NUL-terminated input' '
+	{
+		git ls-tree -z $(cat tree) &&
+		printf "\0" &&
+		git ls-tree -z $(cat tree.withsub)
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch -z <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch stops at a tree with a missing object' '
+	missing=$(test_oid deadbeef) &&
+	{
+		cat top &&
+		echo &&
+		printf "100644 blob %s\tmissing\n" "$missing" &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	test_must_fail git mktree --batch <input >actual &&
+	test_cmp tree actual
+'
+
 test_done

base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926

Signature · 1 line
gitgitgadget
Tested-by on v1 1/1
#1 lore raw
Quoted 7 lines · Roger Koranteng

The only "mktree --batch" test in t1010 is the one checking that it survives a concurrent repack. Nothing covers basic batch mode, which is how the leak fixed in 6272f3bd17 (mktree: plug per-tree leak in --batch mode, 2026-08-29) went unnoticed.

Add a few tests for it. Two trees in one batch should get the same [...]

Thanks for adding these. t1010 passes for me with the patch, also under SANITIZE=leak. With 6272f3bd17 reverted, tests 10, 11, 12 and 14 each fail when run on their own, so they do catch that leak.

Quoted 4 lines

One test feeds two blank lines in a row and expects an empty tree in between. That is what mktree does today, but the documentation says trees are separated by "a single blank line", so I am not sure it is intended. The test records the current behaviour.

I would keep this test. The question about the documentation fits better below the "---" than in the commit message.

t/t1010-mktree.sh test_expect_success PIPE 'mktree --batch survives a concurrent repack retiring a · line 190

Quoted 4 lines
t/t1010-mktree.sh +3 -0
+test_expect_success 'mktree --batch stops at a tree with a missing object' '
[...]
+	test_must_fail git mktree --batch <input >actual &&
+	test_cmp tree actual

Capturing stderr and checking for the "is unavailable" message would make sure it died for the expected reason:

	test_must_fail git mktree --batch <input >actual 2>err &&
	test_cmp tree actual &&
	test_grep "object $missing is unavailable" err
#2 lore raw

PATCH v2 1/1 t1010: add basic tests for mktree --batch

In reply to Roger Koranteng via GitGitGadget

From: Roger Koranteng <[email protected]>

The only "mktree --batch" test in t1010 is the one checking that it survives a concurrent repack. Nothing covers basic batch mode, which is how the leak fixed in 6272f3bd17 (mktree: plug per-tree leak in --batch mode, 2026-08-29) went unnoticed.

Add a few tests for it. Two trees in one batch should get the same IDs as running plain "mktree" on each, a trailing blank line should not add an extra tree, -z input should work, and a tree that names a missing object should make mktree die after printing the trees before it. While at it, check that a blank line is rejected without --batch.

---
t1010: add basic tests for mktree --batch

v2: check the error message in the missing-object test, and move the blank-line question out of the commit message.

One test feeds two blank lines in a row and expects an empty tree in between. That is what mktree does today, but the documentation says trees are separated by "a single blank line", so I am not sure it is intended. Is it, or should mktree reject it?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2270%2Frogerkorantenng%2Fmktree-batch-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2270/rogerkorantenng/mktree-batch-tests-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2270

Range-diff vs v1:

1: 7a928dccb0 ! 1: 40f09c123f t1010: add basic tests for mktree --batch
@@ Commit message
missing object should make mktree die after printing the trees before it. While at it, check that a blank line is rejected without --batch.

- One test feeds two blank lines in a row and expects an empty tree in
- between. That is what mktree does today, but the documentation says
- trees are separated by "a single blank line", so I am not sure it is
- intended. The test records the current behaviour.
-

+ Tested-by: Laveesh Bansal <[email protected]>

## t/t1010-mktree.sh ##
@@ t/t1010-mktree.sh: test_expect_success PIPE 'mktree --batch survives a concurren
+ echo &&
+ cat top.withsub
+ } >input &&
-+ test_must_fail git mktree --batch <input >actual &&
-+ test_cmp tree actual
++ test_must_fail git mktree --batch <input >actual 2>err &&
++ test_cmp tree actual &&
++ test_grep "object $missing is unavailable" err
+'
+
test_done

 t/t1010-mktree.sh | 74 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
t/t1010-mktree.sh +74 -0
index cecba55d45..bf03b62215 100755
@@ -117,4 +117,78 @@ test_expect_success PIPE 'mktree --batch survives a concurrent repack retiring a
 	)
 '
 
+test_expect_success 'mktree --batch writes one tree per input block' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch ignores a trailing blank line' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub &&
+		echo
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch: two blank lines give an empty tree' '
+	{
+		cat top &&
+		echo &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	{
+		cat tree &&
+		git mktree </dev/null &&
+		cat tree.withsub
+	} >expect &&
+	git mktree --batch <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree rejects a blank line without --batch' '
+	{
+		cat top &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	test_must_fail git mktree <input 2>err &&
+	test_grep "blank line only valid in batch mode" err
+'
+
+test_expect_success 'mktree --batch -z reads NUL-terminated input' '
+	{
+		git ls-tree -z $(cat tree) &&
+		printf "\0" &&
+		git ls-tree -z $(cat tree.withsub)
+	} >input &&
+	cat tree tree.withsub >expect &&
+	git mktree --batch -z <input >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'mktree --batch stops at a tree with a missing object' '
+	missing=$(test_oid deadbeef) &&
+	{
+		cat top &&
+		echo &&
+		printf "100644 blob %s\tmissing\n" "$missing" &&
+		echo &&
+		cat top.withsub
+	} >input &&
+	test_must_fail git mktree --batch <input >actual 2>err &&
+	test_cmp tree actual &&
+	test_grep "object $missing is unavailable" err
+'
+
 test_done

base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926

Signature · 1 line
gitgitgadget