fix(test): pin NODE_ENV so React unit tests can render (#2568)
Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Alassane
@@ -0,0 +1,152 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Contract tests for the frontend unit-test workflow's NODE_ENV guard.
|
||||
|
||||
React exports `act()` only from its development build, which it selects with
|
||||
`process.env.NODE_ENV`. `apps/web/vitest.config.ts` pins that variable to
|
||||
"test" so the development build always loads; without the pin, every
|
||||
`render()` throws `TypeError: React.act is not a function` for anyone whose
|
||||
shell inherits the runtime image's `NODE_ENV=production` (`Dockerfile`).
|
||||
|
||||
The pin cannot guard itself. The CI image sets no `NODE_ENV`, so Vitest's own
|
||||
`process.env.NODE_ENV ??= "test"` already yields "test" there and the suite
|
||||
passes whether or not the pin exists. `frontend-tests.yml` therefore exports
|
||||
`NODE_ENV=production` on its test step on purpose, to reproduce the
|
||||
environment that broke and make a deleted pin fail in CI.
|
||||
|
||||
These assertions exist because that export reads like a mistake: removing it
|
||||
breaks nothing visibly, and CI silently stops guarding the fix.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
import re
|
||||
import unittest
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
WORKFLOW = REPO_ROOT / ".github" / "workflows" / "frontend-tests.yml"
|
||||
VITEST_CONFIG = REPO_ROOT / "apps" / "web" / "vitest.config.ts"
|
||||
NPMRC = REPO_ROOT / "apps" / ".npmrc"
|
||||
LINT_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "lint-action-pinning.yml"
|
||||
|
||||
STEP_MARKER = " - name: Run tests\n"
|
||||
NEXT_STEP_MARKER = "\n - name: "
|
||||
|
||||
# The subjects above that live outside `.github/`, and so are not covered by
|
||||
# this job's `.github/workflows/**` trigger.
|
||||
UNTRIGGERED_SUBJECTS = ("apps/web/vitest.config.ts", "apps/.npmrc")
|
||||
|
||||
|
||||
def run_tests_step(workflow: str) -> str:
|
||||
"""Return the body of the `Run tests` step, up to the following step."""
|
||||
_, separator, remainder = workflow.partition(STEP_MARKER)
|
||||
if not separator:
|
||||
raise AssertionError("frontend-tests.yml has no 'Run tests' step")
|
||||
return remainder.partition(NEXT_STEP_MARKER)[0]
|
||||
|
||||
|
||||
def trigger_paths(workflow: str, trigger: str) -> str:
|
||||
"""Return the `paths:` list of `trigger`'s block under `on:`.
|
||||
|
||||
Scoped to the `paths:` key rather than the whole trigger block. GitHub
|
||||
rejects `paths` and `paths-ignore` on the same event, so renaming the key
|
||||
inverts the filter wholesale: the job would then run on everything *except*
|
||||
these files, and a match against the block as a whole would not notice.
|
||||
"""
|
||||
block = re.search(rf"(?m)^ {trigger}:\n(?:^ {{4}}.*\n|^\n)*", workflow)
|
||||
if block is None:
|
||||
raise AssertionError(f"lint-action-pinning.yml has no {trigger} trigger")
|
||||
|
||||
paths = re.search(r"(?m)^ paths:\n((?:^ - .*\n)+)", block.group(0))
|
||||
if paths is None:
|
||||
raise AssertionError(
|
||||
f"lint-action-pinning.yml's {trigger} trigger has no `paths:` list. A "
|
||||
"`paths-ignore:` key in its place inverts the filter, and the job stops "
|
||||
"running for the very files listed under it."
|
||||
)
|
||||
return paths.group(1)
|
||||
|
||||
|
||||
class FrontendTestsWorkflowContractTest(unittest.TestCase):
|
||||
def test_test_step_reproduces_the_production_environment(self) -> None:
|
||||
step = run_tests_step(WORKFLOW.read_text(encoding="utf-8"))
|
||||
|
||||
self.assertIn(
|
||||
"NODE_ENV: production",
|
||||
step,
|
||||
"The 'Run tests' step must export NODE_ENV=production so a deleted "
|
||||
"NODE_ENV pin in apps/web/vitest.config.ts fails CI. Without it the "
|
||||
"CI image sets no NODE_ENV, Vitest defaults it to 'test', and the "
|
||||
"suite passes with or without the pin.",
|
||||
)
|
||||
self.assertIn("pnpm --filter @kandev/web test", step)
|
||||
|
||||
def test_vitest_config_still_pins_the_mode(self) -> None:
|
||||
config = VITEST_CONFIG.read_text(encoding="utf-8")
|
||||
|
||||
self.assertIn(
|
||||
'process.env.NODE_ENV = "test";',
|
||||
config,
|
||||
"apps/web/vitest.config.ts must pin NODE_ENV=test. React resolves "
|
||||
"its production build otherwise, which does not export act(), and "
|
||||
"every render()/renderHook() throws before reaching an assertion.",
|
||||
)
|
||||
|
||||
def test_workspace_installs_keep_dev_dependencies(self) -> None:
|
||||
npmrc = NPMRC.read_text(encoding="utf-8")
|
||||
|
||||
self.assertIn(
|
||||
"production=false",
|
||||
npmrc,
|
||||
"apps/.npmrc must set production=false. This is not for the "
|
||||
"workflow's own install step, which runs before the step-scoped "
|
||||
"export and sees no NODE_ENV. It is for every install performed "
|
||||
"under the same variable the test step reproduces: inside the "
|
||||
"runtime image (`Dockerfile`) or any shell inheriting it, pnpm "
|
||||
"otherwise skips devDependencies, leaving no vitest, eslint, or "
|
||||
"tsc to run at all.",
|
||||
)
|
||||
|
||||
def test_this_contract_runs_when_its_subjects_change(self) -> None:
|
||||
"""The assertions above are only worth anything if CI runs them.
|
||||
|
||||
`lint-action-pinning.yml` triggers on `.github/workflows/**`, which
|
||||
covers `frontend-tests.yml` but neither subject in
|
||||
`UNTRIGGERED_SUBJECTS`. `frontend-tests.yml` does not cover
|
||||
`apps/.npmrc` either, so without an explicit trigger here a PR that
|
||||
deletes `production=false` and nothing else runs no job at all and the
|
||||
assertion above never executes. Mirrors the equivalent test in
|
||||
`release-workflow-contract_test.py`.
|
||||
|
||||
The step assertion at the end is weaker than the path ones by
|
||||
construction: this file runs only from that step, so the PR that deletes
|
||||
it also deletes the only thing that would fail. It holds for local runs
|
||||
and for a step whose command is renamed or moved elsewhere. Keeping the
|
||||
job a required check is what covers outright removal, and that lives in
|
||||
branch protection, not here.
|
||||
"""
|
||||
lint_workflow = LINT_WORKFLOW.read_text(encoding="utf-8")
|
||||
|
||||
for trigger in ("push", "pull_request"):
|
||||
paths = trigger_paths(lint_workflow, trigger)
|
||||
|
||||
for subject in UNTRIGGERED_SUBJECTS:
|
||||
self.assertIn(
|
||||
f' - "{subject}"\n',
|
||||
paths,
|
||||
f"{subject} must be a {trigger} path trigger in "
|
||||
"lint-action-pinning.yml. It is a subject of this contract "
|
||||
"but lives outside .github/, so nothing else brings this "
|
||||
"job up when it changes.",
|
||||
)
|
||||
|
||||
self.assertIn(
|
||||
"\n run: python3 .github/scripts/frontend-tests-workflow-contract_test.py\n",
|
||||
lint_workflow,
|
||||
"lint-action-pinning.yml must run this file, as a live step rather "
|
||||
"than a commented-out one. Without it this file is dead code and "
|
||||
"every assertion here silently stops guarding.",
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -209,7 +209,19 @@ jobs:
|
||||
|
||||
node scripts/run-i18n-ratchet.mjs --base "${BASE_SHA}"
|
||||
|
||||
# Deliberately run under the variable that broke the suite, so a
|
||||
# regression fails here instead of only in a container. React exports
|
||||
# `act()` from its development build alone, `vitest.config.ts` pins
|
||||
# `NODE_ENV=test` to force that build, and this image sets no NODE_ENV of
|
||||
# its own — so without this line CI would pass with the pin deleted while
|
||||
# every `render()` throws `TypeError: React.act is not a function` for
|
||||
# anyone whose shell inherits the runtime image's `NODE_ENV=production`
|
||||
# (`Dockerfile`). That gap is exactly how the breakage went unnoticed.
|
||||
# Scoped to this step: `pnpm install` above must keep seeing devDependencies
|
||||
# through its own resolution, not through `apps/.npmrc`'s `production=false`.
|
||||
- name: Run tests
|
||||
env:
|
||||
NODE_ENV: production
|
||||
run: pnpm --filter @kandev/web test
|
||||
|
||||
- name: Build
|
||||
|
||||
@@ -8,6 +8,9 @@ on:
|
||||
- ".github/scripts/lint-action-pinning.py"
|
||||
- ".github/scripts/lint-action-pinning_test.py"
|
||||
- ".github/scripts/claude-code-review-workflow-contract_test.py"
|
||||
- ".github/scripts/frontend-tests-workflow-contract_test.py"
|
||||
- "apps/web/vitest.config.ts"
|
||||
- "apps/.npmrc"
|
||||
- ".github/scripts/preview-env-workflow-contract_test.py"
|
||||
- ".github/scripts/collect-macos-desktop-diagnostics.sh"
|
||||
- ".github/scripts/release-workflow-contract_test.py"
|
||||
@@ -37,6 +40,9 @@ on:
|
||||
- ".github/scripts/lint-action-pinning.py"
|
||||
- ".github/scripts/lint-action-pinning_test.py"
|
||||
- ".github/scripts/claude-code-review-workflow-contract_test.py"
|
||||
- ".github/scripts/frontend-tests-workflow-contract_test.py"
|
||||
- "apps/web/vitest.config.ts"
|
||||
- "apps/.npmrc"
|
||||
- ".github/scripts/preview-env-workflow-contract_test.py"
|
||||
- ".github/scripts/collect-macos-desktop-diagnostics.sh"
|
||||
- ".github/scripts/release-workflow-contract_test.py"
|
||||
@@ -88,6 +94,9 @@ jobs:
|
||||
- name: Test preview environment workflow contract
|
||||
run: python3 .github/scripts/preview-env-workflow-contract_test.py
|
||||
|
||||
- name: Test frontend tests workflow contract
|
||||
run: python3 .github/scripts/frontend-tests-workflow-contract_test.py
|
||||
|
||||
- name: Test release workflow contract
|
||||
run: python3 .github/scripts/release-workflow-contract_test.py
|
||||
|
||||
|
||||
@@ -3,3 +3,10 @@ public-hoist-pattern[]=@lezer/*
|
||||
|
||||
confirm-modules-purge=false
|
||||
|
||||
# This workspace is installed for development, testing, and builds. The runtime
|
||||
# image ships prebuilt Go and SPA artifacts and does not install `apps/` itself.
|
||||
# Without this, `pnpm install` reports "devDependencies: skipped because NODE_ENV
|
||||
# is set to production" inside the runtime container image (`Dockerfile` sets
|
||||
# `NODE_ENV=production`), leaving no vitest, eslint, or tsc. See the same variable's
|
||||
# effect on React's build variant in `web/vitest.config.ts`.
|
||||
production=false
|
||||
|
||||
+2
-2
@@ -295,6 +295,6 @@ plugin leaks a stale registration.
|
||||
|
||||
## Testing notes
|
||||
|
||||
- `vitest.config.ts` pins `process.env.NODE_ENV = "test"`, and that line is load-bearing. React exports `act()` only from its development build, so under `NODE_ENV=production` — which the runtime image sets (`Dockerfile`) and every container/agent shell inherits, while CI's image does not — `@testing-library/react` falls back to `react-dom/test-utils` and **every** `render()`/`renderHook()` throws `TypeError: React.act is not a function` before any assertion, with CI still green. `vitest-environment.test.tsx` and the `vitest.setup.ts` preflight fail by name if the pin goes, and the `Run tests` step in `.github/workflows/frontend-tests.yml` exports `NODE_ENV=production` on purpose so those guards fire in CI too rather than only in a container.
|
||||
- jsdom secure cookies need cookie-setter interception; Radix Tooltip tests use keyboard focus, while Playwright covers pointer hover with `locator.hover()`.
|
||||
- Scope terminal selectors to the active panel/container; mobile and dockview may
|
||||
mount multiple instances, so shared helpers must not use global selectors.
|
||||
- Scope terminal selectors to the active panel/container; mobile and dockview may mount multiple instances, so shared helpers must not use global selectors.
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import * as React from "react";
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
|
||||
/**
|
||||
* Guards the test environment itself rather than any product code.
|
||||
*
|
||||
* `react/index.js` picks its build from `process.env.NODE_ENV`, and
|
||||
* `cjs/react.production.js` does not export `act`. When that build is loaded,
|
||||
* `@testing-library/react`'s act-compat shim silently falls back to the
|
||||
* deprecated `react-dom/test-utils`, whose production build calls `React.act`
|
||||
* and throws `TypeError: React.act is not a function` on the first `render()`
|
||||
* of every suite — 178 of 268 tests, none of them reaching an assertion.
|
||||
*
|
||||
* That is not hypothetical: the Kandev runtime image exports
|
||||
* `NODE_ENV=production` (`Dockerfile`), Vitest only *defaults* the variable
|
||||
* (`process.env.NODE_ENV ??= "test"`) rather than pinning it, and CI's image
|
||||
* sets none — so the whole frontend suite was dead inside the container while
|
||||
* CI stayed green. `vitest.config.ts` now pins the value; this file is what
|
||||
* makes a regression legible, as one named failure instead of every
|
||||
* render-based suite dying on a stack inside `node_modules`.
|
||||
*
|
||||
* The `render` case is the durable one: it keeps holding if the cause changes
|
||||
* to a duplicated `react` instance or a bad alias, which the two cheaper
|
||||
* assertions would miss.
|
||||
*/
|
||||
|
||||
const PROBE_TEXT = "vitest-environment-probe";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
describe("vitest environment", () => {
|
||||
it("runs with NODE_ENV=test so React loads its development build", () => {
|
||||
expect(process.env.NODE_ENV).toBe("test");
|
||||
});
|
||||
|
||||
it("exposes React.act, which only the development build exports", () => {
|
||||
expect(typeof React.act).toBe("function");
|
||||
});
|
||||
|
||||
it("can render a component to the DOM", () => {
|
||||
render(<div data-testid="vitest-environment-probe">{PROBE_TEXT}</div>);
|
||||
|
||||
expect(screen.getByTestId("vitest-environment-probe").textContent).toBe(PROBE_TEXT);
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,9 @@ import viteConfig from "./vite.config";
|
||||
|
||||
if (process.env.DEBUG === "1") process.env.DEBUG = "";
|
||||
|
||||
// Pins NODE_ENV=test — see apps/web/AGENTS.md "Testing notes" for why this is load-bearing.
|
||||
process.env.NODE_ENV = "test";
|
||||
|
||||
const configuredMaxWorkers = process.env.VITEST_MAX_WORKERS?.trim();
|
||||
const maxWorkers = resolveMaxWorkers(configuredMaxWorkers, Boolean(process.env.CI));
|
||||
|
||||
@@ -28,6 +31,9 @@ export default mergeConfig(
|
||||
exclude: ["e2e/**/*.spec.ts", "e2e/fixtures/**", "e2e/pages/**", "node_modules/**"],
|
||||
pool: "threads",
|
||||
maxWorkers,
|
||||
// Already the default, pinned because it is load-bearing: a run that
|
||||
// collects nothing must exit non-zero rather than read as a green suite.
|
||||
passWithNoTests: false,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -1,7 +1,20 @@
|
||||
import type { Window as HappyDOMWindow } from "happy-dom";
|
||||
import * as React from "react";
|
||||
|
||||
import { initI18nForTests, loadAllLocalesForTests } from "./lib/i18n";
|
||||
|
||||
// Guards against `react` production build or duplicate copy — both make `act` unavailable;
|
||||
// vitest.config.ts pins NODE_ENV=test for the common case. Namespace import is deliberate.
|
||||
if (typeof React.act !== "function") {
|
||||
throw new Error(
|
||||
`React.act is unavailable, so no test can render. React only exports act() from its ` +
|
||||
`development build, and NODE_ENV is "${process.env.NODE_ENV}" here. ` +
|
||||
`Check the NODE_ENV pin in vitest.config.ts, then that only one copy of react is installed ` +
|
||||
`(pnpm why react). Without it @testing-library/react falls back to react-dom/test-utils and ` +
|
||||
`every render() throws "TypeError: React.act is not a function".`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* i18n test bootstrap.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user