Subject: Re: [PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err}

From: Junio C Hamano <[email-masked]>
Date: Thu, 17 Jun 2021 09:18:42 +0900

Đoàn Trần Công Danh  <[email-masked]> writes:

>> By the way, my opinion would be quite different if example like this
>> one ...
>> 
>> >  test_expect_success 'tag --no-contains <existent_tag>' '
>> > -	git tag --no-contains "v1.0" >actual 2>actual.err  &&
>> > -	test_line_count = 0 actual &&
>> > -	test_line_count = 0 actual.err
>> > +	test_line_count_cmd --out = 0 --err = 0 git tag --no-contains v1.0
>> >  '
>> 
>> ... were the majority, but I do not think that is the case.  Most
>> tests that employ the new test_line_count_cmd in this patch still
>> create either actual or actual.err in the working tree anyway, so I
>> do not see much point in adding this new helper---it is hard to
>> explain to new test writers when to use it.
>
> I'm not sure if I get your opinion.  Did you mean you wouldn't take
> whole helper? Or you meant you still wanted to see a new helper for
> checking only stdout?  If it's the former, I'll send a different
> series to only clean "git ls-files ... | wc -l" in t6400 and t6402,
> if it's the latter, I'll rewrite without --err.

I did not see much point in adding test_line_count_cmd with --out
and/or --err options; the upside of having it was dubious after
looking at the users of it in the patch that we are discussing.

I did not consider test_line_count_cmd that only works on the
standard output stream.  From the patch under discussion, it is not
immediately obvious how much such a simplified helper would help
clean up the existing tests, so I have no opinion without seeing
at least some sample uses.