Re: [PATCH 4/4] t5326: test propagating hashcache values

On Wed, Sep 08, 2021 at 03:46:29AM +0200, Ævar Arnfjörð Bjarmason wrote:

On Tue, Sep 07 2021, Taylor Blau wrote:

quoted

Now that we both can propagate values from the hashcache, and respect
the configuration to enable the hashcache at all, test that both of
these function correctly by hardening their behavior with a test.

Signed-off-by: Taylor Blau <redacted>
---
 t/t5326-multi-pack-bitmaps.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index 4ad7c2c969..24148ca35b 100755
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh

@@ -283,4 +283,36 @@ test_expect_success 'pack.preferBitmapTips' '
 	)
 '

+test_expect_success 'hash-cache values are propagated from pack bitmaps' '
+	rm -fr repo &&
+	git init repo &&
+	test_when_finished "rm -fr repo" &&

It seems the need for this "rm -fr repo" dance instead of just relying
on test_when_finished "rm -rf repo" is because of a 3x tests in a
function in tb/multi-pack-bitmaps that should probably be combined into
one, i.e. they share the same logical "repo" setup.

Yeah. There's definitely room for clean-up there if we want to have each
of the tests operate on the same repo. I have always found sharing a
repo between tests difficult to reason about, since we have to assume
that any --run parameters could be passed in.

So I have tended to err on the side of creating and throwing away a new
repo per test, but I understand that's somewhat atypical for Git's
tests.

quoted

+	(
+		cd repo &&
+
+		git config pack.writeBitmapHashCache true &&

s/git config/test_config/, surely.

No, since this is in a subshell (and we don't care about unsetting the
value of a config in a repo that we're going to throw away, anyways).

(Side-note: since this has a /bin/sh shebang, we can't detect that we're
in a subshell and so test_config actually _would_ work here. But
switching this test to run with /bin/bash where we can detect whether or
not we're in a subshell would cause this test to fail with test_config).

Thanks,
Taylor