Add cross-browser and mobile-responsive E2E test matrix#800
Add cross-browser and mobile-responsive E2E test matrix#800Chris0Jeky merged 24 commits intomainfrom
Conversation
Expand playwright.config.ts with Firefox, WebKit, and mobile viewport projects (Pixel 7, iPhone 14) using Playwright's built-in device descriptors. Tag-based grep filters ensure: chromium runs all tests except @mobile, Firefox/WebKit run only @Cross-Browser tests, and mobile projects run only @mobile tests. Existing untagged tests continue to run on chromium unchanged. Refs #87
Four @mobile-tagged test scenarios covering: - Board navigation and column visibility on small screens - Card editing modal fitting within mobile viewport - Sidebar navigation accessibility on small screens - Capture modal usability on mobile viewport Tests validate actual responsive behavior (viewport bounds, element visibility, overflow prevention) rather than just re-running desktop tests at a smaller size. Refs #87
Five @cross-browser-tagged test scenarios covering: - Board creation, column/card workflow, and persistence after reload - Workspace navigation between Home, Boards, and Inbox views - Card edit modal open/close lifecycle - Capture hotkey submission and inbox routing - Filter panel keyboard shortcut toggle These tests run on Chromium, Firefox, and WebKit to catch browser- specific rendering and event handling differences. Refs #87
New reusable-e2e-cross-browser.yml runs Playwright tests across all five projects (chromium, firefox, webkit, mobile-chrome, mobile-safari) with fail-fast disabled so all browsers report independently. Wire into ci-nightly.yml for daily regression and ci-extended.yml for on-demand runs via "testing" label or manual dispatch. The PR merge gate (ci-required.yml) remains chromium-only smoke for fast feedback. Refs #87
Documents the E2E test tagging convention (@smoke, @Cross-Browser, @mobile, @quarantine), CI matrix strategy showing when each project runs, quarantine workflow (identify, tag, investigate, fix), SLA timelines by severity, and prevention guidelines. Refs #87
Add comprehensive section documenting browser projects, test tagging strategy, local run commands, CI configuration details, guidance for writing new tagged E2E tests, and link to the flaky test policy. Refs #87
Update reusable-e2e-smoke.yml to explicitly pass --project=chromium so the PR gate does not accidentally run all browser projects. Add top-level grepInvert for @quarantine tag in playwright.config.ts to exclude quarantined tests from all projects globally. Refs #87
Add reusable-e2e-cross-browser.yml entries to the topology diagram in the ci-required.yml header comment for ci-extended and ci-nightly. Refs #87
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Self-Review FindingsIssues Found and Fixed
Verified Correct (No Fix Needed)
Device Descriptors Verification
CI Runtime Assessment
|
…tine docs Remove matrix.project from Playwright browser cache key in the cross-browser workflow since all browsers are installed in every matrix job. Update flaky test policy to accurately describe the top-level grepInvert configuration. Refs #87
There was a problem hiding this comment.
Pull request overview
Expands the Playwright E2E setup to support a cross-browser + mobile-responsive test matrix, wiring it into nightly/extended CI while keeping the PR gate on a single Chromium project.
Changes:
- Added new
@cross-browserand@mobileE2E specs covering critical desktop journeys and mobile viewport behavior. - Updated Playwright config with 5 projects (desktop + mobile) using tag-based filtering and global quarantine exclusion.
- Added a reusable GitHub Actions workflow to run the full 5-project matrix (nightly +
testinglabel/manual), and pinned the smoke workflow to--project=chromium.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/taskdeck-web/tests/e2e/mobile-responsive.spec.ts | Adds @mobile viewport/responsiveness checks. |
| frontend/taskdeck-web/tests/e2e/cross-browser.spec.ts | Adds @cross-browser critical-journey coverage for Firefox/WebKit parity. |
| frontend/taskdeck-web/playwright.config.ts | Defines desktop/mobile projects with tag filters + global quarantine exclusion. |
| docs/testing/FLAKY_TEST_POLICY.md | Documents flaky test tagging/quarantine and remediation process. |
| docs/TESTING_GUIDE.md | Adds guidance for running the new project/tag matrix locally and in CI. |
| docs/STATUS.md | Records delivery of #87 and CI lane updates. |
| docs/IMPLEMENTATION_MASTERPLAN.md | Logs the matrix expansion as delivered. |
| .github/workflows/reusable-e2e-smoke.yml | Pins PR-gate E2E run to --project=chromium. |
| .github/workflows/reusable-e2e-cross-browser.yml | New reusable workflow running the 5-project Playwright matrix in parallel. |
| .github/workflows/ci-required.yml | Documentation comment update to include the new reusable workflow. |
| .github/workflows/ci-nightly.yml | Adds nightly job invoking the cross-browser matrix workflow. |
| .github/workflows/ci-extended.yml | Adds label/manual-triggered job invoking the cross-browser matrix workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Cache Playwright browsers | ||
| uses: actions/cache@v5 | ||
| with: | ||
| path: ~/.cache/ms-playwright | ||
| key: ms-playwright-${{ runner.os }}-${{ hashFiles('frontend/taskdeck-web/package-lock.json') }} | ||
|
|
There was a problem hiding this comment.
The Playwright browser cache key includes matrix.project, but the workflow installs the full Playwright browser set (npx playwright install --with-deps) for every job. This will create 5 separate caches with identical contents and reduce cache hit rates / waste storage. Consider removing matrix.project from the cache key (or adding a restore key) so all matrix jobs can reuse the same cached browsers.
| - name: Install Playwright browsers | ||
| working-directory: frontend/taskdeck-web | ||
| run: npx playwright install --with-deps | ||
|
|
There was a problem hiding this comment.
Each matrix job installs all Playwright browsers (npx playwright install --with-deps) even though the job only runs a single --project. This increases runtime and network usage for the nightly/extended matrix. Consider installing only the required browser per project (e.g., chromium for chromium/mobile-chrome, firefox for firefox, webkit for webkit/mobile-safari).
| test('@mobile sidebar navigation should remain accessible on small screen', async ({ page }) => { | ||
| await page.goto('/workspace/home') | ||
| await expect(page).toHaveURL(/\/workspace\/home$/) | ||
|
|
||
| const viewportSize = page.viewportSize() | ||
| expect(viewportSize).not.toBeNull() | ||
| expect(viewportSize!.width).toBeLessThan(500) | ||
|
|
||
| // Home heading should be visible | ||
| await expect(page.getByRole('heading', { name: 'Home', exact: true })).toBeVisible() | ||
|
|
||
| // Navigate to boards workspace — use direct URL since sidebar may be | ||
| // collapsed or behind a hamburger on mobile | ||
| await page.goto('/workspace/boards') | ||
| await expect(page).toHaveURL(/\/workspace\/boards$/) | ||
| await expect(page.getByRole('button', { name: '+ New Board' })).toBeVisible() |
There was a problem hiding this comment.
This test title says "sidebar navigation" but the assertions only use direct page.goto(...) navigation and never verify that the sidebar/hamburger UI is accessible or functional on mobile. Either rename the test to match what it actually verifies (workspace route navigation + overflow) or add an interaction/assertion that exercises the sidebar navigation control on small screens.
| The Playwright config excludes `@quarantine` from all CI projects via a top-level `grepInvert` in `playwright.config.ts`. The test still runs locally for debugging (pass `--grep="@quarantine"` explicitly to override). | ||
|
|
||
| The top-level exclusion is already configured: | ||
|
|
||
| ```typescript | ||
| // playwright.config.ts (top level) | ||
| grepInvert: /@quarantine/, |
There was a problem hiding this comment.
The doc suggests adding grepInvert in the top-level use block, but grepInvert is a Playwright config option (like grep) and is not part of the use options object. Update the guidance to say to add it at the top level of defineConfig({...}) (or per-project), matching how it’s configured in playwright.config.ts.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive cross-browser and mobile E2E testing matrix using Playwright, including new test suites, a flaky test policy, and updated CI workflows. While the expansion significantly improves test coverage, several issues were identified regarding the Playwright configuration and test implementation. Specifically, the grepInvert property in the chromium project incorrectly overrides the global quarantine filter, and the documentation incorrectly describes the placement of this property. Additionally, the mobile-specific tests rely on desktop-centric interactions like keyboard shortcuts and direct URL navigation, which bypasses the validation of the mobile UI. There is also significant code duplication between the new test files that should be refactored into shared utilities.
| use: { ...devices['Desktop Chrome'] }, | ||
| /* Default project: runs all tests except @mobile-only scenarios. | ||
| * Existing untagged tests continue to run here unchanged. */ | ||
| grepInvert: /@mobile/, |
There was a problem hiding this comment.
The grepInvert property at the project level overrides the global grepInvert defined on line 56. Consequently, tests tagged with @quarantine will not be excluded from the chromium project, which is the primary project for PR gates. You should combine the patterns to ensure the quarantine policy is enforced.
| grepInvert: /@mobile/, | |
| grepInvert: /@mobile|@quarantine/, |
|
|
||
| The Playwright config excludes `@quarantine` from all CI projects via a top-level `grepInvert` in `playwright.config.ts`. The test still runs locally for debugging (pass `--grep="@quarantine"` explicitly to override). | ||
|
|
||
| The top-level exclusion is already configured: |
There was a problem hiding this comment.
In Playwright, grepInvert is a top-level configuration property, not a property within the use block. The documentation should be corrected to reflect the actual configuration structure.
| The top-level exclusion is already configured: | |
| To add quarantine exclusion to all projects, add this to playwright.config.ts as a top-level property: |
| const captureText = `Mobile capture ${Date.now()}` | ||
|
|
||
| // Open capture modal via keyboard shortcut | ||
| await page.keyboard.press('Control+Shift+C') |
There was a problem hiding this comment.
Using a keyboard shortcut like Control+Shift+C in a mobile-responsive test is unrealistic as mobile devices typically lack these physical keys. To improve the fidelity of this @mobile test, consider triggering the capture modal via the UI (e.g., a button in the header or navigation menu) instead of a keyboard event.
|
|
||
| // Navigate to boards workspace — use direct URL since sidebar may be | ||
| // collapsed or behind a hamburger on mobile | ||
| await page.goto('/workspace/boards') |
There was a problem hiding this comment.
Navigating via page.goto in a mobile-specific test bypasses the verification of the mobile navigation UI (such as the hamburger menu). Since this test is specifically for mobile responsiveness, it should verify that the user can successfully navigate using the UI elements available on a small screen.
| async function createBoard(page: Page, boardName: string) { | ||
| await page.goto('/workspace/boards') | ||
| await expect(page.getByRole('button', { name: '+ New Board' })).toBeVisible() | ||
| await page.getByRole('button', { name: '+ New Board' }).click() | ||
| await page.getByPlaceholder('Board name').fill(boardName) | ||
| await page.getByRole('button', { name: 'Create', exact: true }).click() | ||
| await expect(page).toHaveURL(/\/workspace\/boards\/[a-f0-9-]+$/) | ||
| await expect(page.getByRole('heading', { name: boardName })).toBeVisible() | ||
| } | ||
|
|
||
| async function addColumn(page: Page, columnName: string) { | ||
| await page.getByRole('button', { name: '+ Add Column' }).click() | ||
| await page.getByPlaceholder('Column name').fill(columnName) | ||
| await page.getByRole('button', { name: 'Create', exact: true }).click() | ||
| await expect(page.getByRole('heading', { name: columnName, exact: true })).toBeVisible() | ||
| } | ||
|
|
||
| function columnByName(page: Page, columnName: string) { | ||
| return page | ||
| .locator('[data-column-id]') | ||
| .filter({ has: page.getByRole('heading', { name: columnName, exact: true }) }) | ||
| .first() | ||
| } | ||
|
|
||
| async function addCard(page: Page, columnName: string, cardTitle: string) { | ||
| const column = columnByName(page, columnName) | ||
| await column.getByRole('button', { name: 'Add Card' }).click() | ||
| const addCardInput = column.getByPlaceholder('Enter card title...') | ||
| await expect(addCardInput).toBeVisible() | ||
| await addCardInput.fill(cardTitle) | ||
| const createCardResponse = page.waitForResponse((response) => | ||
| response.request().method() === 'POST' | ||
| && /\/api\/boards\/[a-f0-9-]+\/cards$/i.test(response.url()) | ||
| && response.ok()) | ||
| await column.getByRole('button', { name: 'Add', exact: true }).click() | ||
| await createCardResponse | ||
| await expect( | ||
| page.locator('[data-card-id]').filter({ hasText: cardTitle }).first(), | ||
| ).toBeVisible() | ||
| } |
There was a problem hiding this comment.
These helper functions (createBoard, addColumn, columnByName, addCard) are duplicated exactly in mobile-responsive.spec.ts. To improve maintainability and adhere to DRY principles, these should be extracted into a shared utility file within the tests/e2e/support/ directory.
Adversarial Review - PR #800CRITICAL IssuesC1: CI PR gate is failing - new cross-browser test breaks existing CI (cross-browser.spec.ts:108)
C2: IMPLEMENTATION_MASTERPLAN.md numbering is broken (docs/IMPLEMENTATION_MASTERPLAN.md:1218)
HIGH IssuesH1: Duplicated helper functions across spec files create maintenance burden (cross-browser.spec.ts:23-62, mobile-responsive.spec.ts:22-61)
H2: Mobile capture test relies on Control+Shift+C keyboard shortcut (mobile-responsive.spec.ts:175)
H3: @Cross-Browser tests run on chromium in PR gate by design, but this is underdocumented (reusable-e2e-smoke.yml:76)
MEDIUM IssuesM1: Mobile sidebar test does not actually test sidebar interaction (mobile-responsive.spec.ts:138-165)
M2: Conditional assertions weaken mobile tests (mobile-responsive.spec.ts:116-125, 128-130)
M3: @Cross-Browser capture hotkey test uses Control+Enter which behaves differently on macOS/WebKit (cross-browser.spec.ts:143)
M4: Cross-browser workflow installs all browsers for every matrix job (reusable-e2e-cross-browser.yml:72)
LOW IssuesL1: Flaky test policy SLA timelines may be unrealistic (docs/testing/FLAKY_TEST_POLICY.md:107-111)
L2: Missing @smoke tag documentation alignment (docs/testing/FLAKY_TEST_POLICY.md:24, docs/TESTING_GUIDE.md:257)
Summary
The CI failure (C1) is the highest priority. The PR gate is red and this PR cannot be merged until the failing test is fixed. |
Extract createBoard, addColumn, columnByName, and addCard helpers to a shared boardUiHelpers.ts module. These were duplicated identically across cross-browser.spec.ts and mobile-responsive.spec.ts. The addCard helper now uses a 15s timeout for card visibility to handle slow CI re-render timing that caused the PR gate failure. Refs #87
…mpact Replace duplicated local helpers with imports from boardUiHelpers.ts. Add note in file header that @Cross-Browser tests run in PR gate on chromium and to be mindful of count. Refs #87
…onditional assertions - Import board helpers from shared boardUiHelpers.ts instead of duplicating them locally. - Rename sidebar navigation test to accurately reflect what it validates (workspace views render correctly, not sidebar interaction). - Remove conditional if-guards around modal assertions so regressions in modal rendering are caught instead of silently passing. Refs #87
Add comment warning that @Cross-Browser tests run in the PR gate on chromium and to keep count lean for fast feedback. Refs #87
Renumber from 20 to 13 to follow the previous item 12 in the Operating Notes list. The jump to 20 was a numbering error. Refs #87
- Click card title instead of card body to avoid drag-handle intercept - Combine @mobile|@quarantine in chromium project grepInvert to prevent project-level override from losing global quarantine exclusion
Integrates main branch changes including visual regression tests while preserving cross-browser E2E testing functionality. Resolves: - ci-extended.yml: Keep both e2e-cross-browser and visual-regression jobs - TESTING_GUIDE.md: Keep both cross-browser and visual regression sections
Add exact: true to Edit Card heading selector to avoid matching both the modal heading and cards containing "Edit Card" in their title.
Take main's STATUS.md and add the 3 cross-browser/mobile E2E additions from the PR branch.
Update STATUS.md with post-merge housekeeping entry, recertified test counts (4279 backend + 2245 frontend = ~6500+), and delivered status for distributed caching, SSO/OIDC/MFA, and staged rollout. Update TESTING_GUIDE.md with current test counts and new test categories (resilience, MFA/OIDC, telemetry, cache). Update IMPLEMENTATION_MASTERPLAN.md marking all expansion wave items as delivered. Extend AUTHENTICATION.md with OIDC/SSO login flow, MFA setup/verify/ recovery, API key management, and account linking endpoints. Update MANUAL_TEST_CHECKLIST.md: mark all PRs as merged, add testing tasks for error tracking (#811), MCP HTTP transport (#819), distributed caching (#805), and resilience tests (#820).
Summary
Implements #87 (TST-02): Cross-browser and mobile-responsive E2E matrix expansion.
@cross-browser-tagged critical journey tests (board workflow, navigation, card edit, capture, filter)@mobile-tagged responsive behavior tests (board navigation, card editing modal, sidebar navigation, capture modal)reusable-e2e-cross-browser.ymlruns all 5 projects in parallel matrix; wired into nightly and ci-extended (testing label/manual)reusable-e2e-smoke.ymlpinned to--project=chromiumfor fast PR feedbackgrepInvert: /@quarantine/excludes flaky tests from all projectsdocs/testing/FLAKY_TEST_POLICY.mdwith tagging convention, quarantine process, SLA timelines, prevention guidelinesTest plan
npx playwright test --project=chromiumruns all existing tests (no @mobile exclusion regression)npx playwright test --project=firefox --grep="@cross-browser"runs only tagged testsnpx playwright test --project=mobile-chrome --grep="@mobile"runs only mobile tests--project=chromiumflagCloses #87