]> git.ipfire.org Git - thirdparty/git.git/commit
t7422: fix flaky test caused by buffered stdout
authorPatrick Steinhardt <ps@pks.im>
Fri, 10 Jan 2025 11:31:58 +0000 (12:31 +0100)
committerJunio C Hamano <gitster@pobox.com>
Fri, 10 Jan 2025 17:15:37 +0000 (09:15 -0800)
commit65f586132bfa21c3e9fe7b2803ef526133a3b269
tree0e66498bfec376b04cb7e2389e32a58b5cc3dfc5
parentb537af720ec22c0283923afcdbbc1df306907542
t7422: fix flaky test caused by buffered stdout

One test in t7422 asserts that `git submodule status --recursive`
properly handles SIGPIPE. This test is flaky though and may sometimes
not see a SIGPIPE at all:

    expecting success of 7422.18 'git submodule status --recursive propagates SIGPIPE':
            { git submodule status --recursive 2>err; echo $?>status; } |
                    grep -q X/S &&
            test_must_be_empty err &&
            test_match_signal 13 "$(cat status)"
    ++ git submodule status --recursive
    ++ grep -q X/S
    ++ echo 0
    ++ test_must_be_empty err
    ++ test 1 -ne 1
    ++ test_path_is_file err
    ++ test 1 -ne 1
    ++ test -f err
    ++ test -s err
    +++ cat status
    ++ test_match_signal 13 0
    ++ test 0 = 141
    ++ test 0 = 269
    ++ return 1
    error: last command exited with $?=1
    not ok 18 - git submodule status --recursive propagates SIGPIPE

The issue is caused by a race between git-submodule(1) and grep(1):

  1. git-submodule(1) (or its child process) writes the first X/S line
     we're trying to match.

  2. grep(1) matches the line.

  3a. grep(1) exits, closing the pipe.

  3b. git-submodule(1) (or its child process) writes the rest of its
  lines.

Steps 3a and 3b happen at the same time without any guarantees. If 3a
happens first, we get SIGPIPE. Otherwise, we don't and the test fails.

Fix the issue by generating a couple thousand nested submodules and
matching on the first nested submodule. This ensures that the recursive
git-submodule(1) process completely fills its stdout buffer, which makes
subsequent writes block until the downstream consumer of the pipe either
reads more or closes it.

To verify that this works as expected one can apply the following patch
to the preimage of this commit, which used to reliably trigger the race:

    diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh
    index 3c5177cc30..df6001f8a0 100755
    --- a/t/t7422-submodule-output.sh
    +++ b/t/t7422-submodule-output.sh
    @@ -202,7 +202,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE'
      cd repo &&
      GIT_ALLOW_PROTOCOL=file git submodule add "$(pwd)"/../submodule &&
      { git submodule status --recursive 2>err; echo $?>status; } |
    - grep -q recursive-submodule-path-1 &&
    + { sleep 1 && grep -q recursive-submodule-path-1 && sleep 1; } &&
      test_must_be_empty err &&
      test_match_signal 13 "$(cat status)"
      )

With the pipe-stuffing workaround the test runs successfully.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/t7422-submodule-output.sh