diff --git a/roles/test-e2e-playwright/tasks/02_run_one.yml b/roles/test-e2e-playwright/tasks/02_run_one.yml index f2c8133186..2df6c68864 100644 --- a/roles/test-e2e-playwright/tasks/02_run_one.yml +++ b/roles/test-e2e-playwright/tasks/02_run_one.yml @@ -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: diff --git a/scripts/tests/e2e/rerun-spec.sh b/scripts/tests/e2e/rerun-spec.sh index d0b19a9626..c5ef153bba 100755 --- a/scripts/tests/e2e/rerun-spec.sh +++ b/scripts/tests/e2e/rerun-spec.sh @@ -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" diff --git a/tests/lint/ansible/roles/test_playwright_helpers_staged.py b/tests/lint/ansible/roles/test_playwright_helpers_staged.py new file mode 100644 index 0000000000..5a84868ea8 --- /dev/null +++ b/tests/lint/ansible/roles/test_playwright_helpers_staged.py @@ -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()