Re: [PATCH v3 6/6] last-modified: keep per-path Bloom filters for wildcard pathspecs

Toon Claes [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Toon Claes [off-list ref] writes:

quoted

diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh
index df73c7d0d0..75b18ee83b 100755
--- a/t/t8020-last-modified.sh
+++ b/t/t8020-last-modified.sh

@@ -290,6 +290,32 @@ test_expect_success 'last-modified with Bloom filters and --show-trees' '
 	)
 '
 
+test_expect_success 'last-modified with Bloom filters and top-level wildcard' '

...
+		GIT_TRACE2_PERF="$(pwd)/off.perf" \
+			git -c core.commitGraph=false last-modified -r HEAD \
+			-- "*.c" >expect &&
+		test_grep "data .* bloom_queries:0$" off.perf &&

Ah, OK.  With GIT_TEST_COMMIT_GRAPH=1 exported from the test harness
environment, 'git -c core.commitGraph=false' would not be effective
here.  You would need to do something like:

		GIT_TEST_COMMIT_GRAPH=0 GIT_TRACE2_PERF="$(pwd)/off.perf" \
			git -c core.commitGraph=false last-modified -r HEAD \
			-- "*.c" >expect &&

quoted

+		GIT_TRACE2_PERF="$(pwd)/on.perf" \
+			git -c core.commitGraph=true last-modified -r HEAD \
+			-- "*.c" >actual &&

And in the case where GIT_TEST_COMMIT_GRAPH=0 comes from the
environment, you might want to be explicit about setting the
variable here as well.

Thanks for this suggestion. Yeah, I didn't notice it makes CI fail.

But a little bit of a followup question, I noticed I also should be
setting these in [PATCH 5/6], but test don't fail if not set
appropriately.

Yeah, I noticed it when I queued the two fixup commits near the tip
of 'seen'.  I wrote it off as the test *not* checking everything.
If the test is about what the command does and not about how the
command exactly does its thing, you may not notice the difference
as long as two code paths both produce the right results.

So some tightening of tests might be needed, if we care.