mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 10:59:59 +00:00
fix(e2e): derive the shared Playwright helpers instead of listing them
A helper that no stager copies fails at import, and it fails for every role that imports it at once, because the whole spec file stops loading. That is what mcp-endpoint.js did to all ten MCP specs: they ran zero tests rather than one failing one. The set lived in three places and no two agreed - the deploy task named four helpers, rerun-spec.sh named three, and the specs required five. addon-gating.js was already missing from the rerun path before any of this. Both stagers now glob the runner's own files/*.js, minus the config, so the directory listing is the set. The lint derives what must be staged from the requires the specs actually make, and holds both stagers to globbing, since a set that can be typed twice will differ twice.
This commit is contained in:
1 parent
0a669b793e
commit
87f762bd75
3 files changed
+160
-19
No files matched your search
@@ -160,14 +160,12 @@
|
||||
|
||||
- name: 📋 Copy shared Playwright helpers into staging tests directory
|
||||
ansible.builtin.copy:
|
||||
src: "{{ lookup('path_absolute', 'roles/test-e2e-playwright/files', item) }}"
|
||||
dest: "{{ _playwright_stage_tests_dir }}/{{ item }}"
|
||||
src: "{{ item }}"
|
||||
dest: "{{ _playwright_stage_tests_dir }}/{{ item | basename }}"
|
||||
mode: "0644"
|
||||
loop:
|
||||
- service-gating.js
|
||||
- onion-test.js
|
||||
- addon-gating.js
|
||||
- timeouts.js
|
||||
when: (item | basename) != 'playwright.config.js'
|
||||
with_fileglob:
|
||||
- "{{ lookup('path_absolute', 'roles/test-e2e-playwright/files') }}/*.js"
|
||||
|
||||
- name: ✅ Ensure staging volume directory exists
|
||||
ansible.builtin.file:
|
||||
|
||||
@@ -80,18 +80,11 @@ for role_js in "$role_playwright_dir"/*.js; do
|
||||
[[ -f "$role_js" ]] || continue
|
||||
cp "$role_js" "$stage_dir/tests/$(basename "$role_js")"
|
||||
done
|
||||
helper_src="$repo_root/roles/test-e2e-playwright/files/service-gating.js"
|
||||
if [[ -f "$helper_src" ]]; then
|
||||
cp "$helper_src" "$stage_dir/tests/service-gating.js"
|
||||
fi
|
||||
timeouts_src="$repo_root/roles/test-e2e-playwright/files/timeouts.js"
|
||||
if [[ -f "$timeouts_src" ]]; then
|
||||
cp "$timeouts_src" "$stage_dir/tests/timeouts.js"
|
||||
fi
|
||||
onion_test_src="$repo_root/roles/test-e2e-playwright/files/onion-test.js"
|
||||
if [[ -f "$onion_test_src" ]]; then
|
||||
cp "$onion_test_src" "$stage_dir/tests/onion-test.js"
|
||||
fi
|
||||
for helper_src in "$repo_root/roles/test-e2e-playwright/files"/*.js; do
|
||||
[[ -f "$helper_src" ]] || continue
|
||||
[[ "$(basename "$helper_src")" == "playwright.config.js" ]] && continue
|
||||
cp "$helper_src" "$stage_dir/tests/$(basename "$helper_src")"
|
||||
done
|
||||
personas_dir="$repo_root/roles/test-e2e-playwright/files/personas"
|
||||
if [[ -d "$personas_dir" ]]; then
|
||||
mkdir -p "$stage_dir/tests/personas/utils"
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
"""Lint: a Playwright helper a spec imports must be staged next to it.
|
||||
|
||||
Two stagers copy role specs into `/e2e/tests/` and, beside them, the shared
|
||||
helpers: the deploy task and `rerun-spec.sh`. Both used to name the helpers,
|
||||
and had already drifted apart. A spec importing a helper neither stager copies
|
||||
fails at import with `Cannot find module`, for every role that imports it at
|
||||
once.
|
||||
|
||||
The scan set is what the specs actually import, not a list of helper names: a
|
||||
require that resolves to a sibling in the role's own playwright directory is
|
||||
role-local and needs no staging, and everything else must be staged.
|
||||
|
||||
Suppression (see ``docs/contributing/actions/testing/suppression.md``):
|
||||
|
||||
* ``# nocheck: playwright-helper-staged`` on, or directly above, the require.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
from utils.annotations.suppress import is_suppressed_at
|
||||
from utils.cache.files import (
|
||||
PROJECT_ROOT,
|
||||
iter_project_files_with_content,
|
||||
read_text,
|
||||
)
|
||||
|
||||
_RULE = "playwright-helper-staged"
|
||||
|
||||
RUNNER_FILES = PROJECT_ROOT / "roles/test-e2e-playwright/files"
|
||||
STAGING_TASK = PROJECT_ROOT / "roles/test-e2e-playwright/tasks/02_run_one.yml"
|
||||
RERUN_SCRIPT = PROJECT_ROOT / "scripts/tests/e2e/rerun-spec.sh"
|
||||
|
||||
_REQUIRE_RE = re.compile(r"""require\(["']\./([A-Za-z0-9._-]+)["']\)""")
|
||||
|
||||
_MOUNTED_AS_DIRECTORY = frozenset({"personas"})
|
||||
|
||||
_NOT_A_HELPER = frozenset({"playwright.config.js"})
|
||||
|
||||
_GLOB_MARKER = "roles/test-e2e-playwright/files') }}/*.js"
|
||||
_SHELL_GLOB_MARKER = 'roles/test-e2e-playwright/files"/*.js'
|
||||
|
||||
|
||||
def staged_helpers() -> set[str]:
|
||||
"""Return the helper file names both stagers copy into the tests dir.
|
||||
|
||||
Neither stager names them any more: the deploy task globs the runner's own
|
||||
`files/*.js` and `rerun-spec.sh` does the same, so the directory listing is
|
||||
the set. Three hand-maintained copies of it had already drifted apart when
|
||||
that was the design.
|
||||
"""
|
||||
return {
|
||||
path.name
|
||||
for path in RUNNER_FILES.glob("*.js")
|
||||
if path.name not in _NOT_A_HELPER
|
||||
}
|
||||
|
||||
|
||||
def spec_files() -> list[tuple[str, str]]:
|
||||
"""Return ``(path, content)`` for every role-local Playwright spec."""
|
||||
return [
|
||||
(path, content)
|
||||
for path, content in iter_project_files_with_content(extensions=(".js",))
|
||||
if "/files/playwright/" in path.replace("\\", "/")
|
||||
]
|
||||
|
||||
|
||||
def unstaged_requires() -> list[str]:
|
||||
"""Return one finding per imported helper the runner never stages."""
|
||||
staged = staged_helpers()
|
||||
findings = []
|
||||
for path, content in spec_files():
|
||||
role_dir = Path(path).parent
|
||||
lines = content.splitlines()
|
||||
for index, line in enumerate(lines, start=1):
|
||||
for name in _REQUIRE_RE.findall(line):
|
||||
if (role_dir / f"{name}.js").is_file():
|
||||
continue
|
||||
if name in _MOUNTED_AS_DIRECTORY:
|
||||
continue
|
||||
if is_suppressed_at(lines, index, _RULE):
|
||||
continue
|
||||
if f"{name}.js" not in staged:
|
||||
findings.append(
|
||||
f"{path}:{index}: requires {name!r}, which is neither a "
|
||||
f"sibling spec nor staged by 02_run_one.yml"
|
||||
)
|
||||
return findings
|
||||
|
||||
|
||||
def stagers_without_the_glob() -> list[str]:
|
||||
"""Return one finding per stager that stopped deriving the helper set.
|
||||
|
||||
Both must glob the runner's `files/*.js`. A stager that goes back to
|
||||
naming helpers drifts from the other one silently, which is how
|
||||
`rerun-spec.sh` came to copy three of the five.
|
||||
"""
|
||||
findings = []
|
||||
if _GLOB_MARKER not in read_text(str(STAGING_TASK)):
|
||||
findings.append(f"{STAGING_TASK.name} no longer globs the helper directory")
|
||||
if _SHELL_GLOB_MARKER not in read_text(str(RERUN_SCRIPT)):
|
||||
findings.append(f"{RERUN_SCRIPT.name} no longer globs the helper directory")
|
||||
return findings
|
||||
|
||||
|
||||
class TestPlaywrightHelpersStaged(unittest.TestCase):
|
||||
def test_every_imported_helper_is_staged(self) -> None:
|
||||
findings = unstaged_requires()
|
||||
self.assertEqual(
|
||||
[],
|
||||
findings,
|
||||
f"Playwright helper import(s) that would fail at runtime "
|
||||
f"({len(findings)}):\n" + "\n".join(f" - {f}" for f in findings),
|
||||
)
|
||||
|
||||
def test_both_stagers_derive_the_helper_set(self) -> None:
|
||||
findings = stagers_without_the_glob()
|
||||
self.assertEqual([], findings, "\n".join(findings))
|
||||
|
||||
def test_the_scan_reaches_the_specs(self) -> None:
|
||||
self.assertTrue(
|
||||
spec_files(), "no Playwright spec was scanned, so the rule is vacuous"
|
||||
)
|
||||
|
||||
def test_the_helper_set_is_not_empty(self) -> None:
|
||||
"""An empty set would make every shared import look unstaged."""
|
||||
self.assertIn("service-gating.js", staged_helpers())
|
||||
|
||||
def test_the_config_is_not_treated_as_a_helper(self) -> None:
|
||||
"""It is staged one level up, as the run's config, not beside the specs."""
|
||||
self.assertNotIn("playwright.config.js", staged_helpers())
|
||||
|
||||
def test_a_shared_helper_is_actually_reached(self) -> None:
|
||||
"""Role-local siblings dominate the requires; without a shared one in
|
||||
the set the rule would pass while never testing what it exists for."""
|
||||
shared = {
|
||||
name
|
||||
for path, content in spec_files()
|
||||
for name in _REQUIRE_RE.findall(content)
|
||||
if not (Path(path).parent / f"{name}.js").is_file()
|
||||
and name not in _MOUNTED_AS_DIRECTORY
|
||||
}
|
||||
self.assertTrue(shared, "no spec imports a shared helper")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user