Skip to content

Commit 2b3ed21

Browse files
CSResselCSRessel
committed
fix(pty): Unify pty E2E test timing for less flakiness
1 parent a7eab21 commit 2b3ed21

11 files changed

Lines changed: 33 additions & 90 deletions

File tree

‎codex-rs/core/src/lib.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ pub use mcp_connection_manager::SandboxState;
3838
mod mcp_tool_call;
3939
mod message_history;
4040
mod model_provider_info;
41-
4241
pub mod parse_command;
4342
pub mod powershell;
4443
mod response_processing;
@@ -48,12 +47,9 @@ pub mod token_data;
4847
mod truncate;
4948
mod unified_exec;
5049
mod user_instructions;
51-
pub use model_provider_info::CLAUDE_ACP_PROVIDER_ID;
5250
pub use model_provider_info::DEFAULT_LMSTUDIO_PORT;
5351
pub use model_provider_info::DEFAULT_OLLAMA_PORT;
54-
pub use model_provider_info::GEMINI_ACP_PROVIDER_ID;
5552
pub use model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
56-
pub use model_provider_info::MOCK_ACP_PROVIDER_ID;
5753
pub use model_provider_info::ModelProviderInfo;
5854
pub use model_provider_info::OLLAMA_OSS_PROVIDER_ID;
5955
pub use model_provider_info::WireApi;

‎codex-rs/core/src/model_family.rs‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
use codex_protocol::config_types::ReasoningEffort;
22
use codex_protocol::config_types::Verbosity;
33

