Git development
 help / color / mirror / Atom feed
From: "Roger Koranteng via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Junio C Hamano <gitster@pobox.com>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	Siddharth Asthana <siddharthasthana31@gmail.com>,
	Usman Akinyemi <usmanakinyemi202@gmail.com>,
	Laveesh Bansal <laveeshb@laveeshbansal.com>,
	Roger Koranteng <rogerkorantenng@gmail.com>,
	Roger Koranteng <rogerkorantenng@gmail.com>
Subject: [PATCH v2] t1010: add basic tests for mktree --batch
Date: Tue, 10 Nov 2026 22:13:27 +0000	[thread overview]
Message-ID: <pull.2270.v2.git.1794348807240.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2270.git.1792497283164.gitgitgadget@gmail.com>

From: Roger Koranteng <rogerkorantenng@gmail.com>

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.

Suggested-by: Jeff King <peff@peff.net>
Tested-by: Laveesh Bansal <laveeshb@laveeshbansal.com>
Signed-off-by: Roger Koranteng <rogerkorantenng@gmail.com>
---
    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.
     -
          Suggested-by: Jeff King <peff@peff.net>
     +    Tested-by: Laveesh Bansal <laveeshb@laveeshbansal.com>
          Signed-off-by: Roger Koranteng <rogerkorantenng@gmail.com>
      
       ## 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(+)

diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh
index cecba55d45..bf03b62215 100755
--- a/t/t1010-mktree.sh
+++ b/t/t1010-mktree.sh
@@ -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
-- 
gitgitgadget

  parent reply	other threads:[~2026-11-10 22:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 11:54 [PATCH] t1010: add basic tests for mktree --batch Roger Koranteng via GitGitGadget
2026-11-09 23:12 ` Laveesh Bansal
2026-11-10 22:13 ` Roger Koranteng via GitGitGadget [this message]
2026-11-11 21:50   ` [PATCH v2] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2270.v2.git.1794348807240.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=laveeshb@laveeshbansal.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=rogerkorantenng@gmail.com \
    --cc=siddharthasthana31@gmail.com \
    --cc=usmanakinyemi202@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox