Re:Re: [PATCH v7 1/3] bundle-uri: verify oid before writing refs

>"Xing Xin via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> @@ -33,6 +46,16 @@ test_expect_success 'clone with path bundle' '
>>  	test_cmp expect actual
>>  '
>>  
>> +test_expect_success 'clone with bundle that has bad header' '
>> +	# Write bundle ref fails, but clone can still proceed.
>> +	git clone --bundle-uri="clone-from/bad-header.bundle" \
>> +		clone-from clone-bad-header 2>err &&
>> +	commit_b=$(git -C clone-from rev-parse B) &&
>> +	test_grep "trying to write ref '\''refs/bundles/topic'\'' with nonexistent object $commit_b" err &&
>> +	git -C clone-bad-header for-each-ref --format="%(refname)" >refs &&
>> +	! grep "refs/bundles/" refs
>
>Why not "test_grep !" here?  There are other uses of bare grep in
>the newly added lines, but I won't repeat them here; the same
>comment applies to them.

>> +	test_write_lines refs/bundles/topic >expect &&
>> +	test_cmp expect actual &&
>> +	# Ensure that refs/bundles/topic are sent as "have".
>> +	test_grep "clone> have $(git -C clone-from rev-parse A)" trace-packet.txt
>> +'
>
>Can this rev-parse fail (the failure would be hidden from the test)?
>If so,
>
>	...
>	test_cmp expect actual &&
>	# Ensure that refs/bundles/topic is sent as "have"
>	tip=$(git -C clone-from rev-parse A) &&
>	test_grep "clone> have $tip" trace-packet.txt
>
>would catch such a failure.  You are doing so in the previous test
>in the hunk starting at 33/46 above with commit_b variable already.
>
>There are other uses of git command in $(command substitution) whose
>exit status are ignored in the newly added lines, but I won't
>repeat them here; the same comment applies to them.