mirror of
https://github.com/openai/codex.git
synced 2026-09-14 11:57:03 +00:00
Correct Windows popup selection assertions
Use exact selected preset names in the permissions popup tests and restore the correct Windows expectation for the default unelevated popup opening on Read Only instead of Default. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -50,6 +50,7 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
|
||||
- Replace Windows-specific hard-coded navigation counts with label-driven movement in the permission history snapshot tests.
|
||||
- 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.
|
||||
- Follow-up on `87941e5d7`: the helper compared the entire selected row with `contains(label)`, which made `Smart Approvals` look like `Default` because its description text says "Same workspace-write permissions as Default...". Tighten the helper to match the selected preset name at the start of the row instead of any substring in the rendered line.
|
||||
- Follow-up on `13c9d91b0`: the cross-platform helper bug is fixed, but the default unelevated Windows popup still opens on `Read Only` instead of `Default`. The remaining Windows-only failures are in tests that added exact initial-selection assertions while assuming the popup would start on `Default`.
|
||||
- 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
|
||||
@@ -72,3 +73,5 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
|
||||
| `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. |
|
||||
| `87941e5d7` | Harden remaining Windows permissions popup navigation | failed | `rust-ci` and Bazel both regressed on run `23081011779`. The new generic navigation helper matched the entire selected row with `contains(label)`, so the Smart Approvals description text incorrectly satisfied `Default`. |
|
||||
| `13c9d91b0` | Avoid false matches in permissions popup helper | failed | Run `23081377233` fixed the Linux/macOS/Bazel regression, but both Windows `Tests` jobs still failed. Signed log downloads from both `results-receiver.actions.githubusercontent.com` and the Azure blob log URL hit TLS EOFs in this environment, so the remaining diagnosis comes from CI history plus the test diff: the default unelevated Windows popup still opens on `Read Only`, and the new exact-selection assertions were assuming `Default`. |
|
||||
|
||||
@@ -8374,15 +8374,23 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() {
|
||||
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
#[cfg(target_os = "windows")]
|
||||
let expected_initial = "Read Only";
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let expected_initial = "Default";
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Default"),
|
||||
"expected permissions popup to open with Default selected: {popup}"
|
||||
selected_permissions_popup_name(&popup) == expected_initial,
|
||||
"expected permissions popup to open with {expected_initial} selected: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
#[cfg(target_os = "windows")]
|
||||
let expected_after_one_down = "Default";
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let expected_after_one_down = "Full Access";
|
||||
assert!(
|
||||
!selected_permissions_popup_line(&popup).contains("Default"),
|
||||
"expected moving down to change the selected preset before confirmation: {popup}"
|
||||
selected_permissions_popup_name(&popup) == expected_after_one_down,
|
||||
"expected moving down to select {expected_after_one_down} before confirmation: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
@@ -8411,14 +8419,18 @@ async fn permissions_selection_history_snapshot_after_mode_switch() {
|
||||
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
#[cfg(target_os = "windows")]
|
||||
let expected_initial = "Read Only";
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let expected_initial = "Default";
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Default"),
|
||||
"expected permissions popup to open with Default selected: {popup}"
|
||||
selected_permissions_popup_name(&popup) == expected_initial,
|
||||
"expected permissions popup to open with {expected_initial} selected: {popup}"
|
||||
);
|
||||
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"),
|
||||
selected_permissions_popup_name(&popup) == "Full Access",
|
||||
"expected navigation to land on Full Access before confirmation: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
@@ -8454,13 +8466,13 @@ async fn permissions_selection_history_snapshot_full_access_to_default() {
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Full Access"),
|
||||
selected_permissions_popup_name(&popup) == "Full Access",
|
||||
"expected permissions popup to open with Full Access selected: {popup}"
|
||||
);
|
||||
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"),
|
||||
selected_permissions_popup_name(&popup) == "Default",
|
||||
"expected navigation to land on Default before confirmation: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
@@ -8503,7 +8515,7 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() {
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Default"),
|
||||
selected_permissions_popup_name(&popup) == "Default",
|
||||
"expected permissions popup to open with Default selected: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
@@ -8611,7 +8623,8 @@ async fn permissions_selection_marks_smart_approvals_current_after_session_confi
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"),
|
||||
selected_permissions_popup_name(&popup) == "Smart Approvals"
|
||||
&& selected_permissions_popup_line(&popup).contains("(current)"),
|
||||
"expected SessionConfigured sync to select Smart Approvals in the popup: {popup}"
|
||||
);
|
||||
}
|
||||
@@ -8665,7 +8678,8 @@ async fn permissions_selection_marks_smart_approvals_current_with_custom_workspa
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
|
||||
assert!(
|
||||
selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"),
|
||||
selected_permissions_popup_name(&popup) == "Smart Approvals"
|
||||
&& selected_permissions_popup_line(&popup).contains("(current)"),
|
||||
"expected custom workspace-write details to keep Smart Approvals selected: {popup}"
|
||||
);
|
||||
}
|
||||
@@ -8695,18 +8709,15 @@ async fn permissions_selection_can_disable_smart_approvals() {
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
popup
|
||||
.lines()
|
||||
.any(|line| line.contains("Smart Approvals (current)") && line.contains('›')),
|
||||
selected_permissions_popup_name(&popup) == "Smart Approvals"
|
||||
&& selected_permissions_popup_line(&popup).contains("(current)"),
|
||||
"expected permissions popup to open with Smart Approvals selected: {popup}"
|
||||
);
|
||||
|
||||
move_permissions_popup_selection_to(&mut chat, "Default", KeyCode::Up);
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
popup
|
||||
.lines()
|
||||
.any(|line| line.contains("Default") && line.contains('›')),
|
||||
selected_permissions_popup_name(&popup) == "Default",
|
||||
"expected one Up from Smart Approvals to select Default: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
@@ -8752,18 +8763,14 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context
|
||||
chat.open_permissions_popup();
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
popup
|
||||
.lines()
|
||||
.any(|line| line.contains("(current)") && line.contains('›')),
|
||||
selected_permissions_popup_line(&popup).contains("(current)"),
|
||||
"expected permissions popup to open with the current preset selected: {popup}"
|
||||
);
|
||||
|
||||
move_permissions_popup_selection_to(&mut chat, "Smart Approvals", KeyCode::Down);
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
assert!(
|
||||
popup
|
||||
.lines()
|
||||
.any(|line| line.contains("Smart Approvals") && line.contains('›')),
|
||||
selected_permissions_popup_name(&popup) == "Smart Approvals",
|
||||
"expected one Down from Default to select Smart Approvals: {popup}"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
Reference in New Issue
Block a user