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

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

On Thu, Apr 22 2021, Đoàn Trần Công Danh wrote:

> On 2021-04-21 16:32:40-0700, Junio C Hamano <[email-masked]> wrote:
>> Eric Sunshine <[email-masked]> writes:
>> 
>> > On Wed, Apr 21, 2021 at 12:55 PM Eric Sunshine <[email-masked]> wrote:
>> >>     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.
>> >
>> > The frequency with which this idiom crops up with commands beyond
>> > git-ls-files suggests the more general solution of supporting it
>> > directly in test-lib-functions.sh for any command. For instance:
>> >
>> >     test_cmd_line_count = 3 git ls-files -u &&
>> >
>> > Or, perhaps, a new mode of test_line_count():
>> >
>> >     test_line_count = 3 -c git ls-files -u &&
>
> Hm, I'm not sure how would we implement such mode:
>
> * Will we run such command in a subprocess and and pipe to "wc -l"
>   directly to avoid a temporary file, but we will lose the exit code
>   of running command in process?

Just use a tempfile.

> * Will we run in a separated chain? Thus, a temporary file would be
>   created, skimming over test-lib-functions.sh, I couldn't find any
>   convention for creating such files, except for specific use cases,
>   let's say "*.config".

Yeah we happen not to have one of those in test-lib-functions.sh, but a
bunch of helpers create those.

> * Another clever hacks that I don't know *shrug*

>> 
>> That looks nice on paper, but may be going too far.
>> 
>> We may want to count the lines in the error message,
>
> Let's assume that we solve above puzzle.
>
> Count the lines in the error messages is not too hard to be imagined,
> let's say by -c2 or something like that.
>
>> or we may want
>> to count the lines after filtering the output with pipe.
>
> However, when it involved a pipe, things becomes complicated.

Yes, there's no portable way to both stream stdout/stderr in the correct
order and to intercept it. You'd need a mkfifo. See also how the "--tee"
mode works (i.e. they're squashed).

>> A test file that is dedicated to test ls-files with a file local
>> helper "count_ls_files" smells like a better place to stop, at least
>> to me.
>
> Hence, I'll stick with local help "count_ls_files" for now.

Here's an implementation that works, sans the fancy opts parsing etc. I
just use -1 for "don't care". You can see "test_commit" etc. for how to
do that.

It doesn't emit output, but that's just a matter of re-cat-ing the
relevant files.

Yes we wouldn't have combined output when you do that, but I don't think
we care, you can chain this thing, and most thing that would do so would
only care about piping stdout/stderr to their own files anyway, and not
that we emit one line of stdout, then another of stderr, another of
stdout etc.

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d54..c7251102a3d 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -154,14 +154,10 @@ test_expect_success 'pull conflicting renames' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . blue &&
-	git ls-files -u A >a.stages &&
-	test_line_count = 1 a.stages &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 1 b.stages &&
-	git ls-files -u C >c.stages &&
-	test_line_count = 1 c.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_command = 1 0 git ls-files -u A &&
+	test_line_count_command = 1 0 git ls-files -u B &&
+	test_line_count_command = 1 0 git ls-files -u C &&
+	test_line_count_command = 1 0 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b823c140271..10e3cbe0d47 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -817,6 +817,24 @@ test_line_count () {
 	fi
 }
 
+test_line_count_command () {
+	local op=$1
+	local ocnt=$2
+	local ecnt=$3
+	shift 3
+	"$@" 2>cnt.err >cnt.out
+	local exit_code=$?
+	if test "$ocnt" -gt -1
+	then
+		test_line_count "$op" "$ocnt" cnt.out
+	fi &&
+	if test "$ecnt" -gt -1
+	then
+		test_line_count "$op" "$ecnt" cnt.err
+	fi &&
+	return "$exit_code"
+}
+
 test_file_size () {
 	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"