Vibe 298 #158
Vibe 298 #158
Conversation
Configure GitHub Actions nightly workflow to run comprehensive tests at 2:00 AM UTC daily. Update PR E2E workflow to exclude nightly-tagged tests using --grep-invert "@nightly". Add focused E2E testing guidance to CLAUDE.md covering test patterns, selectors, accessibility integration, and nightly test tagging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Instead of adding --grep-invert "@nightly" in workflow files, configure it at the script level: - test:e2e now excludes nightly tests by default (for local dev and PR runs) - test:e2e:all runs all tests including nightly (for nightly workflow) This provides better developer experience as running yarn test:e2e locally excludes nightly tests by default. Updated CLAUDE.md documentation to reflect the new commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Tagged 247 tests across 23 test files with @nightly to exclude them from PR runs: - Error handling tests (400 errors, validation errors) - Authentication edge cases (multiple roles, rejected users, session management) - Extensive Welsh language tests (beyond basic translation checks) - Detailed keyboard navigation tests - Screen reader/ARIA tests - Responsive design tests - Database verification tests - Edge cases and boundary conditions Happy path tests remain untagged and will run on every PR for fast feedback. Nightly tests will run comprehensive coverage via test:e2e:all command. Files modified: - Authentication: cft-idam, sso, sign-in, cookie-management - Admin: admin-dashboard, system-admin-dashboard, account-home - Data management: add-*, reference-data-upload, manual-upload, remove-publication - User-facing: landing-page, search, view-option, courts-tribunals-list, summary-of-publications - API: blob-ingestion - Other: email-subscriptions, create-media-account, page-structure, 400-error-page 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds a scheduled nightly GitHub Actions workflow (daily 02:00 UTC) that runs E2E tests with Postgres/Redis, Prisma migrations and seeding, Playwright execution and artifact uploads; appends Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as GitHub Actions Scheduler
participant Runner as Actions Runner
participant Services as Postgres & Redis
participant Node as Node/Corepack + Yarn
participant Prisma as Prisma CLI/Client
participant Playwright as Playwright Test Runner
participant Artifacts as Actions Artifacts
Note over Scheduler,Runner: Nightly trigger (daily @ 02:00 UTC) or manual
Scheduler->>Runner: start workflow
Runner->>Runner: checkout repo, restore caches, setup Node/Corepack
Runner->>Node: install dependencies, build packages
Runner->>Services: start Postgres & Redis containers
Services-->>Runner: services healthy
Runner->>Prisma: generate client, migrate, seed, verify seeded artefacts
Prisma-->>Runner: database ready
Runner->>Playwright: prepare browsers (cache/install), run E2E tests
Playwright-->>Runner: test results, traces, logs
Runner->>Artifacts: upload reports, test-results, traces, app logs
Note over Runner,Artifacts: workflow completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Comment |
🎭 Playwright E2E Test Results145 tests 145 ✅ 10m 25s ⏱️ Results for commit 8ee2484. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
e2e-tests/tests/create-media-account.spec.ts (2)
237-245: Reconsider @nightly tag for Welsh language test.The learnings indicate that Welsh translations are required for all user-facing text and must be tested regularly. Tagging this test as @nightly means Welsh support will only be validated once daily rather than on every PR, which could allow Welsh translation regressions to slip through.
Based on learnings, Welsh content should be tested with the
?lng=cyquery parameter on every PR to ensure compliance with the mandatory bilingual requirement.
51-64: Update email validation error message to match Welsh translation and accurately describe the validation failure.The English error message ("Email address field must be populated") contradicts the Welsh translation ("Nodwch gyfeiriad e-bost yn y fformat cywir, e.e. [email protected]" - Enter email address in correct format). Since the test enters "notanemail" (a populated field with invalid format), the error message should indicate the format requirement, not that the field is empty. Update line 62 to expect:
"There is a problem - Enter a valid email address, e.g. [email protected]"Note: The application code's English translation (libs/public-pages/src/pages/create-media-account/en.ts line 22) also needs to be corrected to match the Welsh translation.
🧹 Nitpick comments (9)
e2e-tests/tests/create-media-account.spec.ts (1)
37-37: Consider validation test coverage in regular PR runs.The current tagging strategy moves all validation tests (empty form, email, file type, file size, terms) to nightly runs, leaving only the happy path and basic display tests for regular PR runs. This reduces immediate feedback on validation logic regressions.
Consider keeping at least one or two critical validation tests (e.g., email validation, terms acceptance) in the regular test suite to maintain rapid feedback on common validation scenarios while still reducing overall PR test time.
Also applies to: 51-51, 66-66, 89-89, 113-113
e2e-tests/tests/landing-page.spec.ts (1)
229-249: Keyboard navigation test is a good fit for nightly; tab limit may need tuningAdding
@nightlyhere is sensible given the flakiness risk of tab-walking tests. Be aware the hard-codedfor (let i = 0; i < 15 && !focused; i++)may need adjusting if more focusable elements are added before the Continue button.e2e-tests/tests/account-home.spec.ts (3)
50-59: Nightly-tagging styling-focused tests is fine, but they diverge from new E2E guidanceAdding
@nightlyto these tests (flex layout, heading colour/underline, tile background colour, borders, box shadow, cursor, equal heights, heading font weights/colours, text decoration, etc.) keeps existing coverage while moving brittle CSS assertions out of PR workflows.However, the new CLAUDE.md “E2E Testing with Playwright” section explicitly advises against asserting visual styling such as colours, backgrounds, and other design details. Over time, it’d be good to either:
- Rewrite these to focus on semantic structure and behaviour (roles, labels, interactions), or
- Move exact-appearance checks into a dedicated visual-regression approach if still required.
For now, the change to titles only is safe and the nightly scoping is a good interim compromise.
Also applies to: 94-108, 176-256, 259-316
319-359: Responsive layout tests are reasonable nightly checksThe desktop layout assertions (flex-direction row, gap size, y-coordinate alignment across tiles) are more layout-sensitive and thus brittle, so running them only as
@nightlyis a sensible choice. The behaviour they verify is useful, but if they become flaky, consider focusing on high-level invariants (e.g. presence/ordering of tiles) rather than bounding-box equality.
419-515: Accessibility and keyboard tests are good candidates for nightly, with one minor assertion tweakMoving the heading-hierarchy, descriptive-content, colour-contrast, semantic structure, keyboard navigation, and focus-indicator tests under
@nightlypreserves important accessibility coverage while reducing PR runtime.One minor improvement opportunity: in
should allow keyboard navigation through all tiles @nightly, the final assertion only checksexpect(foundTiles).toBeGreaterThan(0);. Given the test name, assertingfoundTiles === countwould more directly guarantee that focus reaches each tile at least once.e2e-tests/tests/admin-dashboard.spec.ts (1)
85-125: Keyboard navigation tests belong in nightly; assertion could be stricterThe keyboard navigation and focus-indicator checks are good to run nightly, since they can be timing- and DOM-order‑sensitive. The implementation is safe, but note that
expect(foundTiles).toBeGreaterThan(0);doesn’t actually ensure all tiles received focus even though the test name suggests it. If stability allows, consider asserting that all tiles were focused at least once.CLAUDE.md (1)
367-432: E2E Playwright guidance matches the implementation; note future cleanup vs legacy testsThe new E2E section clearly documents:
- Location/naming of specs and
@nightlytagging.- Sequential test pattern including Welsh and inline Axe checks.
- Preferred selectors and “DO NOT test” styling guidance.
- The split between
yarn test:e2eandyarn test:e2e:all, which lines up with the updated e2e-tests/package.json.This is a solid reference for new tests. One minor observation: several existing specs (e.g. account-home/admin-dashboard styling tests) still assert exact colours/borders/box-shadows, which the new guidance discourages. It’s fine to leave them for now—especially as nightly-only—but this section sets a good direction for gradually refactoring those towards more semantic/behavioural checks.
docs/tickets/VIBE-298/ticket.md (1)
1-26: Ticket description matches the PR; consider formatting bare URLsThe ticket succinctly captures the two main tasks (nightly pipeline and CLAUDE.md guidance) and aligns with the implemented changes. If you want to satisfy markdownlint, wrap any bare URLs in proper Markdown link syntax, e.g.
[VIBE-298](https://tools.hmcts.net/jira/browse/VIBE-298)instead of a raw URL.docs/tickets/VIBE-298/plan.md (1)
31-38: Add language identifier to fenced code block.The fenced code block showing the file structure should specify a language identifier for proper rendering and to satisfy markdown linting rules.
Apply this diff:
-``` +```text .github/workflows/ ├── nightly.yml # New nightly scheduled pipeline (uses test:e2e:all) ├── e2e.yml # No changes (uses test:e2e) e2e-tests/package.json # Add test:e2e:all script, update test:e2e to exclude nightly CLAUDE.md # Add focused E2E testing section</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7746ff31bb7a6c508089f2209a3b42a95af307b5 and 9ee02a43adc2866cc7c99c3f94c3b17b2ae3af64. </details> <details> <summary>📒 Files selected for processing (29)</summary> * `.github/workflows/nightly.yml` (1 hunks) * `CLAUDE.md` (1 hunks) * `docs/tickets/VIBE-298/plan.md` (1 hunks) * `docs/tickets/VIBE-298/tasks.md` (1 hunks) * `docs/tickets/VIBE-298/ticket.md` (1 hunks) * `e2e-tests/package.json` (1 hunks) * `e2e-tests/tests/400-error-page.spec.ts` (3 hunks) * `e2e-tests/tests/account-home.spec.ts` (25 hunks) * `e2e-tests/tests/add-jurisdiction.spec.ts` (14 hunks) * `e2e-tests/tests/add-region.spec.ts` (19 hunks) * `e2e-tests/tests/add-sub-jurisdiction.spec.ts` (15 hunks) * `e2e-tests/tests/admin-dashboard.spec.ts` (9 hunks) * `e2e-tests/tests/api/blob-ingestion.spec.ts` (17 hunks) * `e2e-tests/tests/cft-idam/cft-idam.spec.ts` (7 hunks) * `e2e-tests/tests/cookie-management.spec.ts` (6 hunks) * `e2e-tests/tests/courts-tribunals-list.spec.ts` (17 hunks) * `e2e-tests/tests/create-media-account.spec.ts` (8 hunks) * `e2e-tests/tests/email-subscriptions.spec.ts` (2 hunks) * `e2e-tests/tests/landing-page.spec.ts` (9 hunks) * `e2e-tests/tests/manual-upload.spec.ts` (10 hunks) * `e2e-tests/tests/page-structure.spec.ts` (3 hunks) * `e2e-tests/tests/reference-data-upload.spec.ts` (28 hunks) * `e2e-tests/tests/remove-publication.spec.ts` (3 hunks) * `e2e-tests/tests/search.spec.ts` (8 hunks) * `e2e-tests/tests/sign-in.spec.ts` (12 hunks) * `e2e-tests/tests/sso/sso.spec.ts` (5 hunks) * `e2e-tests/tests/summary-of-publications.spec.ts` (7 hunks) * `e2e-tests/tests/system-admin-dashboard.spec.ts` (7 hunks) * `e2e-tests/tests/view-option.spec.ts` (5 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (3)</summary> <details> <summary>**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `**/*.{ts,tsx}`: TypeScript variables must use camelCase (e.g., `userId`, `caseDetails`, `documentId`). Booleans should use `is/has/can` prefix (e.g., `isActive`, `hasAccess`, `canEdit`). > Classes and interfaces must use PascalCase (e.g., `UserService`, `CaseRepository`). DO NOT use `I` prefix for interfaces. > Constants must use SCREAMING_SNAKE_CASE (e.g., `MAX_FILE_SIZE`, `DEFAULT_TIMEOUT`). > Module ordering: constants at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom. > Always add `.js` extension to relative imports (e.g., `import { foo } from "./bar.js"`). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files. > Use workspace aliases (`@hmcts/*`) for imports instead of relative paths across packages. > Database queries must be parameterized using Prisma (never raw SQL with string concatenation). > Never put sensitive data in logs. > Don't add comments unless meaningful - explain why something is done, not what is done. > Favour functional style - use simple functions. Don't use a class unless you have shared state. > Data should be immutable by default - use const and avoid mutations to ensure predictable state. > Functions should have no side effects - avoid modifying external state or relying on mutable data. Files: - `e2e-tests/tests/remove-publication.spec.ts` - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/page-structure.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/summary-of-publications.spec.ts` - `e2e-tests/tests/courts-tribunals-list.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/system-admin-dashboard.spec.ts` - `e2e-tests/tests/email-subscriptions.spec.ts` - `e2e-tests/tests/account-home.spec.ts` - `e2e-tests/tests/search.spec.ts` - `e2e-tests/tests/reference-data-upload.spec.ts` - `e2e-tests/tests/landing-page.spec.ts` - `e2e-tests/tests/sign-in.spec.ts` - `e2e-tests/tests/api/blob-ingestion.spec.ts` - `e2e-tests/tests/manual-upload.spec.ts` - `e2e-tests/tests/400-error-page.spec.ts` - `e2e-tests/tests/sso/sso.spec.ts` - `e2e-tests/tests/cft-idam/cft-idam.spec.ts` - `e2e-tests/tests/admin-dashboard.spec.ts` - `e2e-tests/tests/view-option.spec.ts` - `e2e-tests/tests/cookie-management.spec.ts` </details> <details> <summary>**/*.{ts,tsx,js,jsx}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `**/*.{ts,tsx,js,jsx}`: Files and directories must use kebab-case (e.g., `user-service.ts`, `case-management/`). > Never use CommonJS (`require()`, `module.exports`). Use ES modules (`import`/`export`) exclusively. Files: - `e2e-tests/tests/remove-publication.spec.ts` - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/page-structure.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/summary-of-publications.spec.ts` - `e2e-tests/tests/courts-tribunals-list.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/system-admin-dashboard.spec.ts` - `e2e-tests/tests/email-subscriptions.spec.ts` - `e2e-tests/tests/account-home.spec.ts` - `e2e-tests/tests/search.spec.ts` - `e2e-tests/tests/reference-data-upload.spec.ts` - `e2e-tests/tests/landing-page.spec.ts` - `e2e-tests/tests/sign-in.spec.ts` - `e2e-tests/tests/api/blob-ingestion.spec.ts` - `e2e-tests/tests/manual-upload.spec.ts` - `e2e-tests/tests/400-error-page.spec.ts` - `e2e-tests/tests/sso/sso.spec.ts` - `e2e-tests/tests/cft-idam/cft-idam.spec.ts` - `e2e-tests/tests/admin-dashboard.spec.ts` - `e2e-tests/tests/view-option.spec.ts` - `e2e-tests/tests/cookie-management.spec.ts` </details> <details> <summary>**/*.{test,spec}.ts</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Test files must be co-located with source files using the pattern `*.test.ts` or `*.spec.ts` and use Vitest. Files: - `e2e-tests/tests/remove-publication.spec.ts` - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/page-structure.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/summary-of-publications.spec.ts` - `e2e-tests/tests/courts-tribunals-list.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/system-admin-dashboard.spec.ts` - `e2e-tests/tests/email-subscriptions.spec.ts` - `e2e-tests/tests/account-home.spec.ts` - `e2e-tests/tests/search.spec.ts` - `e2e-tests/tests/reference-data-upload.spec.ts` - `e2e-tests/tests/landing-page.spec.ts` - `e2e-tests/tests/sign-in.spec.ts` - `e2e-tests/tests/api/blob-ingestion.spec.ts` - `e2e-tests/tests/manual-upload.spec.ts` - `e2e-tests/tests/400-error-page.spec.ts` - `e2e-tests/tests/sso/sso.spec.ts` - `e2e-tests/tests/cft-idam/cft-idam.spec.ts` - `e2e-tests/tests/admin-dashboard.spec.ts` - `e2e-tests/tests/view-option.spec.ts` - `e2e-tests/tests/cookie-management.spec.ts` </details> </details><details> <summary>🧠 Learnings (8)</summary> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to /pages//*.ts : Every page must support both English and Welsh with separateenandcycontent objects in the controller, and Welsh content must be tested with?lng=cyquery parameter.**Applied to files:** - `e2e-tests/tests/remove-publication.spec.ts` - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/summary-of-publications.spec.ts` - `e2e-tests/tests/courts-tribunals-list.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/account-home.spec.ts` - `e2e-tests/tests/search.spec.ts` - `e2e-tests/tests/reference-data-upload.spec.ts` - `e2e-tests/tests/landing-page.spec.ts` - `e2e-tests/tests/sign-in.spec.ts` - `e2e-tests/tests/manual-upload.spec.ts` - `e2e-tests/tests/cft-idam/cft-idam.spec.ts` - `CLAUDE.md` - `e2e-tests/tests/view-option.spec.ts` - `e2e-tests/tests/cookie-management.spec.ts` </details> <details> <summary>📚 Learning: 2025-11-27T14:18:22.922Z</summary>Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 137
File: e2e-tests/tests/create-media-account.spec.ts:51-64
Timestamp: 2025-11-27T14:18:22.922Z
Learning: For the create-media-account form in libs/public-pages, the English email validation error message (errorEmailInvalid) should be: "There is a problem - Enter a valid email address, e.g. nameexample.com" to match the Welsh translation and clearly indicate the format requirement rather than suggesting the field is empty.**Applied to files:** - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/sign-in.spec.ts` </details> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Don't skip accessibility testing - WCAG 2.2 AA is mandatory.**Applied to files:** - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/reference-data-upload.spec.ts` - `e2e-tests/tests/manual-upload.spec.ts` - `e2e-tests/tests/400-error-page.spec.ts` </details> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Don't skip Welsh translations - required for all user-facing text.**Applied to files:** - `e2e-tests/tests/add-region.spec.ts` - `e2e-tests/tests/create-media-account.spec.ts` - `e2e-tests/tests/add-sub-jurisdiction.spec.ts` - `e2e-tests/tests/add-jurisdiction.spec.ts` - `e2e-tests/tests/account-home.spec.ts` - `e2e-tests/tests/landing-page.spec.ts` </details> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to libs/*/package.json : All test packages must use"test": "vitest run"script to run tests.**Applied to files:** - `e2e-tests/package.json` </details> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/*.{test,spec}.ts : Test files must be co-located with source files using the pattern*.test.tsor*.spec.tsand use Vitest.**Applied to files:** - `e2e-tests/tests/search.spec.ts` - `CLAUDE.md` </details> <details> <summary>📚 Learning: 2025-11-27T09:48:13.010Z</summary>Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: libs/api/src/blob-ingestion/validation.ts:156-163
Timestamp: 2025-11-27T09:48:13.010Z
Learning: In libs/api/src/blob-ingestion/validation.ts, the permissive date validation in isValidISODate and isValidISODateTime functions is expected behavior and should not be flagged for stricter validation.**Applied to files:** - `e2e-tests/tests/api/blob-ingestion.spec.ts` </details> <details> <summary>📚 Learning: 2025-12-01T11:31:12.342Z</summary>Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to libs//locales//*.ts : Shared content (common button text, phase banners, service names, common error messages) goes in locale files (libs/[module]/src/locales/en.tsandcy.ts). Page-specific content goes in controllers.**Applied to files:** - `CLAUDE.md` </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/tickets/VIBE-298/tasks.md</summary> [uncategorized] ~7-~7: The official name of this software platform is spelled with a capital “H”. Context: ...ly Pipeline Configuration - [x] Create `.github/workflows/nightly.yml` workflow file - ... (GITHUB) --- [uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”. Context: ...tests (including @nightly) - [x] Update `.github/workflows/nightly.yml` to use `test:e2e... (GITHUB) --- [uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”. Context: ...yml` to use `test:e2e:all` - [x] Verify `.github/workflows/e2e.yml` continues to use `te... (GITHUB) --- [style] ~27-~27: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym. Context: ...y - [x] Verify nightly tests execute in nightly workflow - [x] Verify nightly tests are... (ADVERB_REPETITION_PREMIUM) --- [style] ~28-~28: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym. Context: ...xecute in nightly workflow - [x] Verify nightly tests are excluded from PR workflow - [... (ADVERB_REPETITION_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/tickets/VIBE-298/ticket.md</summary> 5-5: Bare URL used (MD034, no-bare-urls) </details> <details> <summary>docs/tickets/VIBE-298/plan.md</summary> 31-31: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: E2E Tests </details> <details> <summary>🔇 Additional comments (46)</summary><blockquote> <details> <summary>e2e-tests/tests/sso/sso.spec.ts (4)</summary><blockquote> `1-2`: **Import paths are correctly formatted with `.js` extensions.** Line 2 properly includes the `.js` file extension for the relative import, which is required for ESM with Node.js "nodenext" module resolution. --- `25-25`: **Consistent nightly tagging strategy across test suite.** The selective tagging is well-reasoned: simpler fast-path tests (System Admin login, unauthenticated redirect) run on every PR, while complex tests (multiple user roles, concurrent sessions, session persistence) run nightly only. This balances CI performance with thorough coverage. Also applies to: 45-45, 65-65, 84-84, 109-109 --- `109-138`: **Proper resource cleanup in concurrent session test.** The test correctly creates and closes browser contexts (lines 136–137), preventing resource leaks. --- `5-5`: **The `@nightly` test tagging and `--grep-invert "@nightly"` filter are correctly implemented.** Playwright's `--grep-invert` flag accepts regular expressions and matches against the full test identifier (project name, test file, describe blocks, test title, and test tags—all joined with spaces). The pattern `"@nightly"` correctly excludes tests tagged with `@nightly` at the end of their titles. Tests at lines 25, 45, 65, 84, and 109 are properly tagged with `@nightly` and will be excluded from PR runs via the `test:e2e` script in package.json. </blockquote></details> <details> <summary>e2e-tests/tests/remove-publication.spec.ts (1)</summary><blockquote> `96-239`: **Nightly-tagging of validation, Welsh, and accessibility checks looks appropriate** Only the test titles were updated with `@nightly`; the flows for validation, Welsh `?lng=cy` support, and Axe accessibility remain unchanged and are good fits for nightly-only execution while lighter smoke tests still run on PRs. </blockquote></details> <details> <summary>e2e-tests/tests/courts-tribunals-list.spec.ts (1)</summary><blockquote> `113-478`: **Comprehensive interaction and filter tests correctly reclassified as nightly** Adding `@nightly` to the language toggle, keyboard navigation, filter-panel, and A–Z navigation tests simply re-scopes when they run; their implementations are unchanged and still provide strong coverage in the nightly suite while lighter smoke tests continue to run on PRs. </blockquote></details> <details> <summary>e2e-tests/tests/cft-idam/cft-idam.spec.ts (1)</summary><blockquote> `36-299`: **Heavier CFT IDAM flows sensibly moved under nightly coverage** The added `@nightly` suffixes reclassify complex CFT IDAM scenarios (rejected roles, language propagation, error paths, multi-session and session-lifecycle checks, rejected-page a11y) to nightly-only, with no changes to the actual flows. Core login behavior still runs on every PR, which is a good balance. </blockquote></details> <details> <summary>e2e-tests/tests/api/blob-ingestion.spec.ts (1)</summary><blockquote> `22-271`: **All Blob Ingestion API tests are now nightly-only—confirm coverage expectations** Every test in this describe block is now tagged `@nightly`, so blob-ingestion auth/validation/content-type behavior will no longer run on standard PR e2e but only in the nightly job. The bodies are unchanged and still look correct; please just confirm this coverage shift is intentional and that the nightly workflow reliably provisions the API on `localhost:3001` before these execute. </blockquote></details> <details> <summary>e2e-tests/tests/page-structure.spec.ts (1)</summary><blockquote> `175-213`: **Responsive viewport checks marked as nightly without changing behavior** Only the titles gained `@nightly`; the mobile/tablet/desktop viewport assertions are unchanged and remain valuable in the nightly run while keeping PR runs lighter. </blockquote></details> <details> <summary>e2e-tests/tests/search.spec.ts (1)</summary><blockquote> `93-325`: **Nightly tags applied to richer validation, Welsh, and keyboard flows while keeping core smoke tests** The affected Search tests now carry `@nightly` but their validation, Welsh-language, no-results, and keyboard-access flows (including Axe checks) are otherwise unchanged. Core smoke coverage (page load, basic search, A–Z navigation) still runs on PRs, so the split between fast checks and deeper UX/a11y coverage looks sensible. </blockquote></details> <details> <summary>e2e-tests/tests/summary-of-publications.spec.ts (1)</summary><blockquote> `113-248`: **Error, navigation-history, and Welsh-language scenarios correctly moved under nightly** The `@nightly` suffixes here only affect when these tests run; invalid `locationId` handling, back-navigation, and Welsh-language/a11y behavior are still thoroughly covered, just in the nightly suite, while the main happy-path and base “no publications” checks continue to run on PRs. </blockquote></details> <details> <summary>e2e-tests/tests/add-sub-jurisdiction.spec.ts (1)</summary><blockquote> `93-556`: **Extensive validation, duplicate, Welsh, and accessibility flows appropriately scoped to nightly** The added `@nightly` markers move the more exhaustive sub‑jurisdiction scenarios (keyboard-only journey, detailed validation permutations, HTML/duplicate checks, Welsh flows, language persistence, and success-page access rules/a11y) into the nightly suite, without altering their logic. Core happy-path creation and base a11y still run on PRs, and the richer Welsh + WCAG 2.2 AA coverage continues to run at least nightly, which aligns with the a11y and Welsh requirements in the shared learnings. Based on learnings, these tests continuing to exist and run nightly satisfies the “don’t skip accessibility testing” and Welsh support expectations. </blockquote></details> <details> <summary>e2e-tests/tests/manual-upload.spec.ts (1)</summary><blockquote> `66-66`: **LGTM! Nightly test tagging aligns with PR objectives.** The addition of `@nightly` suffixes to test descriptions is consistent with the PR's goal of establishing a nightly CI pipeline. These changes enable selective test execution via grep filtering without modifying test behavior. Also applies to: 300-300, 329-329, 392-392, 402-402, 457-457, 591-591, 636-636, 641-641, 665-665, 673-673, 681-681, 718-718 </blockquote></details> <details> <summary>e2e-tests/tests/sign-in.spec.ts (1)</summary><blockquote> `125-125`: **LGTM! Consistent nightly test tagging.** The `@nightly` markers enable filtering tests for nightly runs while preserving core functionality tests for PR validation. This selective approach balances CI execution time with comprehensive coverage. Also applies to: 160-160, 211-211, 239-239, 277-277, 310-310, 346-346, 383-383, 419-419, 444-444, 480-480, 515-515 </blockquote></details> <details> <summary>e2e-tests/tests/system-admin-dashboard.spec.ts (1)</summary><blockquote> `74-74`: **LGTM! Nightly test markers correctly applied.** The tagging pattern preserves essential tests for PR runs while deferring additional validation scenarios to nightly execution. Also applies to: 80-80, 91-91, 113-113, 135-135, 146-146, 157-157, 180-180 </blockquote></details> <details> <summary>e2e-tests/tests/add-region.spec.ts (1)</summary><blockquote> `83-83`: **LGTM! Comprehensive nightly coverage for validation and edge cases.** The nightly tags target validation scenarios, Welsh language support, accessibility checks, and responsive design tests, leaving core happy-path flows for PR validation. Also applies to: 144-144, 176-176, 199-199, 222-222, 242-242, 270-270, 298-298, 324-324, 337-337, 353-353, 366-366, 389-389, 442-442, 447-447, 457-457, 476-476, 489-489, 506-506, 517-517 </blockquote></details> <details> <summary>e2e-tests/tests/add-jurisdiction.spec.ts (1)</summary><blockquote> `83-83`: **LGTM! Strategic nightly test distribution.** The tagging prioritizes validation, error handling, and internationalization tests for nightly execution while keeping critical path tests in PR runs. Also applies to: 147-147, 173-173, 193-193, 213-213, 232-232, 260-260, 296-296, 309-309, 325-325, 338-338, 360-360, 413-413, 418-418, 428-428 </blockquote></details> <details> <summary>e2e-tests/tests/cookie-management.spec.ts (2)</summary><blockquote> `202-202`: **LGTM! Nightly tags appropriately applied to preference persistence and accessibility tests.** The selective tagging defers comprehensive validation scenarios to nightly runs while maintaining core cookie banner functionality in PR checks. Also applies to: 220-220, 293-293, 302-302 --- `249-268`: The CSS classes referenced in the test are properly implemented and configured. Both `.js-cookie-banner-accept` and `.js-cookie-banner-reject` are present in the cookie banner template (`libs/web-core/src/views/components/cookie-banner.njk`), configured in the JavaScript implementation (`apps/web/src/assets/js/index.ts`), and correctly referenced in the test locators. No action required. </blockquote></details> <details> <summary>e2e-tests/tests/view-option.spec.ts (1)</summary><blockquote> `111-111`: **LGTM! Nightly test tagging for validation and language support.** The tagging defers validation error scenarios, language toggling, and navigation tests to nightly execution, keeping core selection flows in PR runs. Also applies to: 147-147, 165-165, 202-202, 226-226 </blockquote></details> <details> <summary>e2e-tests/tests/email-subscriptions.spec.ts (1)</summary><blockquote> `249-249`: **LGTM! Nightly coverage for unsubscribe and authentication scenarios.** The tagging appropriately defers comprehensive unsubscribe validation and authentication protection tests to nightly runs while maintaining core subscription flow in PR checks. Also applies to: 326-326 </blockquote></details> <details> <summary>e2e-tests/tests/landing-page.spec.ts (2)</summary><blockquote> `119-204`: **Welsh landing page tests correctly promoted to nightly** Marking the Welsh-language variants as `@nightly` keeps full bilingual coverage without slowing every PR run, and the assertions still exercise the expected Welsh content and toggles. This aligns with the Welsh support requirements and the new nightly filtering strategy. Based on learnings, Welsh content must remain covered. --- `253-293`: **Viewport-specific responsive checks as nightly-only look appropriate** Tagging the three viewport tests with `@nightly` keeps useful responsive smoke coverage while avoiding redundant PR-time runs. The checks remain lightweight (heading, bullet count, primary CTA visibility) and avoid brittle style assertions. </blockquote></details> <details> <summary>e2e-tests/tests/reference-data-upload.spec.ts (6)</summary><blockquote> `109-161`: **End-to-end and session-behaviour checks sensibly moved to nightly** The keyboard accessibility journey and multi-upload/session-behaviour tests are more stateful and potentially slower, so marking them `@nightly` is a good trade-off while preserving coverage of repeat uploads and summary/confirmation navigation rules. --- `164-177`: **File upload validation edge cases now isolated to nightly** Tagging the “no file selected”, invalid type (`.txt` / `.xlsx`), and empty CSV scenarios as `@nightly` keeps thorough validation coverage without burdening every PR run. The expectations (staying on upload page and showing a GOV.UK-style error summary) remain clear and aligned with the UI. Also applies to: 178-213, 231-247 --- `251-345`: **CSV parsing and pagination robustness tests are well-suited for nightly runs** All the CSV parsing edge cases (header validation, invalid IDs, BOM handling, semicolon content, pagination) are valuable but relatively heavy. Tagging them `@nightly` fits the new strategy, and the tests still assert behaviour rather than styling, which matches the E2E guidance. --- `349-432`: **Data validation behaviours remain well-covered while running only nightly** Duplicate ID handling, error summaries, navigation from error states, and required-field validation are still explicitly exercised; adding `@nightly` just scopes them to the nightly pipeline. No functional regressions introduced. --- `435-487`: **Summary page UX checks are appropriate nightly coverage** The summary page file-name display, change link, navigation back to upload, and full column presence are all high-value but not critical for every PR. Nightly tagging keeps them running regularly without bloating PR feedback time. --- `490-533`: **Confirmation and accessibility checks remain mandatory via nightly pipeline** The confirmation-flow tests (DB upload, navigation links, session clearing) plus the Axe-based WCAG 2.2 checks are still enforced, just via the nightly workflow. That honours the “don’t skip accessibility testing” guidance while keeping PR workflows lighter. Also applies to: 537-556 </blockquote></details> <details> <summary>e2e-tests/tests/account-home.spec.ts (1)</summary><blockquote> `362-405`: **Welsh account-home tests align with language support requirements** The `@nightly`-tagged Welsh variants correctly assert translated heading and tile text, as well as identical layout and hrefs for `/account-home?lng=cy`. That matches the Welsh support expectations called out in the guidelines and keeps bilingual coverage running regularly via the nightly pipeline. Based on learnings, Welsh translations must be exercised. </blockquote></details> <details> <summary>e2e-tests/tests/admin-dashboard.spec.ts (3)</summary><blockquote> `68-82`: **Admin dashboard accessibility checks are appropriately scoped to nightly** Adding `@nightly` to the heading-hierarchy and link-accessibility tests keeps these a11y guarantees in place without adding overhead to every PR. Assertions remain straightforward (single `<h1>`, three visible `.admin-tile` links). --- `129-161`: **Responsive viewport and hover interaction checks are sensibly marked nightly** Verifying rendering across mobile/tablet/desktop viewports and that tiles respond to hover are useful but not critical for every PR. Tagging these tests with `@nightly` matches the performance vs coverage trade-off described in the new E2E guidance. No functional changes introduced. Also applies to: 172-178 --- `181-235`: **Role-based access flows are rightly reserved for nightly runs** The System Admin and CTSC Admin access tests are SSO-heavy, depend on multiple test accounts, and exercise more complex navigation paths. Moving them under `@nightly` keeps this important regression coverage while reducing the chance of flaky PRs caused by external auth. The assertions (correct dashboard heading, nav links, and tile counts per role) still look sound. </blockquote></details> <details> <summary>docs/tickets/VIBE-298/tasks.md (1)</summary><blockquote> `5-53`: **Task checklist is consistent with the implemented changes** The phased checklist lines up with what’s in this PR: nightly workflow, `test:e2e` vs `test:e2e:all`, CLAUDE.md E2E guidance, and verification steps. Once CI has run on this branch, just remember to tick the remaining items in Phase 6 (“Verify CI passes”, “Update ticket with completion summary”). </blockquote></details> <details> <summary>e2e-tests/package.json (1)</summary><blockquote> `8-9`: **Script split between default and full E2E runs is consistent with the nightly strategy** Using `test:e2e` with `--grep-invert "@nightly"` for PR-time runs and `test:e2e:all` for the nightly workflow matches the documented guidance in CLAUDE.md and the `@nightly` tagging in the specs. The root-level workflows correctly call the intended scripts: `test:e2e` for PRs (e2e.yml line 194) and `test:e2e:all` for nightly (nightly.yml line 288). </blockquote></details> <details> <summary>e2e-tests/tests/400-error-page.spec.ts (2)</summary><blockquote> `7-7`: **LGTM - appropriate for nightly testing.** The 400 error page content test is suitable for nightly-only execution as it's not a critical user journey. --- `50-50`: **LGTM - appropriate for nightly testing.** The redirect edge case test is suitable for nightly-only execution. </blockquote></details> <details> <summary>.github/workflows/nightly.yml (9)</summary><blockquote> `1-12`: **LGTM - workflow configuration is correct.** The schedule, manual trigger, and permissions are properly configured for a nightly testing workflow. --- `14-54`: **LGTM - lint job properly configured.** The lint job includes appropriate caching and uses Turbo for efficient monorepo linting with the extended 30-minute timeout for nightly runs. --- `56-111`: **LGTM - test job with comprehensive coverage collection.** The unit test job properly generates the Prisma client, runs tests with coverage, merges reports, and uploads artifacts even on failure (via `if: always()`). --- `118-143`: **LGTM - service containers properly configured.** PostgreSQL and Redis services have appropriate health checks with reasonable intervals and retry settings to ensure services are ready before test execution. --- `216-249`: **LGTM - comprehensive database setup with verification.** The database setup workflow includes proper health checks, schema collation, migrations, seeding, and crucially, verification that seed data exists before proceeding with tests. This catches setup issues early. --- `259-281`: **LGTM - efficient Playwright browser caching strategy.** The caching approach optimizes browser installation by: - Keying on `e2e-tests/package.json` hash for proper invalidation - Installing only Chromium (via `--with-deps` or `install-deps`) - Separating full install from system dependencies update --- `283-289`: **LGTM - correct test command for nightly runs.** The E2E test execution correctly uses `test:e2e:all` to run all tests including those tagged with `@nightly`, and captures comprehensive logs for debugging. --- `290-316`: **LGTM - comprehensive test reporting and artifact collection.** The workflow properly collects test results, reports, and application logs with `if: always()` to ensure artifacts are available for debugging even when tests fail. The 7-day retention is appropriate for nightly runs. --- `145-188`: **Secrets list is complete and accurately referenced in the workflow.** All 23 GitHub secrets referenced in the environment variables (lines 145-188) are correctly configured with proper syntax (`${{ secrets.SECRET_NAME }}`). The workflow includes configurations for session management, SSO, CFT IDAM, and Azure authentication. Ensure these secrets are configured in the GitHub repository settings before running the workflow—missing any will cause job failures. Consider adding a pre-flight check or documentation for maintainers listing these required secrets. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Clarify that agents should create one test per user journey rather than separate tests for each validation, accessibility check, or Welsh translation. Added concrete examples showing good and bad patterns to prevent test proliferation. Key changes: - Emphasize "one test per user journey" principle - Show comparison between minimal journey tests vs excessive granular tests - Clarify that validations/accessibility/Welsh checks belong inline within journey tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/*.{test,spec}.ts : Test files must be co-located with source files using the pattern `*.test.ts` or `*.spec.ts` and use Vitest.
Applied to files:
CLAUDE.md
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
369-369: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (2)
CLAUDE.md (2)
367-478: Verify duplication mentioned in enriched summary.The enriched summary states that the E2E Testing guidance block was "inserted...in two places, resulting in duplicated content within CLAUDE.md," but only one instance is visible in the provided code (lines 367–478). Confirm whether duplicated content exists elsewhere in CLAUDE.md that needs consolidation.
367-478: Comprehensive E2E testing guidance aligns well with PR objectives and learnings.The new section provides clear, actionable guidance on minimizing test count, organizing tests with
@nightlytags, testing Welsh translations (aligned with learnings), and accessibility checks. The running commands (yarn test:e2e vs yarn test:e2e:all) match the updated scripts in e2e-tests/package.json from this PR. Content is well-structured with good examples.
Integrate WCAG 2.2 AA accessibility assertions into the main content test and remove @nightly tag to ensure accessibility compliance is verified in all PR workflows, not just nightly runs. Changes: - Merge separate accessibility test into main journey test - Remove @nightly tag to run accessibility checks in PR workflows - Follow inline accessibility pattern from updated guidelines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace bold text subsection markers with proper markdown headings (####) in the E2E Testing with Playwright section to improve document structure, outline navigation, and accessibility. Changes: - Convert 9 bold subsections to #### headings - Improves markdown outline and document navigation - Maintains all existing content and text 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
ashwini-mv
requested review from
ChrisS1512,
KianKwa,
junaidiqbalmoj and
linusnorton
Simplify the nightly workflow by removing redundant setup steps that are already handled by other mechanisms: - Remove PostgreSQL client installation (not needed - service container handles DB) - Remove explicit PostgreSQL readiness check (service health checks handle this) - Remove manual Prisma schema collation (yarn db:generate handles this automatically) - Remove lint and test jobs (moved to separate workflows) - Increase timeout to 120 minutes for comprehensive E2E testing These changes reduce workflow complexity and execution time while maintaining the same functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
3 participants
Jira link
https://tools.hmcts.net/jira/browse/VIBE-298
Change description
File Created: .github/workflows/nightly.yml
File Modified: e2e-tests/package.json
Changes:
"test:e2e": "node run-with-credentials.js --grep-invert "@nightly"" // NEW: Excludes @nightly tests
"test:e2e:all": "node run-with-credentials.js" // NEW: Runs all test
File Modified: CLAUDE.md
Testing done
yes
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.