Skip to content

Commit 094217e

Browse files
test: stabilize interpreter subprocess fixtures (Fixes #557) (#558)
Replace freshly written executable fixtures with an existing shell and inline commands, eliminating a reproduced `ETXTBSY` launch race without changing production probe behavior. - Preserve typed runner errors and assert command construction, exit status, raw bytes, 128 KiB stderr, and actual timeout outcomes. - Keep the existing deadlines and rejection semantics; add no retries or skips. - Record the fixture lesson in the existing Rust coding skill. Validation: Linux 62 and Windows 57 utility tests, workspace format/Clippy, Unix all-target Clippy, and independent review pass. Identical concurrent Linux stress failed 8/100 batches with the original fixture plus diagnostics and passed 100/100 with the fix. The original CI failure did not record its errno, so that historical detail cannot be established retrospectively. Fixes #557 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3bd7eea commit 094217e

2 files changed

Lines changed: 87 additions & 45 deletions

File tree

  • .github/skills/rust-coding-skill
  • crates/pet-python-utils/src

‎.github/skills/rust-coding-skill/SKILL.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,8 @@ assert_eq!(reads.load(Ordering::Relaxed), 1);
9393

9494
For parser helpers, include malformed input, non-ASCII surrounding data, and case variations. For diagnostics, test pattern classification and expansion filtering separately. Keep temp paths unique with `tempfile` or process/counter-based names.
9595

96-
Before every Rust commit, run targeted tests and invoke the `rust-precommit` skill. Keep that skill as the single source of truth for required format and Clippy commands.
96+
Before every Rust commit, run targeted tests and invoke the `rust-precommit` skill. Keep that skill as the single source of truth for required format and Clippy commands.
97+
98+
## Learnings
99+
100+
Do not execute freshly written scripts as concurrent Unix subprocess fixtures: spawning can fail with `ETXTBSY` (Text file busy). Prefer an existing interpreter such as `/bin/sh -c` with an inline script, or the existing test executable. Assert the typed runner outcome before checking an optional parsed result, so a spawn failure cannot masquerade as a successful negative parsing or timeout test.

‎crates/pet-python-utils/src/env.rs‎

Lines changed: 82 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use pet_core::{arch::Architecture, env::PythonEnv, python_environment::PythonEnv
66
use serde::{Deserialize, Serialize};
77
use std::{
88
path::{Path, PathBuf},
9+
process::{Command, Output},
910
time::{Duration, SystemTime},
1011
};
1112

@@ -101,12 +102,20 @@ fn get_interpreter_details(executable: &Path) -> Option<ResolvedPythonEnv> {
101102
fn get_interpreter_details_with_timeout(
102103
executable: &Path,
103104
timeout: Duration,
105+
) -> Option<ResolvedPythonEnv> {
106+
get_interpreter_details_with_runner(executable, timeout, output)
107+
}
108+
109+
fn get_interpreter_details_with_runner(
110+
executable: &Path,
111+
timeout: Duration,
112+
run: impl FnOnce(&mut Command, Duration) -> Result<Output, ProcessError>,
104113
) -> Option<ResolvedPythonEnv> {
105114
// Spawn the python exe and get the version, sys.prefix and sys.executable.
106115
let executable = executable.to_str()?;
107116
let start = SystemTime::now();
108117
trace!("Executing Python: {} -c {}", executable, PYTHON_INFO_CMD);
109-
let result = output(
118+
let result = run(
110119
new_silent_command(executable).args(["-c", PYTHON_INFO_CMD]),
111120
timeout,
112121
);
@@ -194,45 +203,80 @@ fn parse_interpreter_output(
194203
#[cfg(all(test, unix))]
195204
mod tests {
196205
use super::*;
197-
use std::{os::unix::fs::PermissionsExt, time::Instant};
206+
use std::time::Instant;
198207

199208
// https://github.com/microsoft/python-environment-tools/issues/525:
200209
// A launcher printing GBK-encoded "文件不存在" must not panic discovery.
201210
#[test]
202-
fn get_interpreter_details_handles_non_utf8_stdout() -> std::io::Result<()> {
203-
let directory = tempfile::tempdir()?;
204-
let executable = directory.path().join("python");
205-
std::fs::write(
206-
&executable,
207-
"#!/bin/sh\nprintf '\\316\\304\\274\\376\\262\\273\\264\\346\\324\\332: -c\\r\\n'\n",
208-
)?;
209-
std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755))?;
210-
let result = get_interpreter_details_with_timeout(&executable, Duration::from_secs(5));
211+
fn get_interpreter_details_handles_non_utf8_stdout() {
212+
let result = get_interpreter_details_with_runner(
213+
Path::new("/bin/sh"),
214+
Duration::from_secs(5),
215+
|command, timeout| {
216+
assert_eq!(command.get_program(), "/bin/sh");
217+
assert!(command.get_args().eq(["-c", PYTHON_INFO_CMD]));
218+
let result = output(
219+
new_silent_command("/bin/sh").args([
220+
"-c",
221+
r"printf '\316\304\274\376\262\273\264\346\324\332: -c\r\n'",
222+
]),
223+
timeout,
224+
)
225+
.expect("non-UTF-8 interpreter fixture runner must complete");
226+
assert!(result.status.success());
227+
assert_eq!(
228+
result.stdout,
229+
b"\xce\xc4\xbc\xfe\xb2\xbb\xb4\xe6\xd4\xda: -c\r\n"
230+
);
231+
Ok(result)
232+
},
233+
);
211234
assert!(result.is_none());
212-
directory.close()
213235
}
214236

215237
#[test]
216238
fn noisy_interpreter_output_resolves_only_on_success() {
217-
let directory = tempfile::tempdir().unwrap();
218-
let executable = directory.path().join("python");
219239
let payload = format!(
220240
"{}\n{}",
221241
PYTHON_INFO_JSON_SEPARATOR,
222242
r#"{"version":"3.13.1","sys_prefix":"prefix","executable":"python","is64_bit":true}"#
223243
);
224244
for exit_code in [0, 23] {
225245
let script = format!(
226-
"#!/bin/sh\nprintf '%s' '{}' >&2\nprintf '\\377\\376%s\\n' '{}'\nexit {exit_code}\n",
227-
"x".repeat(128 * 1024), payload
246+
r#"i=0
247+
while [ "$i" -lt 128 ]; do
248+
printf '%1024s' '' >&2
249+
i=$((i + 1))
250+
done
251+
printf '\377\376%s\n' '{payload}'
252+
exit {exit_code}
253+
"#
228254
);
229-
std::fs::write(&executable, script).unwrap();
230-
std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap();
231255
let started = Instant::now();
232-
let result = get_interpreter_details_with_timeout(&executable, Duration::from_secs(5));
256+
let result = get_interpreter_details_with_runner(
257+
Path::new("/bin/sh"),
258+
Duration::from_secs(5),
259+
|command, timeout| {
260+
assert_eq!(command.get_program(), "/bin/sh");
261+
assert!(command.get_args().eq(["-c", PYTHON_INFO_CMD]));
262+
let result =
263+
output(new_silent_command("/bin/sh").args(["-c", &script]), timeout)
264+
.expect("noisy interpreter fixture runner must complete");
265+
assert_eq!(result.status.code(), Some(exit_code));
266+
assert_eq!(result.stderr.len(), 128 * 1024);
267+
assert!(result.stdout.starts_with(&[0xff, 0xfe]));
268+
assert_eq!(&result.stdout[2..], format!("{payload}\n").as_bytes());
269+
Ok(result)
270+
},
271+
);
233272
assert!(started.elapsed() < Duration::from_secs(5));
234273
if exit_code == 0 {
235-
assert_eq!(result.unwrap().version, "3.13.1");
274+
assert_eq!(
275+
result
276+
.expect("successful noisy interpreter fixture must resolve")
277+
.version,
278+
"3.13.1"
279+
);
236280
} else {
237281
assert!(
238282
result.is_none(),
@@ -242,34 +286,28 @@ mod tests {
242286
}
243287
}
244288

245-
/// Regression test for #463: a spawn that never exits must not block the
246-
/// resolve path indefinitely. We use a shell script that sleeps far longer
247-
/// than the test timeout and assert that the call returns None promptly
248-
/// (well under the script's sleep duration).
289+
/// Regression test for #463: a spawn that never exits must not block resolve.
249290
#[test]
250291
fn get_interpreter_details_times_out_on_hanging_executable() {
251-
let tmp_dir = std::env::temp_dir().join(format!(
252-
"pet_resolve_timeout_{}_{}",
253-
std::process::id(),
254-
std::time::SystemTime::now()
255-
.duration_since(std::time::UNIX_EPOCH)
256-
.unwrap()
257-
.as_nanos()
258-
));
259-
std::fs::create_dir_all(&tmp_dir).unwrap();
260-
let fake_exe = tmp_dir.join("hangs");
261-
std::fs::write(&fake_exe, "#!/bin/sh\nexec sleep 60\n").unwrap();
262-
let mut perms = std::fs::metadata(&fake_exe).unwrap().permissions();
263-
perms.set_mode(0o755);
264-
std::fs::set_permissions(&fake_exe, perms).unwrap();
265-
266292
let start = Instant::now();
267-
let result = get_interpreter_details_with_timeout(&fake_exe, Duration::from_millis(200));
293+
let result = get_interpreter_details_with_runner(
294+
Path::new("/bin/sh"),
295+
Duration::from_millis(200),
296+
|command, timeout| {
297+
assert_eq!(command.get_program(), "/bin/sh");
298+
assert!(command.get_args().eq(["-c", PYTHON_INFO_CMD]));
299+
let result = output(
300+
new_silent_command("/bin/sh").args(["-c", "exec sleep 60"]),
301+
timeout,
302+
);
303+
assert!(
304+
matches!(&result, Err(ProcessError::Timeout(_))),
305+
"{result:?}"
306+
);
307+
result
308+
},
309+
);
268310
let elapsed = start.elapsed();
269-
270-
let _ = std::fs::remove_file(&fake_exe);
271-
let _ = std::fs::remove_dir(&tmp_dir);
272-
273311
assert!(result.is_none(), "hanging spawn must return None");
274312
assert!(
275313
elapsed < Duration::from_secs(3),

0 commit comments

Comments
 (0)