4-
use crate::CLAUDE_ACP_PROVIDER_ID;
5-
use crate::GEMINI_ACP_PROVIDER_ID;
6-
use crate::MOCK_ACP_PROVIDER_ID;
74
use crate::config::types::ReasoningSummaryFormat;
85
use crate::tools::handlers::apply_patch::ApplyPatchToolType;
96
use crate::tools::spec::ConfigShellToolType;
@@ -129,18 +126,6 @@ pub fn find_family_for_model(slug: &str) -> Option<ModelFamily> {
129126
needs_special_apply_patch_instructions: true,
130127
shell_type: ConfigShellToolType::Local,
131128
)
132-
} else if slug.starts_with("gemini") {
133-
model_family!(
134-
slug, GEMINI_ACP_PROVIDER_ID,
135-
supports_parallel_tool_calls: true,
136-
)
137-
} else if slug.starts_with("claude") {
138-
model_family!(
139-
slug, CLAUDE_ACP_PROVIDER_ID,
140-
supports_parallel_tool_calls: true,
141-
)
142-
} else if slug.starts_with("mock") {
143-
model_family!(slug, MOCK_ACP_PROVIDER_ID)
144129
} else if slug.starts_with("gpt-4.1") {
145130
model_family!(
146131
slug, "gpt-4.1",

‎codex-rs/core/src/model_provider_info.rs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ const MAX_REQUEST_MAX_RETRIES: u64 = 100;
3636
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
3737
#[serde(rename_all = "lowercase")]
3838
pub enum WireApi {
39-
/// The OpenAI Responses API. This is used by some internal models.
39+
/// The Responses API exposed by OpenAI at `/v1/responses`.
4040
Responses,
4141

42-
/// The OpenAI Chat Completions API. This is the default.
42+
/// Regular Chat Completions compatible with `/v1/chat/completions`.
4343
#[default]
4444
Chat,
4545
}
@@ -216,17 +216,7 @@ pub const DEFAULT_OLLAMA_PORT: u16 = 11434;
216216
pub const LMSTUDIO_OSS_PROVIDER_ID: &str = "lmstudio";
217217
pub const OLLAMA_OSS_PROVIDER_ID: &str = "ollama";
218218

219-
// ACP provider identifiers (used for model inference, not in built_in_model_providers)
220-
// Actual ACP provider configuration is embedded in AcpAgentConfig from codex_acp::registry
221-
pub const CLAUDE_ACP_PROVIDER_ID: &str = "claude-acp";
222-
pub const GEMINI_ACP_PROVIDER_ID: &str = "gemini-acp";
223-
pub const MOCK_ACP_PROVIDER_ID: &str = "mock-acp";
224-
225219
/// Built-in default provider list.
226-
///
227-
/// Note: ACP providers (mock-acp, claude-acp, gemini-acp) are NOT included here.
228-
/// ACP agent configuration is handled by the `codex_acp::registry` module,
229-
/// which embeds provider info directly in `AcpAgentConfig` to avoid circular dependencies.
230220
pub fn built_in_model_providers() -> HashMap<String, ModelProviderInfo> {
231221
use ModelProviderInfo as P;
232222

‎codex-rs/tui-pty-e2e/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,8 @@ fn codex_binary_path() -> String {
547547
}
548548

549549
pub const TIMEOUT: Duration = Duration::from_secs(5);
550-
pub const TIMEOUT_INPUT: Duration = Duration::from_millis(300);
550+
pub const TIMEOUT_INPUT: Duration = Duration::from_millis(100);
551+
pub const TIMEOUT_PRESNAPSHOT: Duration = Duration::from_millis(500);
551552

552553
/// Normalize dynamic content in screen output for snapshot testing
553554
pub fn normalize_for_snapshot(contents: String) -> String {

‎codex-rs/tui-pty-e2e/tests/acp_mode.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use tui_pty_e2e::Key;
1010
use tui_pty_e2e::SessionConfig;
1111
use tui_pty_e2e::TIMEOUT;
1212
use tui_pty_e2e::TIMEOUT_INPUT;
13+
use tui_pty_e2e::TIMEOUT_PRESNAPSHOT;
1314
use tui_pty_e2e::TuiSession;
1415
use tui_pty_e2e::normalize_for_input_snapshot;
1516

@@ -160,8 +161,7 @@ fn test_acp_mode_startup_snapshot() {
160161
.wait_for_text("›", TIMEOUT)
161162
.expect("ACP mode should start");
162163

163-
std::thread::sleep(TIMEOUT_INPUT);
164-
164+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
165165
insta::assert_snapshot!(
166166
"acp_mode_startup",
167167
normalize_for_input_snapshot(session.screen_contents())

‎codex-rs/tui-pty-e2e/tests/acp_tool_calls.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use tui_pty_e2e::Key;
3030
use tui_pty_e2e::SessionConfig;
3131
use tui_pty_e2e::TIMEOUT;
3232
use tui_pty_e2e::TIMEOUT_INPUT;
33+
use tui_pty_e2e::TIMEOUT_PRESNAPSHOT;
3334
use tui_pty_e2e::TuiSession;
3435
use tui_pty_e2e::normalize_for_input_snapshot;
3536

@@ -194,8 +195,7 @@ fn test_acp_tool_call_snapshot() {
194195
.wait_for_text("Read complete", Duration::from_secs(10))
195196
.expect("Should receive response");
196197

197-
std::thread::sleep(TIMEOUT_INPUT);
198-
198+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
199199
insta::assert_snapshot!(
200200
"acp_tool_call_rendered",
201201
normalize_for_input_snapshot(session.screen_contents())

‎codex-rs/tui-pty-e2e/tests/input_handling.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use insta::assert_snapshot;
22
use std::time::Duration;
33
use tui_pty_e2e::Key;
44
use tui_pty_e2e::TIMEOUT;
5-
use tui_pty_e2e::TIMEOUT_INPUT;
5+
use tui_pty_e2e::TIMEOUT_PRESNAPSHOT;
66
use tui_pty_e2e::TuiSession;
77
use tui_pty_e2e::normalize_for_input_snapshot;
88

@@ -23,9 +23,7 @@ fn test_ctrl_c_clears_input() {
2323
.wait_for(|s| !s.contains("draft message"), TIMEOUT)
2424
.expect("Input was not cleared");
2525

26-
std::thread::sleep(TIMEOUT_INPUT);
27-
std::thread::sleep(TIMEOUT_INPUT);
28-
std::thread::sleep(TIMEOUT_INPUT);
26+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
2927
assert_snapshot!(
3028
"ctrl_c_clears",
3129
normalize_for_input_snapshot(session.screen_contents())
@@ -48,9 +46,7 @@ fn test_backspace() {
4846
session.wait_for_text("Hel", TIMEOUT).unwrap();
4947
session.wait_for(|s| !s.contains("Hello"), TIMEOUT).unwrap();
5048

51-
std::thread::sleep(TIMEOUT_INPUT);
52-
std::thread::sleep(TIMEOUT_INPUT);
53-
std::thread::sleep(TIMEOUT_INPUT);
49+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
5450
assert_snapshot!(
5551
"typing_and_backspace",
5652
normalize_for_input_snapshot(session.screen_contents())
@@ -72,9 +68,7 @@ fn test_arrows() {
7268
session.send_key(Key::Down).unwrap();
7369
std::thread::sleep(Duration::from_millis(100));
7470

75-
std::thread::sleep(TIMEOUT_INPUT);
76-
std::thread::sleep(TIMEOUT_INPUT);
77-
std::thread::sleep(TIMEOUT_INPUT);
71+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
7872
assert_snapshot!(
7973
"model_changed",
8074
normalize_for_input_snapshot(session.screen_contents())

‎codex-rs/tui-pty-e2e/tests/manual-test-cancellation.md‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎codex-rs/tui-pty-e2e/tests/prompt_flow.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use tui_pty_e2e::Key;
33
use tui_pty_e2e::SessionConfig;
44
use tui_pty_e2e::TIMEOUT;
55
use tui_pty_e2e::TIMEOUT_INPUT;
6+
use tui_pty_e2e::TIMEOUT_PRESNAPSHOT;
67
use tui_pty_e2e::TuiSession;
78
use tui_pty_e2e::normalize_for_input_snapshot;
89

@@ -30,7 +31,7 @@ fn test_submit_prompt_default_response() {
3031
.wait_for_text("Test message 2", TIMEOUT)
3132
.expect("Did not receive second mock response");
3233

33-
std::thread::sleep(TIMEOUT_INPUT);
34+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
3435
assert_snapshot!(
3536
"prompt_submitted",
3637
normalize_for_input_snapshot(session.screen_contents())
@@ -67,7 +68,7 @@ fn test_submit_prompt_missing_model() {
6768
)
6869
.unwrap();
6970

70-
std::thread::sleep(TIMEOUT_INPUT);
71+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
7172
assert_snapshot!(
7273
"missing_model",
7374
normalize_for_input_snapshot(session.screen_contents())
@@ -92,7 +93,7 @@ fn test_submit_prompt_custom_response() {
9293
.wait_for_text("This is a custom test response", TIMEOUT)
9394
.expect("Did not receive custom response");
9495

95-
std::thread::sleep(TIMEOUT_INPUT);
96+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
9697
assert_snapshot!(
9798
"custom_response",
9899
normalize_for_input_snapshot(session.screen_contents())
@@ -112,7 +113,7 @@ fn test_multiline_input() {
112113
session.wait_for_text("Line 2", TIMEOUT).unwrap();
113114
session.wait_for_text("Line 3", TIMEOUT).unwrap();
114115

115-
std::thread::sleep(TIMEOUT_INPUT);
116+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
116117
assert_snapshot!(
117118
"multiline_input",
118119
normalize_for_input_snapshot(session.screen_contents())

‎codex-rs/tui-pty-e2e/tests/startup.rs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::time::Duration;
33
use std::time::Instant;
44
use tui_pty_e2e::SessionConfig;
55
use tui_pty_e2e::TIMEOUT;
6-
use tui_pty_e2e::TIMEOUT_INPUT;
6+
use tui_pty_e2e::TIMEOUT_PRESNAPSHOT;
77
use tui_pty_e2e::TuiSession;
88
use tui_pty_e2e::normalize_for_input_snapshot;
99

@@ -23,13 +23,14 @@ fn test_startup_shows_banner() {
2323
session
2424
.wait_for_text("Welcome to Codex", TIMEOUT)
2525
.expect("Prompt did not appear");
26-
std::thread::sleep(TIMEOUT_INPUT);
26+
27+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
2728

2829
let contents = session.screen_contents();
2930
assert!(contents.contains("Welcome to Codex"));
3031
assert_snapshot!(
3132
"startup_shows_welcome",
32-
normalize_for_input_snapshot(session.screen_contents())
33+
normalize_for_input_snapshot(contents)
3334
);
3435
}
3536

@@ -48,14 +49,16 @@ fn test_startup_welcome_with_dimensions() {
4849
session
4950
.wait_for_text("OpenAI Codex", TIMEOUT)
5051
.expect("Prompt did not appear");
51-
std::thread::sleep(TIMEOUT_INPUT);
52+
53+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
5254

5355
// Verify terminal size is respected
5456
let contents = session.screen_contents();
5557
assert!(contents.lines().count() <= 40);
58+
5659
assert_snapshot!(
5760
"startup_welcome_dimensions_40x120",
58-
normalize_for_input_snapshot(session.screen_contents())
61+
normalize_for_input_snapshot(contents)
5962
);
6063
}
6164

@@ -74,7 +77,8 @@ fn test_runs_in_temp_directory_by_default() {
7477
session
7578
.wait_for_text("To get started", TIMEOUT)
7679
.expect("Prompt did not appear");
77-
std::thread::sleep(TIMEOUT_INPUT);
80+
81+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
7882

7983
let contents = session.screen_contents();
8084

@@ -93,7 +97,7 @@ fn test_runs_in_temp_directory_by_default() {
9397
);
9498
assert_snapshot!(
9599
"runs_in_temp_directory",
96-
normalize_for_input_snapshot(session.screen_contents())
100+
normalize_for_input_snapshot(contents)
97101
);
98102
}
99103

@@ -105,7 +109,7 @@ fn test_trust_screen_is_skipped_with_default_config() {
105109
session
106110
.wait_for_text("›", TIMEOUT)
107111
.expect("Prompt did not appear");
108-
std::thread::sleep(TIMEOUT_INPUT);
112+
std::thread::sleep(TIMEOUT_PRESNAPSHOT);
109113

110114
let contents = session.screen_contents();
111115

@@ -124,7 +128,7 @@ fn test_trust_screen_is_skipped_with_default_config() {
124128
);
125129
assert_snapshot!(
126130
"trust_screen_skipped",
127-
normalize_for_input_snapshot(session.screen_contents())
131+
normalize_for_input_snapshot(contents)
128132
);
129133
}
130134

0 commit comments

Comments
 (0)