git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] t4053: avoid race when killing background processes

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 10, 2023, 14:33 UTC
Message-ID
<pull.1571.git.1691677993195.gitgitgadget@gmail.com>
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: Junio C Hamano
Message 1 of 5 in “t4053: avoid race when killing background processes”
  1. t4053: avoid race when killing background processesPhillip Wood via GitGitGadget, Aug 10, 2023
  2. Junio C HamanoAug 10, 2023
  3. Phillip WoodAug 11, 2023
  4. Jeff KingAug 11, 2023
  5. Junio C HamanoAug 11, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.