Skip to content

Commit bd1dfb2

Browse files
committed
ci: scope website e2e check to runtime changes
Addresses CodeRabbit review: - The check keyed off any apps/website/ file plus fix-style title wording, so a test-only or CI-only change with a "fix:" subject would demand a Playwright test. Narrow the trigger to runtime files under apps/website/src and apps/website/public, treat title wording as a hint to confirm against the diff rather than a qualifying signal on its own, and exempt test/tooling/CI-only changes explicitly. - Any unrelated spec edit satisfied the requirement. Require the added or updated assertion to exercise the route, flow, or behaviour the PR actually changed. - Document the packages/ hand-off. The website consumes four workspace packages, and changes there run the website e2e workflow, but the generic check already requires a browser_tests/ regression test for them. Expanding this check to packages/ would demand two e2e tests for one shared-package fix, so record the hand-off instead. - The path instructions cited the website-unit gate for every apps/website/src file, including src/content, src/i18n and content.config.ts, which vitest.config.ts excludes from coverage. Note the exceptions so reviewers do not ask for tests that cannot move the gate, or for extraction into an uninstrumented directory.
1 parent 8395c4f commit bd1dfb2

1 file changed

Lines changed: 12 additions & 4 deletions

File tree

.coderabbit.yaml

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,16 @@ reviews:
6262
Do not inspect reverse diffs, files changed only on the base branch, or files outside this PR.
6363
If the changed-file list or commit subjects are unavailable, mark the check inconclusive instead of guessing.
6464
65-
This check applies ONLY when the PR changes files under `apps/website/`. If no `apps/website/` files changed, pass immediately.
65+
This check applies ONLY when the PR changes website runtime files under `apps/website/src/` or `apps/website/public/`. If no such files changed, pass immediately — that includes PRs touching only `apps/website/e2e/`, tooling, config, or CI.
66+
67+
Changes confined to `packages/` are deliberately out of scope here, even though the website consumes those packages: the generic "End-to-end regression coverage for fixes" check already requires a `browser_tests/` regression test for them. Do not demand a second website-specific test for a shared-package change.
6668
6769
Fail if all of the following are true:
68-
1. The change alters observable website behaviour: it fixes a bug (title or a commit subject uses `fix`, `fixed`, `fixes`, `fixing`, `bugfix`, or `hotfix`), adds a new page or route, or changes an interactive flow, form, or navigation.
69-
2. The PR does not add or update a meaningful Playwright assertion under `apps/website/e2e/**/*.spec.ts`.
70+
1. The diff itself changes observable website runtime behaviour fixes a user-visible bug, adds a page or route, or changes an interactive flow, form, or navigation. A `fix`/`bugfix`/`hotfix` style title or commit subject is only a hint: confirm it against the diff, and never treat the wording alone as qualifying.
71+
2. The PR does not add or update a Playwright assertion under `apps/website/e2e/**/*.spec.ts` that actually exercises the route, flow, or behaviour this PR changed. An unrelated assertion elsewhere in the suite does not satisfy this.
7072
3. The PR description lacks a concrete explanation of why an end-to-end regression test was not added.
7173
72-
Do not fail for text/copy-only edits, translation-only changes, static asset swaps, styling-only changes, generated files, dependency metadata, or refactors that preserve behaviour.
74+
Do not fail for text/copy-only edits, translation-only changes, static asset swaps, styling-only changes, generated files, dependency metadata, refactors that preserve behaviour, or changes confined to tests, tooling, or CI.
7375
A reformatted or whitespace-only edit to an existing spec does not count as adding coverage.
7476
7577
Pass otherwise.
@@ -129,6 +131,9 @@ reviews:
129131
status, so new behaviour needs a colocated Vitest test. Treat
130132
`docs/guidance/vitest.md` as required review context. Flag new
131133
exported logic that no test exercises.
134+
Exception: `src/content/**`, `src/i18n/**` and `src/content.config.ts`
135+
are excluded from coverage in `apps/website/vitest.config.ts`, so do
136+
not invoke the `website-unit` gate for changes confined to those.
132137
- path: 'apps/website/src/**/*.astro'
133138
instructions: |
134139
`.astro` files are excluded from coverage because V8 cannot
@@ -137,6 +142,9 @@ reviews:
137142
for it to be extracted into a `.ts` module beside the component so it
138143
is covered by the `website-unit` gate. Markup and static content are
139144
fine to leave inline.
145+
Extraction only helps where the destination is instrumented:
146+
`src/content/**` and `src/i18n/**` are excluded from coverage, so do
147+
not ask for logic to be moved into them.
140148
- path: '{browser_tests,apps/website/e2e}/**/*.spec.ts'
141149
instructions: |
142150
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`,

0 commit comments

Comments
 (0)