diff --git a/docs/contributing/artefact/files/role/playwright.specs.js.md b/docs/contributing/artefact/files/role/playwright.specs.js.md index cbb0404262..505499ee16 100644 --- a/docs/contributing/artefact/files/role/playwright.specs.js.md +++ b/docs/contributing/artefact/files/role/playwright.specs.js.md @@ -284,6 +284,8 @@ Use `test.setTimeout()` at the top of the test body (after the gates) and do The chosen budget MUST be visible in a comment on the same line as `test.setTimeout`, naming the slowest sub-step it covers, so future readers can adjust it without re-deriving. +A role whose spec waits on a long background build of its app SHOULD set `playwright.run_after` in its `meta/tests.yml` to every invokable role except itself, so its pass runs after every other role's pass (see [Per-role settings](../../../../../roles/test-e2e-playwright/README.md#per-role-settings)). + ### Cleanup and teardown 🧹 Scenarios that create persistent state (WordPress posts, Discourse topics, Keycloak group memberships, file uploads, …) MUST clean up in a `try { ... } finally { ... }` block. diff --git a/plugins/lookup/role_tests.py b/plugins/lookup/role_tests.py index 3204bc5fe7..8f71d10fe1 100644 --- a/plugins/lookup/role_tests.py +++ b/plugins/lookup/role_tests.py @@ -6,7 +6,8 @@ Terms: application_id and a dotted key path. A missing file, missing key, or non-mapping intermediate returns the ``default`` kwarg (``None`` when not given). ``meta/tests.yml`` carries per-role test-harness settings (e.g. the CLI e2e timeout for roles whose test legitimately outlasts the -uniform default). +uniform default). A string value carrying Jinja is rendered with +``application_id`` bound to the queried role, whoever asks. """ from __future__ import annotations @@ -17,6 +18,7 @@ from typing import Any from ansible.errors import AnsibleError from ansible.plugins.lookup import LookupBase +from utils.cache.base import _render_with_templar from utils.cache.yaml import load_yaml_any from utils.roles.mapping import ROLE_FILE_META_TESTS @@ -50,4 +52,10 @@ class LookupModule(LookupBase): if not isinstance(node, dict) or key not in node: return [default] node = node[key] + if isinstance(node, str) and "{{" in node: + node = _render_with_templar( + node, + templar=getattr(self, "_templar", None), + variables={"application_id": application_id}, + ) return [node] diff --git a/roles/test-e2e-playwright/README.md b/roles/test-e2e-playwright/README.md index 8cf2deea67..d185e76ae9 100644 --- a/roles/test-e2e-playwright/README.md +++ b/roles/test-e2e-playwright/README.md @@ -64,6 +64,25 @@ For the file-level contract, use [Contributing `playwright.env.j2`](../../docs/a - `roles/test-e2e-playwright/templates/package.json.j2` (rendered per-deploy; pins `@playwright/test` from `meta/services.yml.playwright.version`) - `roles/test-e2e-playwright/files/playwright.config.js` (copied as-is) +## Per-role settings + +A tested role declares its Playwright settings under `playwright:` in its own `meta/tests.yml`, read through `lookup('role_tests', application_id, 'playwright.')`. A Jinja value renders with `application_id` bound to the declaring role. + +| Key | Purpose | +|---|---| +| `timeout` | Seconds for the role's pass, for specs that legitimately outlast `test_e2e_playwright_run_timeout` | +| `run_after` | Role names whose passes run before this role's pass. Names outside the round constrain nothing; a name that is no role, the role itself, or a cycle fails the run. | + +Passes run the `provides: sso` role first, then in deploy order. `lookup('playwright_order', ...)` then moves a role with `run_after` behind the roles it names and leaves every other role in place. + +A role whose specs wait on a long background build names every invokable role except itself, so its pass runs last and the earlier passes overlap the build: + +```yaml +playwright: + timeout: 9000 + run_after: "{{ lookup('deployment').all | difference([application_id]) }}" +``` + ## Included files This role ships central Playwright defaults: diff --git a/roles/test-e2e-playwright/lookup_plugins/playwright_order.py b/roles/test-e2e-playwright/lookup_plugins/playwright_order.py new file mode 100644 index 0000000000..afa65c074b --- /dev/null +++ b/roles/test-e2e-playwright/lookup_plugins/playwright_order.py @@ -0,0 +1,83 @@ +"""Lookup `playwright_order`: run each role's Playwright pass after the roles +its ``meta/tests.yml`` names under ``playwright.run_after``. + + lookup('playwright_order', roles) + +Term: the round's Playwright roles in their discovery order. Each role's +``playwright.run_after`` is read through the ``role_tests`` lookup, so a Jinja +value renders with ``application_id`` bound to that role. A named role outside +the round constrains nothing, and a role no constraint moves keeps its place. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from ansible.errors import AnsibleError +from ansible.plugins.loader import lookup_loader +from ansible.plugins.lookup import LookupBase + +from utils.roles.order import stable_order_after + +RUN_AFTER = "playwright.run_after" + + +def _role_names(role: str, value: Any, known: set[str]) -> list[str]: + """Return *value* as the role names *role* runs after. + + Args: + role: the role whose ``meta/tests.yml`` declared *value*. + value: the rendered ``playwright.run_after``. + known: every role directory under ``roles/``. + + Raises: + AnsibleError: *value* is not a list of role names, or names a role + that does not exist. + """ + if not isinstance(value, list) or not all( + isinstance(name, str) and name.strip() for name in value + ): + raise AnsibleError( + f"{role}: {RUN_AFTER} in meta/tests.yml must render to a list of " + f"role names, got {value!r}." + ) + unknown = sorted(set(value) - known) + if unknown: + raise AnsibleError(f"{role}: {RUN_AFTER} names unknown roles {unknown}.") + return value + + +class LookupModule(LookupBase): + def run( + self, terms, variables: dict[str, Any] | None = None, **kwargs + ) -> list[list[str]]: + if len(terms) != 1 or not isinstance(terms[0], list): + raise AnsibleError( + "lookup('playwright_order', roles) expects exactly one list of " + "role names." + ) + roles = [str(role) for role in terms[0]] + base = (variables or {}).get("playbook_dir") + if not base: + raise AnsibleError( + "lookup('playwright_order'): 'playbook_dir' is unavailable." + ) + roles_dir = Path(str(base)) / "roles" + known = {path.name for path in roles_dir.iterdir() if path.is_dir()} + + role_tests = lookup_loader.get( + "role_tests", loader=self._loader, templar=self._templar + ) + after = { + role: _role_names( + role, + role_tests.run([role, RUN_AFTER], variables=variables, default=[])[0], + known, + ) + for role in roles + } + try: + return [stable_order_after(roles, after)] + except RuntimeError as exc: + raise AnsibleError(f"lookup('playwright_order'): {exc}") from exc diff --git a/roles/test-e2e-playwright/vars/main.yml b/roles/test-e2e-playwright/vars/main.yml index 0c254f659d..fbe6fb6b2b 100644 --- a/roles/test-e2e-playwright/vars/main.yml +++ b/roles/test-e2e-playwright/vars/main.yml @@ -15,11 +15,12 @@ TEST_E2E_PLAYWRIGHT_FULLY_PARALLEL: false TEST_E2E_PLAYWRIGHT_APPS: >- {{ - playbook_dir - | discover_playwright_roles( - only_roles=test_e2e_playwright_only_roles, - skip_roles=test_e2e_playwright_skip_roles - ) + lookup('playwright_order', + playbook_dir + | discover_playwright_roles( + only_roles=test_e2e_playwright_only_roles, + skip_roles=test_e2e_playwright_skip_roles + )) }} TEST_E2E_PLAYWRIGHT_SHARED_DIR: "{{ TEST_E2E_PLAYWRIGHT_STAGE_BASE_DIR }}/_shared" diff --git a/roles/web-app-docs/meta/tests.yml b/roles/web-app-docs/meta/tests.yml index b00fe6e2ae..1d091b29aa 100644 --- a/roles/web-app-docs/meta/tests.yml +++ b/roles/web-app-docs/meta/tests.yml @@ -4,3 +4,4 @@ cli: timeout: 9000 playwright: timeout: 9000 + run_after: "{{ lookup('deployment').all | difference([application_id]) }}" diff --git a/tests/unit/python/plugins/lookup/test_role_tests.py b/tests/unit/python/plugins/lookup/test_role_tests.py index 77e2e6b3b5..c5bae263f7 100644 --- a/tests/unit/python/plugins/lookup/test_role_tests.py +++ b/tests/unit/python/plugins/lookup/test_role_tests.py @@ -12,11 +12,15 @@ import unittest from pathlib import Path from ansible.errors import AnsibleError +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar from plugins.lookup.role_tests import LookupModule from utils.cache.yaml import dump_yaml from utils.roles.mapping import ROLE_FILE_META_TESTS +RUN_AFTER_EXPRESSION = "{{ invokables | difference([application_id]) }}" + class TestRoleTestsLookup(unittest.TestCase): @classmethod @@ -26,7 +30,33 @@ class TestRoleTestsLookup(unittest.TestCase): cls.root = Path(cls._tmp.name) tests_yml = cls.root / "roles" / "svc-runner" / ROLE_FILE_META_TESTS tests_yml.parent.mkdir(parents=True) - dump_yaml(tests_yml, {"cli": {"timeout": 9000}}) + dump_yaml( + tests_yml, + { + "cli": {"timeout": 9000}, + "playwright": {"run_after": RUN_AFTER_EXPRESSION}, + }, + ) + + def test_a_jinja_value_renders_with_the_queried_role_as_application_id(self): + variables = { + "playbook_dir": str(self.root), + "application_id": "web-app-caller", + "invokables": ["svc-runner", "web-app-caller", "web-app-x"], + } + lookup = LookupModule( + loader=DataLoader(), + templar=Templar(loader=DataLoader(), variables=variables), + ) + [rendered] = lookup.run( + ["svc-runner", "playwright.run_after"], variables=variables + ) + self.assertEqual(sorted(rendered), ["web-app-caller", "web-app-x"]) + + def test_a_jinja_value_stays_raw_without_a_templar(self): + self.assertEqual( + self._run(["svc-runner", "playwright.run_after"]), [RUN_AFTER_EXPRESSION] + ) def _run(self, terms, **kwargs): return LookupModule().run( diff --git a/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/__init__.py b/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/__init__.py new file mode 100644 index 0000000000..087c384b57 --- /dev/null +++ b/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/__init__.py @@ -0,0 +1,5 @@ +from pathlib import Path + +PROJECT_ROOT: Path = Path(__file__).resolve().parents[6] + +__all__ = ["PROJECT_ROOT"] diff --git a/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/test_playwright_order.py b/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/test_playwright_order.py new file mode 100644 index 0000000000..3ef7d0d399 --- /dev/null +++ b/tests/unit/python/roles/test-e2e-playwright/lookup_plugins/test_playwright_order.py @@ -0,0 +1,119 @@ +import importlib.util +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from ansible.errors import AnsibleError +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar + +from plugins.lookup.role_tests import LookupModule as RoleTestsLookup +from utils.cache.yaml import dump_yaml +from utils.roles.mapping import ROLE_FILE_META_TESTS + +from . import PROJECT_ROOT + +ROUND = [ + "web-app-keycloak", + "web-app-matomo", + "web-app-docs", + "web-app-stalwart", + "web-app-prometheus", +] + + +def _load_module(): + path = ( + PROJECT_ROOT + / "roles" + / "test-e2e-playwright" + / "lookup_plugins" + / "playwright_order.py" + ) + spec = importlib.util.spec_from_file_location("playwright_order_lookup", path) + module = importlib.util.module_from_spec(spec) + assert spec and spec.loader + sys.modules["playwright_order_lookup"] = module + spec.loader.exec_module(module) + return module + + +_plugin = _load_module() + + +class TestPlaywrightOrder(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory(prefix="playwright_order_") + self.addCleanup(self.tmp.cleanup) + self.playbook_dir = Path(self.tmp.name) + for role in [*ROUND, "web-app-outside"]: + (self.playbook_dir / "roles" / role / "meta").mkdir(parents=True) + self.variables = { + "playbook_dir": str(self.playbook_dir), + "invokables": [*ROUND, "web-app-outside"], + } + + def _declare_run_after(self, role: str, run_after) -> None: + dump_yaml( + self.playbook_dir / "roles" / role / ROLE_FILE_META_TESTS, + {"playwright": {"timeout": 9000, "run_after": run_after}}, + ) + + def _order(self, roles: list[str]) -> list[str]: + templar = Templar(loader=DataLoader(), variables=self.variables) + lookup = _plugin.LookupModule(loader=DataLoader(), templar=templar) + role_tests = RoleTestsLookup(loader=DataLoader(), templar=templar) + with patch.object(_plugin, "lookup_loader") as loader: + loader.get.return_value = role_tests + return lookup.run([roles], variables=self.variables)[0] + + def test_without_run_after_the_order_is_kept(self): + self.assertEqual(self._order(list(ROUND)), ROUND) + + def test_every_invokable_but_itself_puts_docs_behind_all_others(self): + self._declare_run_after( + "web-app-docs", "{{ invokables | difference([application_id]) }}" + ) + others = [role for role in ROUND if role != "web-app-docs"] + self.assertEqual(self._order(list(ROUND)), [*others, "web-app-docs"]) + + def test_run_after_only_constrains_roles_in_the_round(self): + self._declare_run_after("web-app-matomo", ["web-app-outside"]) + self._declare_run_after("web-app-docs", ["web-app-stalwart"]) + self.assertEqual( + self._order(list(ROUND)), + [ + "web-app-keycloak", + "web-app-matomo", + "web-app-stalwart", + "web-app-docs", + "web-app-prometheus", + ], + ) + + def test_a_role_naming_itself_raises(self): + self._declare_run_after("web-app-docs", ["web-app-docs"]) + with self.assertRaises(AnsibleError): + self._order(list(ROUND)) + + def test_a_cycle_raises(self): + self._declare_run_after("web-app-docs", ["web-app-prometheus"]) + self._declare_run_after("web-app-prometheus", ["web-app-docs"]) + with self.assertRaises(AnsibleError): + self._order(list(ROUND)) + + def test_an_unknown_role_raises(self): + self._declare_run_after("web-app-docs", ["web-app-typo"]) + with self.assertRaises(AnsibleError): + self._order(list(ROUND)) + + def test_a_value_that_is_no_list_raises(self): + self._declare_run_after("web-app-docs", "web-app-stalwart") + with self.assertRaises(AnsibleError): + self._order(list(ROUND)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/utils/roles/test_order.py b/tests/unit/python/utils/roles/test_order.py index 6f349129f6..1a786ed25b 100644 --- a/tests/unit/python/utils/roles/test_order.py +++ b/tests/unit/python/utils/roles/test_order.py @@ -13,6 +13,7 @@ from utils.roles.order import ( build_dependency_graph, find_cycle, ordered_roles, + stable_order_after, topological_sort, ) @@ -82,5 +83,37 @@ class TestRoleOrder(unittest.TestCase): ordered_roles(self.temp_dir, "grp") +class TestStableOrderAfter(unittest.TestCase): + NAMES = ("keycloak", "matomo", "docs", "stalwart", "jitsi") + + def test_no_constraint_keeps_the_order(self): + self.assertEqual(stable_order_after(list(self.NAMES), {}), list(self.NAMES)) + + def test_a_name_after_all_others_moves_last_and_nothing_else_moves(self): + others = [name for name in self.NAMES if name != "docs"] + result = stable_order_after(list(self.NAMES), {"docs": others}) + self.assertEqual(result, [*others, "docs"]) + + def test_a_satisfied_constraint_moves_nothing(self): + result = stable_order_after(list(self.NAMES), {"stalwart": ["matomo"]}) + self.assertEqual(result, list(self.NAMES)) + + def test_names_outside_the_list_constrain_nothing(self): + result = stable_order_after( + list(self.NAMES), {"matomo": ["absent"], "docs": ["jitsi", "gone"]} + ) + self.assertEqual(result, ["keycloak", "matomo", "stalwart", "jitsi", "docs"]) + + def test_a_name_listing_itself_raises(self): + with self.assertRaises(RuntimeError): + stable_order_after(list(self.NAMES), {"docs": ["docs"]}) + + def test_a_cycle_raises_and_names_its_members(self): + with self.assertRaises(RuntimeError) as ctx: + stable_order_after(list(self.NAMES), {"docs": ["jitsi"], "jitsi": ["docs"]}) + self.assertIn("docs", str(ctx.exception)) + self.assertIn("jitsi", str(ctx.exception)) + + if __name__ == "__main__": unittest.main() diff --git a/utils/roles/order.py b/utils/roles/order.py index ad801f8050..2c39928681 100644 --- a/utils/roles/order.py +++ b/utils/roles/order.py @@ -7,10 +7,14 @@ here instead and read through the `group_roles` lookup. from __future__ import annotations +import heapq from collections import defaultdict, deque from functools import cache from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from collections.abc import Iterable, Mapping from utils.cache.yaml import load_yaml from utils.roles.mapping import ROLE_FILE_META_MAIN, ROLE_FILE_VARS_MAIN @@ -165,6 +169,54 @@ def topological_sort(graph, in_degree, roles=None) -> list[str]: return sorted_roles +def stable_order_after( + names: list[str], after: Mapping[str, Iterable[str]] +) -> list[str]: + """Order *names* so each one follows the names it lists in *after*. + + Args: + names: the names in their current order. + after: per name, the names it must follow. Names absent from *names* + constrain nothing. + + Returns: + *names* reordered: the next name is always the earliest one in *names* + whose predecessors are all placed, so a name moves only when a + constraint forces it. + + Raises: + RuntimeError: the constraints among *names* form a cycle, a name that + lists itself included. + """ + index = {name: position for position, name in enumerate(names)} + blockers = { + name: {dep for dep in after.get(name, ()) if dep in index} for name in names + } + followers: dict[str, list[str]] = defaultdict(list) + for name, deps in blockers.items(): + for dep in deps: + followers[dep].append(name) + + ready = [index[name] for name in names if not blockers[name]] + heapq.heapify(ready) + placed: list[str] = [] + while ready: + name = names[heapq.heappop(ready)] + placed.append(name) + for follower in followers[name]: + blockers[follower].discard(name) + if not blockers[follower]: + heapq.heappush(ready, index[follower]) + + if len(placed) != len(names): + stuck = [name for name in names if blockers[name]] + raise RuntimeError( + "Circular run_after: " + + "; ".join(f"{name} after {sorted(blockers[name])}" for name in stuck) + ) + return placed + + @cache def ordered_roles(roles_dir: str, group: str) -> tuple[dict[str, str], ...]: """Return ``({'role': …, 'app': …}, …)`` for *group*, in run_after order.