Repository navigation
test(e2e): update four stale e2e specs to match intentional product changes - #30115
Conversation
|
No actionable comments were generated in the recent review.
|
There was a problem hiding this comment.
🧹 Nitpick comments: 1
tests/e2e/os-opened-documents.spec.ts: 1
39-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRetain ARIA grid and cell assertions for CSV/TSV documents.
getByTestId('csv-grid').getByText(...)only checks that the text remains under the test-id element. It can pass if the element losesrole="grid"or its cell structure while the text remains. This OS-opened-document path no longer checks those semantics. Keep the test ID for scoping, but use grid and gridcell role assertions for the result cells.Suggested fix
const table = page.getByTestId('csv-grid') + if (extension !== 'md') { + await expect(table).toHaveRole('grid') + } const coldResult = extension === 'md' ? page.getByRole('heading', { name: 'Cold result' }) - : table.getByText('Cold result', { exact: true }) + : table.getByRole('gridcell', { name: 'Cold result', exact: true }) const warmResult = extension === 'md' ? page.getByRole('heading', { name: 'Warm result' }) - : table.getByText('Warm result', { exact: true }) + : table.getByRole('gridcell', { name: 'Warm result', exact: true })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b43ab0a1-50d8-473e-922b-cf8fa9a84d77
📥 Commits
Reviewing files that changed from the base of the PR and between 100b6b1 and 1895eb1af63c3e28b6dbc30d2e48ecb9179e7cf8.
📒 Files selected for processing: 8
tests/e2e/add-project-error-toast-stacking.spec.tstests/e2e/ai-vault-panel-search.spec.tstests/e2e/linear-url-workspace-entry.spec.tstests/e2e/os-opened-documents.spec.tstests/e2e/paired-cmd-j-host-qualified-tabs.spec.tstests/e2e/profile-state-save-delay.spec.tstests/e2e/workspace-emoji-picker.spec.tstests/e2e/worktree-jump-palette-filter.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
1895eb1 to
33fe472
Compare
|
Pushed a follow-up for CodeRabbit's nitpick and the formatter:
It is a force-push because the first commit's author email was changed to my GitHub noreply address. Its content is unchanged. Targeted run of both specs on my fork: 4/4 passed (md, csv and tsv cases plus the Cmd+J spec), https://github.com/SimonSQLML/orca/actions/runs/39490755521 For context, #30897 (merged today) fixed nine other stale full-suite specs. It shares no files with this PR, and this branch still merges cleanly into current main. |
|
@brennanb2025 @AmethystLiang could one of you take a look when you have a moment? The workflows on the current head are waiting for approval ( For context: after #30897, the nightly full run on main at 3086fe2 (https://github.com/stablyai/orca/actions/runs/39642002390) still fails 16 spec files, and 8 of them are the ones this PR updates. #30897 left those eight to this PR. The branch merges cleanly into current main. |
Align still-stale e2e expectations with shipped product behavior: - the Cmd+J placeholder names workspaces and tasks - the worktree meta dialog is titled "Workspace details" - OS-opened CSV/TSV files are editable and render as an ARIA grid - the delayed-save notice carries the app toaster's close button Fixes stablyai#30114 Co-authored-by: Neil <neil@stably.ai>
33fe472 to
0fba66c
Compare
|
Thanks @nwparker for rebasing this onto current main and tightening the assertions! |
Since stablyai#33270, Create worktree reads Linear status from the repo's host (linear:status IPC) instead of the store, so the store-only fixture left Linear disconnected in the composer and the pasted URL was never looked up. Stub the linear:status handler with the same connected status.
ELI5
Orca has automated tests that click through the app the way a person would. Some of the app's wording and structure changed on purpose: a dialog is now called "Workspace details", the Cmd+J search box mentions workspaces and tasks, and spreadsheet files open in an editable grid. A few tests were still looking for the old wording, so they failed on every run and could hide real breakages. This PR updates those tests to look for what the app shows today. The app itself does not change.
What Changed
Tests only, 4 files. Each one now matches what the shipped product does:
tests/e2e/linear-url-workspace-entry.spec.tsandtests/e2e/workspace-emoji-picker.spec.ts: the Cmd+J placeholder is nowSearch chats, terminals, workspaces, tasks, settings, and actions...(worktree-jump-palette-surface.tsx).tests/e2e/workspace-emoji-picker.spec.ts: the worktree meta dialog is now titledWorkspace details(WorktreeMetaDialogDraft.tsx). It used to beEdit Worktree Details.tests/e2e/os-opened-documents.spec.ts: CSV/TSV files opened from the OS are editable. They render asrole="grid"namedCSV editorwithgridcellcells (CsvGrid.tsx), not as atablewithcellcells.tests/e2e/profile-state-save-delay.spec.ts: the app's<Toaster closeButton>(App.tsx) adds a close button to every toast. Before, the spec expected 0 buttons. Now it asserts that the toast's only button is sonner's close button (data-close-button). Strict mode still fails if an action button is ever added.Before: these specs failed on
mainevery run. After: they pass.Why
Specs that always fail make the suite noisy and hide real regressions. Every new selector was checked against the current product source, so this is a test-only fix. Matching on the ARIA role and the accessible name checks the user-facing contract directly, so no extra test-ids are needed.
I rebased the contributor's original change onto current
mainand dropped the hunks thatmainalready fixed: toaster z-index, the paired Cmd+J placeholder, the palette filter placeholder, and the ai-vault unavailable-reason spec (rewritten in #33234).Linked Issue
Fixes #30114
Visual Proof
N/A. This only changes e2e test expectations. The product UI and behavior are the same.
Testing
On macOS, background launch only (
ORCA_BACKGROUND_LAUNCH=1):origin/main, the in-scope tests fail: the Cmd+J placeholder isn't found, the CSV/TSVcoldResultisn't visible, the toast has 1 button where the spec expects 0, and the details dialog isn't found.main: 7 passed, 1 failed. The failure islinear-url-workspace-entry.spec.ts:130("pasting into the composer selects the Linear issue without ArrowDown"):window.__orcaTestReleaseLinearLookupis never installed. It fails the same way onmain. It is not stale copy and is out of scope here.oxlint,oxfmt --check, ast-grep, the anti-slop and code-quality oxlint configs, andcheck-changed-code-quality.mjsare clean on the touched files.Review
Reviewed independently by two agents and a verify pass. All approved with no blockers.
Agent skill upstream boundary
Notes
Test-only change. No impact on security, cross-platform behavior, SSH/remote, mobile, backwards compatibility, or performance.
Original fix by @SimonSQLML in #30115.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)