t7006: clean up SIGPIPE handling in trace2 tests

Comit c24b7f6736 (pager: test for exit code with and without SIGPIPE, 2021-02-02) introduced some tests that don't reliably generate SIGPIPE where we expect it (i.e., when our pager doesn't read all of the output from git-log). There are two problems that somewhat cancel each other out. First is that the output of git-log isn't very large (only around 800 bytes). So even if the pager doesn't read all of our output, it's racy whether or not we'll actually get a SIGPIPE (we won't if we write all of the output into the pipe buffer before the pager exits). But we wrap git-log with test_terminal, which is supposed to propagate the exit status of git-log. However, it doesn't always do so; test_terminal will copy to stdout any lines that it got from our fake pager, and it pipes to an empty command. So most of the time we are seeing a SIGPIPE from test_terminal itself (though this is likewise racy). Let's try to make this more robust in two ways: 1. We'll put a commit with a huge message at the tip of history. Since this is over a megabyte, it should fill the OS pipe buffer completely, causing git-log to keep trying to write even after the pager has exited. 2. We'll redirect the output of test_terminal to /dev/null. That means it can never get SIGPIPE itself, and will always be giving us the exit code from git-log. These two changes reveal that one of the tests was looking for the wrong behavior. If we try to start a pager that does not exist (according to execve()), then the error propagates from start_command() back to the pager code as an error, and we avoid redirecting git-log's stdout to the broken pager entirely. Instead, it goes straight to the original stdout (test_terminal's pty in this case), and we do not see a SIGPIPE at all. So the test "git attempts to page to nonexisting pager command, gets SIGPIPE" is checking the wrong outcome; it should be looking for a successful exit (and was only confused by test_terminal's SIGPIPE). There's a related test, "git discards nonexisting pager without SIGPIPE", which sets the pager to a shell command which will read all input and _then_ run a non-existing command. But that doesn't trigger the same execve() behavior. We really do run the shell there and redirect git-log's stdout to it. And the fact that the shell then exits 127 is not interesting. It is not different at that point than the earlier test to check for "exit 1". So we can drop that test entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 21, 2021 at 17:54 UTC f7991f01f2d1bc800a47adcf66a1b29a1f7cf697
1 file changed +14 -28
t/t7006-pager.sh
+14 -28
@@ -661,6 +661,13 @@ test_expect_success 'setup trace2' '
661 export GIT_TRACE2_BRIEF
662 '
663
664 +test_expect_success 'setup large log output' '
665 + perl -e "
666 + print \"this is a long commit message\" x 50000
667 + " >commit-msg &&
668 + git commit --allow-empty -F commit-msg
669 +'
670 +
671 test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
672 test_when_finished "rm pager-used trace.normal" &&
673 test_config core.pager ">pager-used; head -n 1; exit 0" &&
@@ -670,7 +677,7 @@ test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
677
678 if test_have_prereq !MINGW
679 then
673 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
680 + OUT=$( ((test_terminal git log; echo $? 1>&3) >/dev/null) 3>&1 ) &&
681 test_match_signal 13 "$OUT"
682 else
683 test_terminal git log
@@ -691,7 +698,7 @@ test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '
698
699 if test_have_prereq !MINGW
700 then
694 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
701 + OUT=$( ((test_terminal git log; echo $? 1>&3) >/dev/null) 3>&1 ) &&
702 test_match_signal 13 "$OUT"
703 else
704 test_terminal git log
@@ -712,7 +719,7 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
719
720 if test_have_prereq !MINGW
721 then
715 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
722 + OUT=$( ((test_terminal git log; echo $? 1>&3) >/dev/null) 3>&1 ) &&
723 test "$OUT" -eq 0
724 else
725 test_terminal git log
@@ -724,28 +731,7 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
731 test_path_is_file pager-used
732 '
733
727 -test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '
728 - test_when_finished "rm pager-used trace.normal" &&
729 - test_config core.pager "wc >pager-used; does-not-exist" &&
730 - GIT_TRACE2="$(pwd)/trace.normal" &&
731 - export GIT_TRACE2 &&
732 - test_when_finished "unset GIT_TRACE2" &&
733 -
734 - if test_have_prereq !MINGW
735 - then
736 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
737 - test "$OUT" -eq 0
738 - else
739 - test_terminal git log
740 - fi &&
741 -
742 - grep child_exit trace.normal >child-exits &&
743 - test_line_count = 1 child-exits &&
744 - grep " code:127 " child-exits &&
745 - test_path_is_file pager-used
746 -'
747 -
748 -test_expect_success TTY 'git attempts to page to nonexisting pager command, gets SIGPIPE' '
734 +test_expect_success TTY 'git skips paging nonexisting command' '
735 test_when_finished "rm trace.normal" &&
736 test_config core.pager "does-not-exist" &&
737 GIT_TRACE2="$(pwd)/trace.normal" &&
@@ -754,8 +740,8 @@ test_expect_success TTY 'git attempts to page to nonexisting pager command, gets
740
741 if test_have_prereq !MINGW
742 then
757 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
758 - test_match_signal 13 "$OUT"
743 + OUT=$( ((test_terminal git log; echo $? 1>&3) >/dev/null) 3>&1 ) &&
744 + test "$OUT" -eq 0
745 else
746 test_terminal git log
747 fi &&
@@ -774,7 +760,7 @@ test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
760
761 if test_have_prereq !MINGW
762 then
777 - OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
763 + OUT=$( ((test_terminal git log; echo $? 1>&3) >/dev/null) 3>&1 ) &&
764 test_match_signal 13 "$OUT"
765 else
766 test_terminal git log