From: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
Phillip Wood <phillip.wood@dunelm.org.uk>,
Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH] t4053: avoid race when killing background processes
Date: Thu, 10 Aug 2023 14:33:13 +0000 [thread overview]
Message-ID: <pull.1571.git.1691677993195.gitgitgadget@gmail.com> (raw)
From: Phillip Wood <phillip.wood@dunelm.org.uk>
The test 'diff --no-index reads from pipes' starts a couple of
background processes that write to the pipes that are passed to "diff
--no-index". If the test passes then we expect these processes to exit
as all their output will have been read. However if the test fails
then we want to make sure they do not hang about on the users machine
and the test remembers they should be killed by calling
test_when_finished "! kill $!"
after each background process is created. Unfortunately there is a
race where test_when_finished may run before the background process
exits even when all its output has been read resulting in the kill
command succeeding which causes the test to fail. Fix this by ignoring
the exit status of the kill command. If the diff is successful we
could instead wait for the background process to exit and check their
status but that feels like it is testing the platform's printf
implementation rather than git's code.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t4053: avoid race when killing background processes
Thanks to Peff for reporting this. Junio - this fixes a regression
introduced in the current cycle. It is fairly minor though so I'm not
sure if you want to pick it up now or wait until 2.42.0 is out.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1571%2Fphillipwood%2Fdiff-no-index-pipes-fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1571/phillipwood/diff-no-index-pipes-fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1571
t/t4053-diff-no-index.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index a28b9ff2434..1fb7d334620 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -248,11 +248,11 @@ test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' '
{
(test_write_lines a b c >old) &
} &&
- test_when_finished "! kill $!" &&
+ test_when_finished "kill $! || :" &&
{
(test_write_lines a x c >new) &
} &&
- test_when_finished "! kill $!" &&
+ test_when_finished "kill $! || :" &&
cat >expect <<-EOF &&
diff --git a/old b/new-link
base-commit: a82fb66fed250e16d3010c75404503bea3f0ab61
--
gitgitgadget
next reply other threads:[~2023-08-10 14:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-10 14:33 Phillip Wood via GitGitGadget [this message]
2023-08-10 17:40 ` [PATCH] t4053: avoid race when killing background processes Junio C Hamano
2023-08-11 9:56 ` Phillip Wood
2023-08-11 14:44 ` Jeff King
2023-08-11 15:47 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1571.git.1691677993195.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).