Broaden the CI test suite ahead of the stack upgrades

.github/dependabot.yml

@@ -16,12 +16,15 @@ updates:
   - package-ecosystem: "npm"
     directory: "/"
     ignore:
-      # Enzyme only supports React 16 (enzyme-adapter-react-16), so React majors break the unit tests.
+      # React majors need coordinated changes (react-refetch, the router, the test libraries below), so they're done by hand.
       - dependency-name: "react"
         update-types: ["version-update:semver-major"]
       - dependency-name: "react-dom"
         update-types: ["version-update:semver-major"]
-      - dependency-name: "react-test-renderer"
+      # @testing-library/react 13+ requires React 18, and @testing-library/dom is pinned to the 8.x that 12.x uses.
+      - dependency-name: "@testing-library/react"
+        update-types: ["version-update:semver-major"]
+      - dependency-name: "@testing-library/dom"
         update-types: ["version-update:semver-major"]
       # 3.x is a rewrite that's incompatible with react-metrics-graphics (see update-deps/denied.md).
       - dependency-name: "metrics-graphics"

.github/workflows/ci.yml

@@ -35,6 +35,8 @@ jobs:
               run: npm run lint
             - name: Run unit tests
               run: npm run test:jest
+            - name: Build
+              run: npm run build
 
     e2e:
         name: End-to-end tests
@@ -55,5 +57,31 @@ jobs:
               run: npx playwright install --with-deps chromium
             - name: Compile Stylus
               run: npm run build:css
-            - name: Run end-to-end tests
-              run: npm run test:playwright
+            - name: Run end-to-end tests on recorded data
+              run: npm run test:playwright:recorded
+
+    # Not a required check. These are the same specs, run against the real
+    # data.firefox.com, which changes weekly; a failure usually means the
+    # recording in tests/playwright/fixtures/ needs updating, not that the
+    # change under review is broken.
+    live-data:
+        name: Live data tests
+        runs-on: ubuntu-latest
+        timeout-minutes: 20
+        if: github.repository == 'mozilla/ensemble'
+        steps:
+            - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+              with:
+                  persist-credentials: false
+            - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
+              with:
+                  node-version-file: '.nvmrc'
+                  cache: npm
+            - name: Install dependencies
+              run: npm ci
+            - name: Install Playwright browsers
+              run: npx playwright install --with-deps chromium
+            - name: Compile Stylus
+              run: npm run build:css
+            - name: Run end-to-end tests on live data
+              run: npm run test:playwright:live

CONTRIBUTING.md

@@ -20,8 +20,11 @@ how to work in the code, not what the code is.
     npm run build:app                    # vite build; confirmed working, no flags needed
     npm run build:version.json           # writes build/version.json; must run after build:app, needs git on PATH
     npm run size                         # source-map-explorer on build/assets/index-*.js (needs a prior build)
-    npm run test:playwright              # playwright test; starts the dev server itself if one
-                                          # isn't already running on :3000
+    npm run test:playwright              # every Playwright project; starts the dev server itself if
+                                          # one isn't already running on :3000
+    npm run test:playwright:recorded     # specs on the recording (CI's required check)
+    npm run test:playwright:live         # the same specs on live data (CI's non-required check)
+    npm run record:fixtures              # re-record tests/playwright/fixtures/
 
 Run one Vitest file directly: `npx vitest run src/tests/jest/Dashboard.test.jsx` (confirmed
 working). Run one Playwright spec directly: `npx playwright test contact.spec.js`.
@@ -134,38 +137,114 @@ arriving after a long gap. Optimize for them.
 
 ## Testing guidelines
 
