From 8ccc42dfd68ea8b324060e19b897ae3a20c0d70b Mon Sep 17 00:00:00 2001 From: Roy Han Date: Mon, 2 Mar 2026 18:49:18 -0800 Subject: [PATCH] remove tests for now --- codex-rs/tui/src/app.rs | 220 ------------------ .../tui/src/bottom_pane/approval_overlay.rs | 171 -------------- ...ply_patch_declined_accepting_revision.snap | 6 - ...et__tests__approval_modal_patch_notes.snap | 22 -- codex-rs/tui/src/chatwidget/tests.rs | 183 --------------- ...atch_with_notes_follow_up_user_prompt.snap | 7 - 6 files changed, 609 deletions(-) delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_declined_accepting_revision.snap delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch_notes.snap delete mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__tests__reject_patch_with_notes_follow_up_user_prompt.snap diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7efea158bc..10e758657e 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3660,12 +3660,9 @@ mod tests { use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::openai_models::ModelAvailabilityNux; - use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; - use codex_protocol::protocol::FileChange; - use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::protocol::SessionSource; @@ -3867,181 +3864,6 @@ mod tests { panic!("expected approval action to submit a thread-scoped op"); } - #[tokio::test] - async fn reject_patch_with_notes_submits_ops_and_renders_follow_up() -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let thread_id = ThreadId::new(); - app.active_thread_id = Some(thread_id); - - reject_patch_with_notes_for_test( - &mut app, - thread_id, - "call-1", - "i want the content to say bye instead.", - ) - .await; - - assert_eq!( - op_rx.try_recv(), - Ok(Op::PatchApproval { - id: "call-1".to_string(), - decision: ReviewDecision::Denied, - }) - ); - assert_eq!( - op_rx.try_recv(), - Ok(Op::UserInput { - items: vec![UserInput::Text { - text: "i want the content to say bye instead.".to_string(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - }) - ); - - drain_insert_history_cells(&mut app, &mut app_event_rx); - - let user_messages: Vec = app - .transcript_cells - .iter() - .filter_map(|cell| { - cell.as_any() - .downcast_ref::() - .map(|cell| cell.message.clone()) - }) - .collect(); - assert_eq!( - user_messages, - vec!["i want the content to say bye instead.".to_string()] - ); - assert_snapshot!( - "reject_patch_with_notes_follow_up_user_prompt", - render_transcript_cells_for_snapshot(&app, 80) - ); - Ok(()) - } - - #[tokio::test] - async fn reject_patch_with_notes_skips_steering_when_deny_fails() -> Result<()> { - let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; - - reject_patch_with_notes_for_test( - &mut app, - ThreadId::new(), - "call-missing", - "try again with fewer files", - ) - .await; - - let mut errors = Vec::new(); - while let Ok(event) = app_event_rx.try_recv() { - if let AppEvent::InsertHistoryCell(cell) = event { - let text = cell - .display_lines(120) - .into_iter() - .map(|line| { - line.spans - .into_iter() - .map(|span| span.content.into_owned()) - .collect::() - }) - .collect::>() - .join("\n"); - errors.push(text); - } - } - - assert_eq!(errors.len(), 1); - assert!(errors[0].contains("Failed to find thread")); - Ok(()) - } - - #[tokio::test] - async fn reject_patch_with_notes_routes_denial_to_originating_thread() -> Result<()> { - let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let active = app.server.start_thread(app.config.clone()).await?; - let target = app.server.start_thread(app.config.clone()).await?; - app.active_thread_id = Some(active.thread_id); - - let active_channel = ThreadEventChannel::new(THREAD_EVENT_CHANNEL_CAPACITY); - { - let mut store = active_channel.store.lock().await; - store.push_event(Event { - id: "active-approval".to_string(), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "active-call".to_string(), - turn_id: "active-turn".to_string(), - changes: HashMap::from([( - PathBuf::from("active.rs"), - FileChange::Add { - content: "fn active() {}\n".to_string(), - }, - )]), - reason: None, - grant_root: None, - }), - }); - } - app.thread_event_channels - .insert(active.thread_id, active_channel); - - let target_channel = ThreadEventChannel::new(THREAD_EVENT_CHANNEL_CAPACITY); - { - let mut store = target_channel.store.lock().await; - store.push_event(Event { - id: "target-approval".to_string(), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "target-call".to_string(), - turn_id: "target-turn".to_string(), - changes: HashMap::from([( - PathBuf::from("target.rs"), - FileChange::Add { - content: "fn target() {}\n".to_string(), - }, - )]), - reason: None, - grant_root: None, - }), - }); - } - app.thread_event_channels - .insert(target.thread_id, target_channel); - - reject_patch_with_notes_for_test( - &mut app, - target.thread_id, - "target-call", - "apply only the target file", - ) - .await; - - assert!( - op_rx.try_recv().is_err(), - "cross-thread steering should not route through the active thread sender" - ); - - let active_pending = app - .thread_event_channels - .get(&active.thread_id) - .expect("active channel") - .store - .lock() - .await - .has_pending_thread_approvals(); - let target_pending = app - .thread_event_channels - .get(&target.thread_id) - .expect("target channel") - .store - .lock() - .await - .has_pending_thread_approvals(); - - assert!(active_pending); - assert!(!target_pending); - Ok(()) - } - #[tokio::test] async fn routed_thread_event_does_not_recreate_channel_after_reset() -> Result<()> { let mut app = make_test_app().await; @@ -4686,48 +4508,6 @@ mod tests { ) } - async fn reject_patch_with_notes_for_test( - app: &mut App, - thread_id: ThreadId, - approval_id: &str, - text: &str, - ) { - app.reject_patch_approval_with_notes(thread_id, approval_id.to_string(), text.to_string()) - .await; - } - - fn drain_insert_history_cells( - app: &mut App, - app_event_rx: &mut tokio::sync::mpsc::UnboundedReceiver, - ) { - while let Ok(event) = app_event_rx.try_recv() { - if let AppEvent::InsertHistoryCell(cell) = event { - app.transcript_cells.push(cell.into()); - } - } - } - - fn render_transcript_cells_for_snapshot(app: &App, width: u16) -> String { - app.transcript_cells - .iter() - .enumerate() - .flat_map(|(idx, cell)| { - let mut lines = cell.display_lines(width); - if idx > 0 && !lines.is_empty() { - lines.insert(0, Line::from("")); - } - lines - }) - .map(|line| { - line.spans - .iter() - .map(|span| span.content.as_ref()) - .collect::() - }) - .collect::>() - .join("\n") - } - fn test_otel_manager(config: &Config, model: &str) -> OtelManager { let model_info = codex_core::test_support::construct_model_info_offline(model, config); OtelManager::new( diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 08254f10ae..7ca44badb3 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -1364,24 +1364,6 @@ mod tests { } } - fn make_patch_request() -> ApprovalRequest { - let mut changes = HashMap::new(); - changes.insert( - PathBuf::from("README.md"), - FileChange::Add { - content: "hello\nworld\n".to_string(), - }, - ); - ApprovalRequest::ApplyPatch { - thread_id: ThreadId::new(), - thread_label: None, - id: "patch-test".to_string(), - reason: Some("review these edits".to_string()), - cwd: PathBuf::from("/tmp"), - changes, - } - } - #[test] fn ctrl_c_aborts_and_clears_queue() { let (tx, _rx) = unbounded_channel::(); @@ -1851,157 +1833,4 @@ mod tests { } assert_eq!(decision, Some(ReviewDecision::Approved)); } - - #[test] - fn patch_reject_shortcuts_open_notes_and_show_expected_labels() { - let (tx, _rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let view = ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - let rendered = render_overlay_lines(&view, 80); - - assert!( - rendered.contains("(y)"), - "patch option should show y shortcut: {rendered}" - ); - assert!( - rendered.contains("(a)"), - "patch option should show a shortcut: {rendered}" - ); - assert!( - rendered.contains("(tab)"), - "patch option should show tab shortcut: {rendered}" - ); - assert!( - !rendered.contains("(esc)"), - "patch option should not show esc shortcut: {rendered}" - ); - assert!( - rendered.contains("esc to interrupt"), - "patch modal should show interrupt hint under options: {rendered}" - ); - - let assert_opens_notes = |events: &[KeyEvent]| { - let (tx, _rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = - ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - for event in events { - view.handle_key_event(*event); - } - - let state = view.patch_state().expect("patch state"); - assert_eq!( - state.options_state.selected_idx, - Some(PATCH_REJECT_OPTION_INDEX) - ); - assert_eq!(state.focus, PatchFocus::Notes); - assert!(view.patch_notes_visible()); - assert!(!view.is_complete()); - }; - - assert_opens_notes(&[KeyEvent::from(KeyCode::Tab)]); - assert_opens_notes(&[KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE)]); - assert_opens_notes(&[KeyEvent::new(KeyCode::Char('3'), KeyModifiers::NONE)]); - assert_opens_notes(&[ - KeyEvent::from(KeyCode::Down), - KeyEvent::from(KeyCode::Down), - KeyEvent::from(KeyCode::Enter), - ]); - } - - #[test] - fn patch_shortcuts_bind_expected_actions() { - let (tx, mut rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - - view.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); - - let mut decision = None; - while let Ok(event) = rx.try_recv() { - if let AppEvent::SubmitThreadOp { - op: Op::PatchApproval { decision: d, .. }, - .. - } = event - { - decision = Some(d); - break; - } - } - assert_eq!(decision, Some(ReviewDecision::Approved)); - - let (tx, mut rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - - view.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE)); - - let mut decision = None; - while let Ok(event) = rx.try_recv() { - if let AppEvent::SubmitThreadOp { - op: Op::PatchApproval { decision: d, .. }, - .. - } = event - { - decision = Some(d); - break; - } - } - assert_eq!(decision, Some(ReviewDecision::ApprovedForSession)); - - let (tx, mut rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - view.handle_key_event(KeyEvent::from(KeyCode::Tab)); - - assert_eq!(view.on_ctrl_c(), CancellationEvent::Handled); - - let mut decision = None; - while let Ok(event) = rx.try_recv() { - if let AppEvent::SubmitThreadOp { - op: Op::PatchApproval { decision: d, .. }, - .. - } = event - { - decision = Some(d); - break; - } - } - assert_eq!(decision, Some(ReviewDecision::Abort)); - } - - #[test] - fn patch_notes_validate_before_submit_and_emit_event() { - let (tx, mut rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_patch_request(), tx, Features::with_defaults()); - view.handle_key_event(KeyEvent::from(KeyCode::Tab)); - - view.handle_key_event(KeyEvent::from(KeyCode::Enter)); - - assert!( - rx.try_recv().is_err(), - "unexpected event for empty note submit" - ); - assert!(!view.is_complete()); - assert!(view.patch_note_error_visible()); - - view.patch_state_mut() - .expect("patch state") - .composer - .set_text_content("use smaller diffs".to_string(), Vec::new(), Vec::new()); - - view.handle_key_event(KeyEvent::from(KeyCode::Enter)); - - let event = rx.try_recv().expect("reject event"); - assert!( - matches!( - event, - AppEvent::RejectPatchApprovalWithNotes { text, .. } - if text == "use smaller diffs" - ), - "expected reject-with-notes event" - ); - assert!(view.is_complete()); - } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_declined_accepting_revision.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_declined_accepting_revision.snap deleted file mode 100644 index 2328a5a639..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_declined_accepting_revision.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tui/src/chatwidget/tests.rs -assertion_line: 7580 -expression: rendered ---- -• Accepting revision diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch_notes.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch_notes.snap deleted file mode 100644 index 10a5ef4fb6..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch_notes.snap +++ /dev/null @@ -1,22 +0,0 @@ ---- -source: tui/src/chatwidget/tests.rs -assertion_line: 7041 -expression: terminal.backend().vt100().screen().contents() ---- - - - Would you like to make the following edits? - - Reason: The model wants to apply changes - - README.md (+2 -0) - - 1 +hello - 2 +world - - 1. Yes, proceed (y) - 2. Yes, and don't ask again for these files (a) - › 3. No, and tell Codex what to do differently (tab) - enter to send | tab to go back | esc to interrupt - - › split the edits into smaller commits diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index fcf312031d..d89ed4bf55 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -7001,51 +7001,6 @@ async fn approval_modal_patch_snapshot() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn approval_modal_patch_notes_snapshot() -> anyhow::Result<()> { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - chat.config - .permissions - .approval_policy - .set(AskForApproval::OnRequest)?; - - let mut changes = HashMap::new(); - changes.insert( - PathBuf::from("README.md"), - FileChange::Add { - content: "hello\nworld\n".into(), - }, - ); - chat.handle_codex_event(Event { - id: "sub-approve-patch-notes".into(), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "call-approve-patch-notes".into(), - turn_id: "turn-approve-patch-notes".into(), - changes, - reason: Some("The model wants to apply changes".into()), - grant_root: Some(PathBuf::from("/tmp")), - }), - }); - - chat.handle_key_event(KeyEvent::from(KeyCode::Tab)); - chat.bottom_pane - .handle_paste("split the edits into smaller commits".to_string()); - - let height = chat.desired_height(80); - let mut terminal = - ratatui::Terminal::new(VT100Backend::new(80, height)).expect("create terminal"); - terminal.set_viewport_area(Rect::new(0, 0, 80, height)); - terminal - .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("draw patch approval notes modal"); - assert_snapshot!( - "approval_modal_patch_notes", - terminal.backend().vt100().screen().contents() - ); - - Ok(()) -} - #[tokio::test] async fn interrupt_restores_queued_messages_into_composer() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; @@ -7558,32 +7513,6 @@ async fn apply_patch_events_emit_history_cells() { ); } -#[tokio::test] -async fn apply_patch_declined_shows_accepting_revision_without_detail() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - - chat.handle_codex_event(Event { - id: "declined".into(), - msg: EventMsg::PatchApplyEnd(PatchApplyEndEvent { - call_id: "call-declined".into(), - turn_id: "turn-declined".into(), - stdout: String::new(), - stderr: "patch rejected by user".into(), - success: false, - changes: HashMap::new(), - status: CorePatchApplyStatus::Declined, - }), - }); - - let cells = drain_insert_history(&mut rx); - let rendered = lines_to_single_string(cells.last().expect("declined patch cell")); - assert_snapshot!("apply_patch_declined_accepting_revision", rendered); - assert!( - !rendered.contains("patch rejected by user"), - "declined patch row should not show the low-level rejection detail" - ); -} - #[tokio::test] async fn apply_patch_manual_approval_adjusts_header() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; @@ -7729,118 +7658,6 @@ async fn apply_patch_approval_sends_op_with_call_id() { assert!(found, "expected PatchApproval op to be sent"); } -#[tokio::test] -async fn apply_patch_notes_choose_between_soft_reject_and_abort() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - let mut changes = HashMap::new(); - changes.insert( - PathBuf::from("file.rs"), - FileChange::Add { - content: "fn main(){}\n".into(), - }, - ); - chat.handle_codex_event(Event { - id: "sub-124".into(), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "call-1000".into(), - turn_id: "turn-1000".into(), - changes, - reason: None, - grant_root: None, - }), - }); - - chat.handle_key_event(KeyEvent::from(KeyCode::Tab)); - chat.bottom_pane - .handle_paste("apply only the formatter changes".to_string()); - chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - - let mut saw_reject = false; - let mut saw_abort = false; - let mut saw_interruption_banner = false; - while let Ok(event) = rx.try_recv() { - match event { - AppEvent::RejectPatchApprovalWithNotes { text, .. } => { - assert_eq!(text, "apply only the formatter changes"); - saw_reject = true; - } - AppEvent::SubmitThreadOp { - op: - Op::PatchApproval { - decision: codex_protocol::protocol::ReviewDecision::Abort, - .. - }, - .. - } => { - saw_abort = true; - } - AppEvent::InsertHistoryCell(cell) => { - let rendered = cell - .display_lines(120) - .into_iter() - .map(|line| { - line.spans - .into_iter() - .map(|span| span.content.into_owned()) - .collect::() - }) - .collect::>() - .join("\n"); - if rendered.contains("Conversation interrupted") { - saw_interruption_banner = true; - } - } - _ => {} - } - } - - assert!(saw_reject, "expected reject-with-notes app event"); - assert!(!saw_abort, "soft reject should not abort the turn"); - assert!( - !saw_interruption_banner, - "soft reject should not show the interruption banner" - ); - - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - let mut changes = HashMap::new(); - changes.insert( - PathBuf::from("file.rs"), - FileChange::Add { - content: "fn main(){}\n".into(), - }, - ); - chat.handle_codex_event(Event { - id: "sub-125".into(), - msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { - call_id: "call-1001".into(), - turn_id: "turn-1001".into(), - changes, - reason: None, - grant_root: None, - }), - }); - - chat.handle_key_event(KeyEvent::from(KeyCode::Tab)); - chat.bottom_pane - .handle_paste("this note should be discarded".to_string()); - chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); - - let mut saw_abort = false; - while let Ok(event) = rx.try_recv() { - if let AppEvent::SubmitThreadOp { - op: Op::PatchApproval { decision, .. }, - .. - } = event - && decision == codex_protocol::protocol::ReviewDecision::Abort - { - saw_abort = true; - break; - } - } - - assert!(saw_abort, "esc should still abort the patch approval flow"); -} - #[tokio::test] async fn apply_patch_full_flow_integration_like() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__reject_patch_with_notes_follow_up_user_prompt.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__reject_patch_with_notes_follow_up_user_prompt.snap deleted file mode 100644 index 9f19529598..0000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__app__tests__reject_patch_with_notes_follow_up_user_prompt.snap +++ /dev/null @@ -1,7 +0,0 @@ ---- -source: tui/src/app.rs -assertion_line: 3932 -expression: "render_transcript_cells_for_snapshot(&app, 80)" ---- - -› i want the content to say bye instead.