Re: [PATCH 2/6] run-command tests: test stdout of run_command_parallel()

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Anthony Sottile <asottile@umich.edu>,
	Emily Shaffer <emilyshaffer@google.com>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH 2/6] run-command tests: test stdout of run_command_parallel()
Date: Fri, 22 Apr 2022 21:24:17 -0700	[thread overview]
Message-ID: <xmqq1qxo4eby.fsf@gitster.g> (raw)
In-Reply-To: patch-2.6-d9c9b158130-20220421T122108Z-avarab@gmail.com

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Extend the tests added in c553c72eed6 (run-command: add an
> asynchronous parallel child processor, 2015-12-15) to test stdout in
> addition to stderr. A subsequent commit will add additional related
> tests for a new feature, making it obvious how the output of the two
> compares on both stdout and stderr will make this easier to reason
> about.

OK.

The original cared only about the standard error stream, and it was
sensible to name the actual output there "actual" and the correct
output "expect" to be compared.

But now we care _both_ the standard output and the standard error
streams, so if we call one "out", wouldn't we want to call the other
one "err", I wonder?  If it makes sense, it still is OK if we are
not doing so inside this step or in the series, but then it would
make sense to leave ourselves a NEEDSWORK: note to do so later when
the tree is quiescent.

Other than that, this one is pretty much boringly OK, and boring is
good.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t0061-run-command.sh | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
> index ee281909bc3..131fcfda90f 100755
> --- a/t/t0061-run-command.sh
> +++ b/t/t0061-run-command.sh
> @@ -130,17 +130,20 @@ World
>  EOF
>  
>  test_expect_success 'run_command runs in parallel with more jobs available than tasks' '
> -	test-tool run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
> +	test-tool run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>actual &&
> +	test_must_be_empty out &&
>  	test_cmp expect actual
>  '
>  
>  test_expect_success 'run_command runs in parallel with as many jobs as tasks' '
> -	test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
> +	test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>actual &&
> +	test_must_be_empty out &&
>  	test_cmp expect actual
>  '
>  
>  test_expect_success 'run_command runs in parallel with more tasks than jobs available' '
> -	test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
> +	test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>actual &&
> +	test_must_be_empty out &&
>  	test_cmp expect actual
>  '
>  
> @@ -154,7 +157,8 @@ asking for a quick stop
>  EOF
>  
>  test_expect_success 'run_command is asked to abort gracefully' '
> -	test-tool run-command run-command-abort 3 false 2>actual &&
> +	test-tool run-command run-command-abort 3 false >out 2>actual &&
> +	test_must_be_empty out &&
>  	test_cmp expect actual
>  '
>  
> @@ -163,7 +167,8 @@ no further jobs available
>  EOF
>  
>  test_expect_success 'run_command outputs ' '
> -	test-tool run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
> +	test-tool run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>actual &&
> +	test_must_be_empty out &&
>  	test_cmp expect actual
>  '