From 988ee39b02a02b2b17a8f2827a92afaab3c80a2f Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Mon, 29 Sep 2025 17:01:54 +0400 Subject: [PATCH] test: stop running Test_sshConfigProxyCommandEscape in parallel (#20009) Fixes https://github.com/coder/internal/issues/1035 Or, at least, closes a remaining race that seems pretty likely. The tests in question write a file, close the file, then execute the file. Sometimes Linux errors saying "text file busy" which means the file is still open for writing. What I think is going on is: 1. Test_sshConfigProxyCommandEscape goroutine opens the file and begins writing. 2. Some other, unrelated test execs a command, which causes a `fork()` syscall. The child process now has a copy of the file descriptor to our open file. 3. Test_sshConfigProxyCommandEscape goroutine executes the file and gets "text file busy". 4. The child process calls the `exec` syscall, which closes the file (due to `CLOEXEC` being set). The race is very tight because 3 has to happen before 4 (and, 3 involves it's own fork/exec), but it's not impossible on a busy system. c.f. #14233 which was an earlier attempt to fix this. It only prevented the subtests from running in parallel. When the subtests were all running in parallel, the flake was fairly likely because you've got all this fork() activity happening at the same time. But, since the main test was in parallel there is still a chance a totally different test is `fork`'ing at in inopportune time. --- cli/configssh_internal_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cli/configssh_internal_test.go b/cli/configssh_internal_test.go index 0ddfedf077..df97527d64 100644 --- a/cli/configssh_internal_test.go +++ b/cli/configssh_internal_test.go @@ -135,11 +135,13 @@ func Test_sshConfigSplitOnCoderSection(t *testing.T) { } } -// This test tries to mimic the behavior of OpenSSH -// when executing e.g. a ProxyCommand. -// nolint:tparallel +// This test tries to mimic the behavior of OpenSSH when executing e.g. a ProxyCommand. +// nolint:paralleltest func Test_sshConfigProxyCommandEscape(t *testing.T) { - t.Parallel() + // Don't run this test, or any of its subtests in parallel. The test works by writing a file and then immediately + // executing it. Other tests might also exec a subprocess, and if they do in parallel, there is a small race + // condition where our file is open when they fork, and remains open while we attempt to execute it, causing + // a "text file busy" error. tests := []struct { name string