-**Vitest + Enzyme**, configured in the `test` block of `vite.config.mjs` (the same file that
+**Vitest + React Testing Library**, configured in the `test` block of `vite.config.mjs` (the same file that
 configures the Vite build) — `environment: 'jsdom'`, `globals: true`, `setupFiles:
 ['./src/setupTests.js']`, scoped to `include: ['src/tests/jest/**/*.test.jsx']` specifically so
 Vitest's own default file-discovery glob doesn't also try to run Playwright's `.spec.js` files.
-Only two files, both in `src/tests/jest/`, named `PascalCase.test.jsx` (the directory is still
+Test files live in `src/tests/jest/`, named `PascalCase.test.jsx` (the directory is still
 named `jest` even though Vitest is what actually runs these — a naming leftover from before this
-repository's toolchain migration, not worth a rename on its own). `src/setupTests.js` puts `React`
-and `shallow` on the global object, so test files import neither: they `import Dashboard from
-'../../components/views/Dashboard';` and call bare `shallow(<Dashboard … />)`. The existing style is
-top-level `it(...)` with long descriptive sentences, a `beforeAll` that builds a `requiredProps`
-object, and assertions via `.find(selector).exists()` and `.html()).toContain(...)`. Shallow
-rendering only; no snapshots; `react-refetch` is never mocked; there are no container tests.
+repository's toolchain migration, not worth a rename on its own). `@testing-library/react` is pinned
+to 12.x, the last line that supports React 16, and `@testing-library/dom` is pinned to the 8.x it
+uses so `@testing-library/user-event` shares the same copy. `src/setupTests.js` puts `React` on the
+global object, because the classic JSX runtime needs it in scope, so test files don't import it.
+They import `render` and `screen` from `@testing-library/react` and assert on the rendered DOM —
+`container.querySelector('#dashboard-description')`, or `innerHTML` where the markup itself is the
+point, as in the sanitization test. The setup file also registers the jest-dom matchers
+(`toHaveTextContent`, `toBeInTheDocument`) and fails any test that calls `console.error`
+unexpectedly, so React warnings show up as failures. A test that expects an error replaces it first
+with `vi.spyOn(console, 'error').mockImplementation(() => {})`. The existing style is top-level
+`it(...)` with long descriptive sentences. No snapshots.
+
+`react-refetch` captures `window.fetch` when `connect()` runs, which is at module import, so a stub
+set inside a test is silently ignored and the container goes to the real network. Install it with
+`vi.hoisted(() => { const mock = vi.fn(); window.fetch = mock; return mock; })` so it exists before
+the imports, and `mockReset()` it in `beforeEach`; `SummaryMetricContainer.test.jsx` is the
+simplest example. Containers whose view needs real layout (`views/Chart` uses metrics-graphics,
+`views/SummaryMetric` draws with d3) are tested by replacing that view with `vi.mock` and asserting
+on the props it received. Plain-markup views such as `views/DataTable` render for real.
+`Chart.test.jsx` renders the real `views/Chart` with `react-metrics-graphics` replaced by
+`vi.mock('react-metrics-graphics', () => ({ default: vi.fn(() => null) }))`, and asserts on the
+options it computes. `SummaryMetric.test.jsx` renders the real d3 summary bar inside
+`<div id="application"><main>`, with `HTMLElement.prototype.offsetWidth` spied to 490: jsdom
+reports 0px, and at 490px the bar draws at its 450px maximum, so segment widths are predictable.
+`Routes.test.jsx` renders `views/Application` inside `MemoryRouter` through a single `renderAt(path)`
+function, so a router upgrade changes one place, and mocks `react-ga`. Its route cases come from
+`config.json`. ESLint
+allows the name `screen` in test files despite `no-shadow`'s `builtinGlobals`, because importing
+React Testing Library's `screen` is that library's documented idiom.
 
 **Playwright** specs live in `tests/playwright/specs/`, configured by `playwright.config.js` at the
 repository root. Specs are `camelCase.spec.js` or `kebab-case.spec.js` (matching the component or
-page under test), CommonJS, using `@playwright/test`'s own `test`/`expect` — for example
-`test('Page loads', async ({ page }) => { await expect(page.locator('#contact')).toBeVisible(); })`.
-Shared helpers (`linkWorks`, `linksWork`, `flagForUpdate`, `metricTitleIsCorrect`) live in
+page under test), CommonJS. Every spec except `jsDisabled.spec.js` imports `test`, `expect` and,
+where needed, `readFixture` from `tests/playwright/fixtureData.js` rather than from
+`@playwright/test` — for example `test('Page loads', async ({ page }) => { await
+expect(page.locator('#contact')).toBeVisible(); })`. Shared helpers (`linkWorks`, `linksWork`,
+`flagForUpdate`, `metricTitleIsCorrect`, `changeRegionAndWaitForMetrics`) live in
 `tests/playwright/utils.js`. `linkWorks` and `linksWork` check only links within the app and skip
 external URLs, which failed too often for reasons outside this repository. `flagForUpdate`
-deliberately fails when an element count changes, to force a human to look. The `dashboards/*.spec.js` specs assert exact metric titles and section
-ordering against live production data, so upstream data changes break them by design — this is not
-a bug in the tests, and updating their expectations to match reality is normal, expected
-maintenance. Two browser projects: `chromium` (everything except `jsDisabled.spec.js`) and
-`chromium-no-js` (`jsDisabled.spec.js` only, run with `javaScriptEnabled: false` — this is
-Chromium-only because `nightwatch.conf.js`, the file this suite replaced, only ever tested Chrome
-too; it is not a gap introduced by this migration). Playwright starts the dev server itself
-(`webServer: { command: 'npm start', ... }`) if one isn't already running on `:3000`, reusing an
-existing one when there is one. Point the suite at a different environment with
-`PLAYWRIGHT_BASE_URL`, for example `PLAYWRIGHT_BASE_URL=https://data.firefox.com npx playwright
-test` — there is no separate `test:playwright:stage`/`test:playwright:prod` script for this, unlike
-the old Nightwatch scripts, since the environment variable already covers it in one line.
+deliberately fails when an element count changes, to force a human to look. Playwright starts the
+dev server itself (`webServer: { command: 'npm start', ... }`) if one isn't already running on
+`:3000`, reusing an existing one when there is one — so if something unrelated is on `:3000`, the
+suite silently tests that instead.
+
+The same specs run in two modes, chosen by Playwright project:
+
+- **Recorded**, the `chromium` project (every spec but `jsDisabled.spec.js`) plus `chromium-no-js`
+  (`jsDisabled.spec.js` only, with `javaScriptEnabled: false`; Chromium-only because the Nightwatch
+  suite it replaced only tested Chrome). `npm run test:playwright:recorded` runs these, and so does
+  CI's "End-to-end tests" check, which the `main` ruleset requires before merging.
+  `fixtureData.js`'s `test` answers every `https://data.firefox.com/datasets/desktop/**` request
+  from `tests/playwright/fixtures/`, whose paths mirror the URLs, and answers other third-party
+  requests (Google Analytics) with an empty response. It fails the test on any page error, any
+  console error, or any dataset request with no recording. Only the default region and `Germany`
+  are recorded; a metric request for another region is served the default region's file for the
+  same metric, so specs can pick any region, while a missing metric or dashboard still fails.
+- **Live**, the `live-data` project: the same specs, minus `fixtures/` (which read their expected
+  values from the recording) and `jsDisabled.spec.js`, against the real data.firefox.com with no
+  interception. `npm run test:playwright:live` runs these, and so does CI's "Live data tests"
+  check, which is **not** required. `npm run test:playwright` runs every project.
+
+A spec that expects errors allows them with `test.use({ allowedErrors: /pattern/ })`. That takes a
+single RegExp because `test.use()` reads an array as a `[value, options]` pair. To change one
+response, register a later `page.route()`, which takes precedence, as `errors.spec.js` and
+`descriptions.spec.js` do. The specs about what charts draw (`chart-series`, `region-data`,
+`summary-bars`, `dates`, `boundaries`) serve a small controlled JSON body for one metric this way,
+so their assertions don't depend on what was recorded. The `fixtures/` specs read expected values with `readFixture(...)`; the
+`dashboards/*.spec.js` specs hardcode exact metric titles and section ordering. Those hardcoded
+values match the recording, and the live check compares them with reality — so upstream data
+changes break the live check by design. That is not a bug in the tests: a red "Live data tests"
+means the recording is stale, and the fix is to update it in a separate pull request.
+
+### Running the tests against live data
+
+1. Run `npx playwright install chromium` once.
+2. Run `npm run test:playwright:live`. It starts the local dev server and runs the specs against
+   data.firefox.com.
+3. To test a deployed site instead, run
+   `PLAYWRIGHT_BASE_URL=https://data.firefox.com npm run test:playwright:live`. Any other
+   environment works the same way; with `PLAYWRIGHT_BASE_URL` set, no local server is started.
+4. If port 3000 is already in use by something else, start `npx vite --port 3001 --strictPort` in
+   another terminal and run with `PLAYWRIGHT_BASE_URL=http://localhost:3001`.
+5. Expect failures when upstream data has changed. Read them as "the recording is stale", not "the
+   code is broken". The hardware dashboard renders slowly on the full live data (#409), so its
+   "Charts render" test can need the retry.
+
+### Updating the recorded data
+
+1. Run `npm run record:fixtures`. It replaces `tests/playwright/fixtures/` with each dashboard's
+   `index.json`, plus every metric and summary metric for the default region and for `Germany`
+   where the dashboard supports regions, trimmed to the 12 most recent dates (the full hardware
+   history runs to several megabytes).
+2. Run `git diff --stat tests/playwright/fixtures` and skim it for unexpected changes: new or
+   removed metrics, regions, titles, or odd populations such as a `"NULL"` bucket.
+3. Run `npm run test:playwright:recorded`.
+   - Failures in the `fixtures/` specs mean the data's shape changed, because those specs read
+     their expected values from the recording.
+   - Failures in the hardcoded-title specs (`dashboards/`) mean titles or ordering changed. Update
+     the hardcoded expectations to match.
+4. Run `npm run test:playwright:live`. It should now agree with the recorded run.
+5. Commit the recording and any spec updates together, in one commit titled like "Re-record
+   Playwright fixtures".
 
 Tests describe the code as it is now. Assert what the code does; never add a test whose purpose is
 to prove that removed behavior is absent — a negative assertion about history passes forever while
@@ -182,8 +261,7 @@ style indirection layer, so a failure points straight at the markup it cares abo
 | Renaming a landmark ID | Breaks Playwright selectors silently. |
 | `build:version.json` | Needs `build/` to already exist and `git` on PATH; must follow `build:app`. |
 | `no-shadow` / `no-redeclare` with `builtinGlobals` | Naming a local `name`, `status`, `history`, `event` is a hard error. |
-| A first-time route visit against the Vite dev server | Can occasionally 504 or fail a dynamic import ("Outdated Optimize Dep") while Vite's dependency pre-bundler catches up with a newly-discovered chunk. A reload resolves it. This does not happen against a production build (`vite build`/`vite preview`), which has no dependency pre-bundling step. |
-| `metrics-graphics` charts and `path.mg-line1` | Each chart renders many invisible `.mg-voronoi` hover paths that also carry the `mg-line1` class (confirmed: 292 matches for one chart). A Playwright locator for this selector needs `.first()`, or it throws a strict-mode "multiple elements matched" error. |
+| `metrics-graphics` charts and `path.mg-line1` | Each chart renders many invisible `.mg-voronoi` hover paths that also carry the `mg-line1` class (confirmed: 292 matches for one chart), and a population with a single data point draws a zero-size line path, shown as a dot instead. To check that a chart drew a line, use `path.mg-main-line` with `.filter({ visible: true }).first()`. |
 
 Known rot, recorded for recognition only, not as a roadmap: `react-app-polyfill` and
 `babel-polyfill`, plus a `browserslist` of `>0.2%, not dead, not ie <= 10, not op_mini all`, were

docs/architecture/frontend.md

@@ -108,7 +108,7 @@ permanent list in this file, for the same reason the wider issue backlog isn't s
     ├── index.jsx                    entry: ReactDOM.render + BrowserRouter
     ├── config.json                  the route/dashboard table + next-button flow (above)
     ├── registerServiceWorker.js     only unregister() is called
-    ├── setupTests.js                enzyme adapter; puts React and shallow on the global object
+    ├── setupTests.js                puts React on the global object; jest-dom matchers; fails on console.error
     ├── components/
     │   ├── containers/              6 stateful, data-fetching .jsx files (above)
     │   ├── decorators/              withTracker.jsx (react-ga), withNextButton.jsx