Skip to content

Commit 66916e2

Browse files
authored
Fix flaky test (openai#4672)
This issue was due to the fact that the timeout is not always sufficient to have enough character for truncation + a race between synthetic timeout and process kill
1 parent 26bb3fc commit 66916e2

File tree

1 file changed

+40
-30
lines changed

1 file changed

+40
-30
lines changed

codex-rs/core/tests/suite/tools.rs

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -414,37 +414,47 @@ async fn shell_timeout_includes_timeout_prefix_and_metadata() -> Result<()> {
414414
.find(|item| item.get("call_id").and_then(Value::as_str) == Some(call_id))
415415
.expect("timeout output present");
416416

417-
let output_json: Value = serde_json::from_str(
418-
timeout_item
419-
.get("output")
420-
.and_then(Value::as_str)
421-
.expect("timeout output string"),
422-
)?;
423-
assert_eq!(
424-
output_json["metadata"]["exit_code"].as_i64(),
425-
Some(124),
426-
"expected timeout exit code 124",
427-
);
417+
let output_str = timeout_item
418+
.get("output")
419+
.and_then(Value::as_str)
420+
.expect("timeout output string");
428421

429-
let stdout = output_json["output"].as_str().unwrap_or_default();
430-
assert!(
431-
stdout.starts_with("command timed out after "),
432-
"expected timeout prefix, got {stdout:?}"
433-
);
434-
let first_line = stdout.lines().next().unwrap_or_default();
435-
let duration_ms = first_line
436-
.strip_prefix("command timed out after ")
437-
.and_then(|line| line.strip_suffix(" milliseconds"))
438-
.and_then(|value| value.parse::<u64>().ok())
439-
.unwrap_or_default();
440-
assert!(
441-
duration_ms >= timeout_ms,
442-
"expected duration >= configured timeout, got {duration_ms} (timeout {timeout_ms})"
443-
);
444-
assert!(
445-
stdout.contains("[... omitted"),
446-
"expected truncated output marker, got {stdout:?}"
447-
);
422+
// The exec path can report a timeout in two ways depending on timing:
423+
// 1) Structured JSON with exit_code 124 and a timeout prefix (preferred), or
424+
// 2) A plain error string if the child is observed as killed by a signal first.
425+
if let Ok(output_json) = serde_json::from_str::<Value>(output_str) {
426+
assert_eq!(
427+
output_json["metadata"]["exit_code"].as_i64(),
428+
Some(124),
429+
"expected timeout exit code 124",
430+
);
431+
432+
let stdout = output_json["output"].as_str().unwrap_or_default();
433+
assert!(
434+
stdout.starts_with("command timed out after "),
435+
"expected timeout prefix, got {stdout:?}"
436+
);
437+
let first_line = stdout.lines().next().unwrap_or_default();
438+
let duration_ms = first_line
439+
.strip_prefix("command timed out after ")
440+
.and_then(|line| line.strip_suffix(" milliseconds"))
441+
.and_then(|value| value.parse::<u64>().ok())
442+
.unwrap_or_default();
443+
assert!(
444+
duration_ms >= timeout_ms,
445+
"expected duration >= configured timeout, got {duration_ms} (timeout {timeout_ms})"
446+
);
447+
} else {
448+
// Fallback: accept the signal classification path to deflake the test.
449+
assert!(
450+
output_str.contains("execution error"),
451+
"unexpected non-JSON output: {output_str:?}"
452+
);
453+
assert!(
454+
output_str.contains("Signal(") || output_str.to_lowercase().contains("signal"),
455+
"expected signal classification in error output, got {output_str:?}"
456+
);
457+
}
448458

449459
Ok(())
450460
}

0 commit comments

Comments
 (0)