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

[Toon Claes <toon@iotcl.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Toon Claes <toon@iotcl.com> writes:
>
>> 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 &&
>
>> +		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.

I just sent out version 4, but to make it really waterproof, test case
'last-modified with Bloom filters and --show-trees' also should use
trace2 counters. Although I wasn't convinced it's worth it?

-- 
Laters,
Toon