Re: [PATCH GSoC v2 2/6] t5701: use the test_file_size() helper

On Sat Aug 1, 2026 at 6:27 AM CEST, Junio C Hamano wrote:
> Pablo Sabater <[email protected]> writes:
>
>> An object-info test uses 'wc -c <two.t | xargs' to get the file size.
>> Update it to use the test_file_size() helper instead.
>
> What is missing from this description is what is wrong with the use
> of that "wc -c | xargs" construct.  What benefit is this change
> supposed to gain?

True, the patch was small and I did not write the commit message
properly.

The xargs is there to strip the leading blanks that wc adds on some
OS (I tested this on macOS).
test_file_size() reports the size without any padding, so nothing needs
to be stripped.

I'll write the log correctly next reroll.

>
>> Mentored-by: Karthik Nayak <[email protected]>
>> Mentored-by: Chandra Pratap <[email protected]>
>> Signed-off-by: Pablo Sabater <[email protected]>
>> ---
>>  t/t5701-git-serve.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
>> index 9a575aa098..b4d6beef11 100755
>> --- a/t/t5701-git-serve.sh
>> +++ b/t/t5701-git-serve.sh
>> @@ -356,8 +356,8 @@ test_expect_success 'basics of object-info' '
>>
>>  	cat >expect <<-EOF &&
>>  	size
>> -	$(git rev-parse two:two.t) $(wc -c <two.t | xargs)
>> -	$(git rev-parse two:two.t) $(wc -c <two.t | xargs)
>> +	$(git rev-parse two:two.t) $(test_file_size two.t)
>> +	$(git rev-parse two:two.t) $(test_file_size two.t)
>>  	0000
>>  	EOF
>
> It is not like we want to avoid piping wc -c into xargs and hide the
> exit status from "wc -c".  We are already losing the exit status of
> "git rev-parse" anyway.
>
> If the test after the change were like this
>
> 	two_object=$(git rev-parse two:two.t) &&
> 	two_size=$(test_file_size two.t) &&
> 	cat >expect <<-EOF &&
> 	size
> 	$two_object $two_size
> 	$two_object $two_size
> 	0000
> 	EOF
>
> you can sell it as "we do not want to lose exit status of 'git
> rev-parse'", "we do not need to run the same command twice", etc.
> But it is unclear what we gain by rewriting the wc-piped-to-xargs
> to test_file_size.

I will do that, I'll move the object name and size to variables so they
are not recomputed.

Thanks for the review,
Pablo