Subject: Re: [PATCH 2/2] t6402: check exit status of ls-files

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 22 Apr 2021 14:45:35 +0200

On Wed, Apr 21 2021, Eric Sunshine wrote:

> On Wed, Apr 21, 2021 at 6:41 AM Đoàn Trần Công Danh
> <[email-masked]> wrote:
>> We will lose the exit status of "git ls-files" if it's being run in
>> anywhere-but-not-final part of a pipe.
>>
>> Let's send the output of "git ls-files" to a file first,
>> and adjust the expected result for "git ls-files -o" since a new
>> untracked file will be created as a side effect.
>>
>> Signed-off-by: Đoàn Trần Công Danh <[email-masked]>
>> ---
>> diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
>> @@ -631,20 +663,33 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
>> -               test 5 -eq "$(git ls-files -s | wc -l)" &&
>> -               test 3 -eq "$(git ls-files -u | wc -l)" &&
>> -               test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
>> +               git ls-files -s >output &&
>> +               test_line_count = 5 output &&
>> +               git ls-files -u >output &&
>> +               test_line_count = 3 output &&
>> +               git ls-files -u one~HEAD >output &&
>> +               test_line_count = 1 output &&
>
> This idiom crops up so frequently in this test script that it almost
> begs for the introduction of a helper function rather than applying
> the manual transformation repeatedly. For instance, the helper might
> be called like this:
>
>     count_ls_files 5 -s &&
>     count_ls_files 3 -u &&
>     count_ls_files 1 -u one~HEAD &&
>     ...
>
> The nice thing about having a helper function is that it can clean up
> after itself by not leaving a new file lying around, thus you wouldn't
> have to make adjustments to the expected number of untracked files (as
> mentioned in the commit message). If this is the sort of thing which
> comes up often enough (if there are more such cases beyond the two
> scripts you changed in this series), then it might make sense to
> promote the helper function to test-lib-functions.sh.

Agreed, but I'd saythat it makes more sense to have something like:

	test_line_count = 1 -- git ls-files

Or maybe another function, but in any case not something specific to
ls-files, but just a wrapper like the various "$@" wrappers in
test-lib-functions.sh to count the lines emitted by an arbitrary
command.

It would also leave things open for:

    test_line_count --stout=1 --stderr=2 -- git ls-files
    test_line_count --combined=3 -- git ls-files

Or whatever, to count the different output streams.