mirror of
https://github.com/openai/codex.git
synced 2026-04-27 16:15:09 +00:00
Stabilize app-server notify initialize test (#13939)
## What changed - This PR changes only the flaky test setup for `turn_start_notify_payload_includes_initialize_client_name`. - Instead of shelling out to `python3` to write the notify payload, the test uses the first-party `codex-app-server-test-notify-capture` helper. - The helper writes `notify.json` atomically and the test waits for the file to exist before reading it. ## Why this fixes the flake - The old test depended on an external Python interpreter being present and behaving consistently on every CI runner. - It also raced the file write: the test could observe the path before the payload had been fully written, which produced partial reads and intermittent assertion failures. - Moving the write into a repo-owned helper removes the external dependency, and atomic write-plus-wait makes the handoff deterministic. ## Scope - Test-only change.
This commit is contained in:
@@ -193,13 +193,13 @@ async fn turn_start_notify_payload_includes_initialize_client_name() -> Result<(
|
||||
let server = create_mock_responses_server_sequence_unchecked(responses).await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let notify_file = codex_home.path().join("notify.json");
|
||||
let notify_capture = cargo_bin("test_notify_capture")?;
|
||||
let notify_capture = cargo_bin("codex-app-server-test-notify-capture")?;
|
||||
let notify_capture = notify_capture
|
||||
.to_str()
|
||||
.expect("notify capture path should be valid UTF-8");
|
||||
let notify_file = notify_file
|
||||
let notify_file_str = notify_file
|
||||
.to_str()
|
||||
.expect("notify output path should be valid UTF-8");
|
||||
.expect("notify file path should be valid UTF-8");
|
||||
create_config_toml_with_extra(
|
||||
codex_home.path(),
|
||||
&server.uri(),
|
||||
@@ -207,7 +207,7 @@ async fn turn_start_notify_payload_includes_initialize_client_name() -> Result<(
|
||||
&format!(
|
||||
"notify = [{}, {}]",
|
||||
toml_basic_string(notify_capture),
|
||||
toml_basic_string(notify_file)
|
||||
toml_basic_string(notify_file_str)
|
||||
),
|
||||
)?;
|
||||
|
||||
@@ -255,9 +255,8 @@ async fn turn_start_notify_payload_includes_initialize_client_name() -> Result<(
|
||||
)
|
||||
.await??;
|
||||
|
||||
let notify_file = Path::new(notify_file);
|
||||
fs_wait::wait_for_path_exists(notify_file, Duration::from_secs(5)).await?;
|
||||
let payload_raw = tokio::fs::read_to_string(notify_file).await?;
|
||||
fs_wait::wait_for_path_exists(¬ify_file, Duration::from_secs(5)).await?;
|
||||
let payload_raw = tokio::fs::read_to_string(¬ify_file).await?;
|
||||
let payload: Value = serde_json::from_str(&payload_raw)?;
|
||||
assert_eq!(payload["client"], "xcode");
|
||||
|
||||
@@ -292,6 +291,9 @@ model_provider = "mock_provider"
|
||||
|
||||
{extra}
|
||||
|
||||
[features]
|
||||
shell_snapshot = false
|
||||
|
||||
[model_providers.mock_provider]
|
||||
name = "Mock provider for test"
|
||||
base_url = "{server_uri}/v1"
|
||||
|
||||
@@ -35,6 +35,51 @@ use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
async fn wait_for_responses_request_count_to_stabilize(
|
||||
server: &wiremock::MockServer,
|
||||
expected_count: usize,
|
||||
settle_duration: std::time::Duration,
|
||||
) -> Result<()> {
|
||||
timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
let mut stable_since: Option<tokio::time::Instant> = None;
|
||||
loop {
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.context("failed to fetch received requests")?;
|
||||
let responses_request_count = requests
|
||||
.iter()
|
||||
.filter(|request| {
|
||||
request.method == "POST" && request.url.path().ends_with("/responses")
|
||||
})
|
||||
.count();
|
||||
|
||||
if responses_request_count > expected_count {
|
||||
anyhow::bail!(
|
||||
"expected exactly {expected_count} /responses requests, got {responses_request_count}"
|
||||
);
|
||||
}
|
||||
|
||||
if responses_request_count == expected_count {
|
||||
match stable_since {
|
||||
Some(stable_since) if stable_since.elapsed() >= settle_duration => {
|
||||
return Ok::<(), anyhow::Error>(());
|
||||
}
|
||||
None => stable_since = Some(tokio::time::Instant::now()),
|
||||
Some(_) => {}
|
||||
}
|
||||
} else {
|
||||
stable_since = None;
|
||||
}
|
||||
|
||||
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_unsubscribe_unloads_thread_and_emits_thread_closed_notification() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -172,6 +217,13 @@ async fn thread_unsubscribe_during_turn_interrupts_turn_and_emits_thread_closed(
|
||||
};
|
||||
assert_eq!(payload.thread_id, thread_id);
|
||||
|
||||
wait_for_responses_request_count_to_stabilize(
|
||||
&server,
|
||||
1,
|
||||
std::time::Duration::from_millis(200),
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user