Re: [PATCH] test-lib-functions: fix test_subcommand_inexact

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 23, 2022, 21:45 UTC
Message-ID: <YjuVAgjaiqrcT7P1@nand.local>
In-Reply-To: <e5331972-512a-c498-6a1b-927f21ef9de2@github.com>

On Wed, Mar 23, 2022 at 10:55:37AM -0400, Derrick Stolee wrote:

Show 5 quoted lines

> > So, perhaps #3 ;-)?
>
> I'll default to #3 (do nothing), but if this shows up again
> I'll plan on adding a comment to the helper to be clear on
> how "inexact" the helper really is.

I wonder if we could sidestep the whole issue with test_subcommand_inexact by testing this behavior by looking at the contents of the packs themselves.

If we have a kept pack, and then add some new objects, and run "git repack --write-midx -adb", the new pack should not contain any of the objects found in the old (kept) pack. And that's the case after this patch, but was broken before it.

Here's a test which constructs that scenario and then asserts that there isn't any overlap between the newly created pack and the old, kept pack.

--- 8< ---

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 5922fb5bdd..96812fa226 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -370,9 +370,31 @@ test_expect_success '--write-midx with preferred bitmap tips' '
 '

 test_expect_success '--write-midx -b packs non-kept objects' '
-	GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
-		git repack --write-midx -a -b &&
-	test_subcommand_inexact git pack-objects --honor-pack-keep <trace.txt
+	git init repo &&
+	test_when_finished "rm -fr repo" &&
+	(
+		cd repo &&
+
+		test_commit base &&
+		git repack -ad &&
+
+		find $objdir/pack -name "*.idx" >before &&
+		>$objdir/pack/$(basename $(cat before) .idx).keep &&
+
+		test_commit other &&
+		git repack --write-midx -a -b -d &&
+
+		find $objdir/pack -name "*.idx" | sort >after &&
+
+		git show-index <$(cat before) >old.raw &&
+		git show-index <$(comm -13 before after) >new.raw &&
+
+		cut -d" " -f2 old.raw | sort >old.objects &&
+		cut -d" " -f2 new.raw | sort >new.objects &&
+
+		comm -12 old.objects new.objects >shared.objects &&
+		test_must_be_empty shared.objects
+	)
 '

 test_expect_success TTY '--quiet disables progress' '

--- >8 ---

It does seem a little word-y to me, but I think you could clean it up a
little bit, too. If you want to take that patch, I think we could
reasonably use the above diff as a first patch, and then remove the
declaration of test_subcommand_inexact in a second patch.

(Feel free to forge my s-o-b if you do want to pick that up).

Thanks,
Taylor