Skip to content

Commit 6772e5c

Browse files
karthiknadigCopilot
andcommitted
test: stabilize interpreter subprocess fixtures (Fixes #557)
Use a stable shell with inline commands instead of executing freshly written files that can fail with ETXTBSY under concurrent tests. Preserve typed runner diagnostics and strengthen outcome assertions without changing production fallback behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3bd7eea commit 6772e5c

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)