Skip to content

Commit f668ed5

Browse files
karthiknadigCopilot
andcommitted
test: integrate shutdown fixture fixes (Refs #532)
Merge the reviewed parent follow-ups, retaining the bounded readiness handshake, isolated real-pipe scenario, complete outer cleanup budget, and all native protocol cases without rewriting published history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2 parents bfbe17e + 83df603 commit f668ed5

2 files changed

Lines changed: 53 additions & 1 deletion

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,7 @@ Before every Rust commit, run targeted tests and invoke the `rust-precommit` ski
9898
## Learnings
9999

100100
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.
101+
102+
For real-pipe EOF/EPIPE tests, create the pipe inside an isolated test subprocess when other test threads spawn children. Unix `CLOEXEC` closes descriptors at exec, not fork: a concurrent child can temporarily retain a reader, allowing the only write to succeed before the final reader disappears. A readiness handshake alone does not prevent this race. Keep the operation's measured deadline separate from setup, and make an outer fixture deadline cover readiness, waits both before and after forced termination, reader joins, and fallback `Drop` cleanup.
103+
104+
Use a per-worktree Cargo target directory when validating stacked changes so native fixtures cannot execute another worktree's stale binary. On WSL, run timing-sensitive Linux binaries from the native Linux filesystem rather than a Windows mount, where page faults can stall in filesystem RPC. When launching instrumented PET with `env_clear()`, retain `LLVM_PROFILE_FILE` exactly so child coverage reaches the collector instead of an uncollected default profile.

‎crates/pet/tests/jsonrpc_server_test.rs‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -868,8 +868,56 @@ fn truncated_input_exits_unsuccessfully_without_an_error_flood() {
868868

869869
#[test]
870870
fn closed_output_exits_without_waiting_for_stdin_eof() {
871+
// Concurrent fork/exec can temporarily inherit a pipe reader despite CLOEXEC.
872+
// Isolate this scenario so its dropped handle really is the final reader.
873+
if std::env::var_os("PET_TEST_CLOSED_OUTPUT_CHILD").is_none() {
874+
let mut child = Command::new(std::env::current_exe().unwrap())
875+
.args([
876+
"--exact",
877+
"closed_output_exits_without_waiting_for_stdin_eof",
878+
"--nocapture",
879+
])
880+
.env("PET_TEST_CLOSED_OUTPUT_CHILD", "1")
881+
.stdin(Stdio::null())
882+
.stdout(Stdio::null())
883+
.stderr(Stdio::inherit())
884+
.spawn()
885+
.expect("isolated closed-output fixture must spawn");
886+
// Cover readiness, both forced-shutdown waits, reader joining, and Drop cleanup.
887+
let status = jsonrpc_client::shutdown_fixture(&mut child, Duration::from_secs(40)).unwrap();
888+
assert!(
889+
status.success(),
890+
"isolated closed-output fixture failed: {status}"
891+
);
892+
return;
893+
}
894+
871895
let mut fixture = ShutdownFixture::spawn();
872-
drop(fixture.child.stdout.take());
896+
let mut stdout = BufReader::new(fixture.child.stdout.take().unwrap());
897+
let (sender, receiver) = mpsc::sync_channel(1);
898+
let reader = thread::spawn(move || {
899+
let response = jsonrpc_client::read_message(&mut stdout);
900+
let _ = sender.send((stdout, response));
901+
});
902+
fixture.send(br#"{"jsonrpc":"2.0","id":"ready","method":"info"}"#);
903+
let (stdout, response) = match receiver.recv_timeout(Duration::from_secs(10)) {
904+
Ok(result) => result,
905+
Err(error) => {
906+
drop(receiver);
907+
fixture.child.stdin.take();
908+
let shutdown =
909+
jsonrpc_client::shutdown_fixture(&mut fixture.child, Duration::from_secs(4));
910+
let joined = jsonrpc_client::join_reader(reader, Duration::from_secs(4));
911+
panic!("server readiness failed: {error}; shutdown: {shutdown:?}; reader: {joined:?}");
912+
}
913+
};
914+
jsonrpc_client::join_reader(reader, Duration::from_secs(1)).unwrap();
915+
let response = response
916+
.unwrap()
917+
.expect("ready server must respond to info");
918+
assert_eq!(response["id"], "ready");
919+
drop(stdout);
920+
873921
let started = Instant::now();
874922
fixture.send(br#"{"jsonrpc":"2.0","id":1,"method":"info"}"#);
875923
let status = jsonrpc_client::wait_for_exit(&mut fixture.child, Duration::from_secs(1)).unwrap();

0 commit comments

Comments
 (0)