Re: [PATCH v2 2/5] pager: test for exit code with and without SIGPIPE

[Denton Liu <liu.denton@gmail.com>]

On Tue, Feb 02, 2021 at 02:59:58AM +0100, Ævar Arnfjörð Bjarmason wrote:
> This test construct is stolen from 7559a1be8a0 (unblock and unignore
> SIGPIPE, 2014-09-18). The reason not to make the test itself depend on
> the MINGW prerequisite is to make a subsequent commit easier to read.

[...]

> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index fdb450e446a..0aa030962b1 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -656,4 +656,86 @@ test_expect_success TTY 'git tag with auto-columns ' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
> +	test_when_finished "rm pager-used" &&
> +	test_config core.pager ">pager-used; head -n 1; exit 0" &&

I may be missing something but this code seems racy, especially since
the history is relatively short at this point. It seems like it's
plausible for git log to be able to dump its output entirely before the
pager part even runs. In that case, it'd fail due to success being its
exit code since it wouldn't be killed by SIGPIPE. 

This is what my `test-tool pager` approach was hoping to prevent since
that would guarantee a SIGPIPE.

Sidenote, going back to 7559a1be8a0 (unblock and unignore SIGPIPE,
2014-09-18), it seems like those tests are also racy since it's
theoretically possible for all of the output to be produced before the
piped command gets to it. However, in that case, they're producing a
huge amount of output so this raciness seems mostly academic.

Thanks,
Denton

> +
> +	if test_have_prereq !MINGW
> +	then
> +		OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
> +		test_match_signal 13 "$OUT"
> +	else
> +		test_terminal git log
> +	fi &&
> +	test_path_is_file pager-used
> +'