Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:

>> -		test 2 = $(git ls-files -s | wc -l) &&
>> -		test 2 = $(git ls-files -u | wc -l) &&
>> -		test 2 = $(git ls-files -o | wc -l) &&
>
> Here 'git ls-files -o' should have listed two untracked files ...
>
>> +		git ls-files -s >out &&
>> +		test_line_count = 2 out &&
>> +		git ls-files -u >out &&
>> +		test_line_count = 2 out &&
>> +		git ls-files -o >out &&
>> +		test_line_count = 3 out &&
>
> ... but now you expect it to list three.  I was about to point out the
> typo, but then noticed that you expect it to list one more untracked
> file than before in all subsequent tests...  now that can't be just a
> typo, can it?
>
> Please mention in the commit message that when using an intermediate
> file to store the output, 'git ls-files -o' will list that file, too,
> that's why the number of expected untracked files had to be adjusted;
> so future readers won't have to figure this out themselves.

I'd expect that a reader of the commit who cares enough to bother to
wonder by looking at the patch and seeing that 2 became 3 would know
why already.  And a reader of the resulting file would not know that
the 3 used to be 2, and won't be helped by "we used to count to 2,
now we have 'out' also counted" that much, especially in the commit
log message.  What would help the latter would be to name which
three paths we expect to see in the comment (or test against the
exact list of paths, instead of using test_line_count).

> An alternative to consider would be to add a .gitignore file in the
> initial commit to ignore 'out', then the number of untracked files
> don't have to be adjusted.

I think that is a preferred solution that we've used in ls-files and
status tests successfully.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux