Harden remaining Windows permissions popup navigation

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-03-14 05:08:11 +00:00
parent e96d895c9f
commit 87941e5d75
2 changed files with 21 additions and 20 deletions

View File

@@ -48,7 +48,8 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
- Make the generic permissions-popup helper recognize `Read Only`, which is a real preset on Windows.
- Replace Windows-specific hard-coded navigation counts with label-driven movement in the permission history snapshot tests.
- Rationale: commit `8bc3d489a` fixed the cross-platform `(current)` suffix issue, but `rust-ci` still failed on both Windows test lanes. The remaining Windows-only difference is that the permissions popup includes `Read Only` and selection wraps, so hard-coded extra key presses can land on the wrong preset even when the product behavior is correct.
- Replace the remaining Windows-only extra navigation in the full-access confirmation/history test, and use the same label-driven movement in the adjacent Smart Approvals popup tests.
- Rationale: commit `e96d895c9` still failed on both Windows `Tests` jobs in run `23080608881`. The remaining common pattern is Windows-only popup navigation that assumes a fixed number of `Up`/`Down` presses even though the Windows menu includes `Read Only` and selection wraps.
## Constraints
@@ -69,3 +70,4 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
| `fc98d21ad` | Select Smart Approvals in session-configured popup tests | failed | `Bazel (experimental)` failed on run `23079852646` across macOS and Linux. Investigation narrowed the likely issue to the overly broad `selected_popup_line()` helper introduced in this commit. |
| `a30e8e2ec` | Narrow permissions popup selection helper | failed | `Bazel (experimental)` still failed on Linux in run `23080049381`. The helper narrowing was necessary, but Linux snapshots showed the generic `(current)` assertions were still too strict for the default/full-access permission history tests. |
| `8bc3d489a` | Relax popup current-label assertions | failed | `rust-ci` failed on run `23080137075` in both Windows test jobs after Linux and Bazel turned green. The remaining Windows-only issue is likely the `Read Only` preset and wrapping popup navigation in the generic permission history tests. |
| `e96d895c9` | Stabilize Windows permissions popup navigation | failed | `rust-ci` failed on run `23080608881` in both Windows test jobs again. The history snapshot tests were fixed, but another Windows-only extra `Down` remained in `permissions_full_access_history_cell_emitted_only_after_confirmation`, and adjacent popup-selection tests still relied on fixed-step navigation. |

View File

@@ -6615,6 +6615,19 @@ fn selected_permissions_popup_line(popup: &str) -> &str {
})
}
fn move_permissions_popup_selection_to(chat: &mut ChatWidget, label: &str, direction: KeyCode) {
for _ in 0..4 {
let popup = render_bottom_popup(chat, 120);
if selected_permissions_popup_line(&popup).contains(label) {
return;
}
chat.handle_key_event(KeyEvent::from(direction));
}
let popup = render_bottom_popup(chat, 120);
panic!("expected permissions popup to select {label}: {popup}");
}
#[tokio::test]
async fn apps_popup_stays_loading_until_final_snapshot_updates() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
@@ -8386,13 +8399,7 @@ async fn permissions_selection_history_snapshot_after_mode_switch() {
selected_permissions_popup_line(&popup).contains("Default"),
"expected permissions popup to open with Default selected: {popup}"
);
for _ in 0..3 {
let popup = render_bottom_popup(&chat, 120);
if selected_permissions_popup_line(&popup).contains("Full Access") {
break;
}
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
}
move_permissions_popup_selection_to(&mut chat, "Full Access", KeyCode::Down);
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_permissions_popup_line(&popup).contains("Full Access"),
@@ -8434,13 +8441,7 @@ async fn permissions_selection_history_snapshot_full_access_to_default() {
selected_permissions_popup_line(&popup).contains("Full Access"),
"expected permissions popup to open with Full Access selected: {popup}"
);
for _ in 0..3 {
let popup = render_bottom_popup(&chat, 120);
if selected_permissions_popup_line(&popup).contains("Default") {
break;
}
chat.handle_key_event(KeyEvent::from(KeyCode::Up));
}
move_permissions_popup_selection_to(&mut chat, "Default", KeyCode::Up);
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_permissions_popup_line(&popup).contains("Default"),
@@ -8684,7 +8685,7 @@ async fn permissions_selection_can_disable_smart_approvals() {
"expected permissions popup to open with Smart Approvals selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Up));
move_permissions_popup_selection_to(&mut chat, "Default", KeyCode::Up);
let popup = render_bottom_popup(&chat, 120);
assert!(
popup
@@ -8741,7 +8742,7 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context
"expected permissions popup to open with the current preset selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
move_permissions_popup_selection_to(&mut chat, "Smart Approvals", KeyCode::Down);
let popup = render_bottom_popup(&chat, 120);
assert!(
popup
@@ -8787,9 +8788,7 @@ async fn permissions_full_access_history_cell_emitted_only_after_confirmation()
chat.config.notices.hide_full_access_warning = None;
chat.open_permissions_popup();
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
#[cfg(target_os = "windows")]
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
move_permissions_popup_selection_to(&mut chat, "Full Access", KeyCode::Down);
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
let mut open_confirmation_event = None;