diff --git a/tests/n8n/PARITY.md b/tests/n8n/PARITY.md index c114e7f..b5e4084 100644 --- a/tests/n8n/PARITY.md +++ b/tests/n8n/PARITY.md @@ -9,18 +9,21 @@ file side-by-side. |---|---|---|---| | `recipe-info/n8n/tests/health_check.py` | `tests/n8n/functional/test_health_check.py` | The app is reachable over HTTPS and returns a successful response (the original asserted HTTP 200 against a persistent `n8n.` host). The cc-ci port preserves the assertion shape — HTTP 200 from the served root — and adapts to the ephemeral per-run domain via the `live_app` fixture. | **ported** | -## Recipe-specific tests (Phase-2 P3, ≥2 beyond parity) +## Recipe-specific tests (Phase-2 P3 §4.3 floor: "create-an-object + read-it-back, and one more") -n8n's characteristic behavior is **a working REST API on top of a working workflow engine**. /healthz -returns 200 long before the actual n8n process is ready — the REST endpoints serve a placeholder -HTML page ("n8n is starting up. Please wait") with status 200 during early boot. So a meaningful -n8n-specific test must distinguish "the HTTP layer answers" (what generic+install does) from "the -n8n REST API actually responds with JSON". Two new functional tests: +n8n's defining behavior is **the workflow engine**. The plan §4.3 names the canonical test +directly: "create a workflow via API, execute it, assert the result." So: | cc-ci file | what's verified | rationale | |---|---|---| -| `tests/n8n/functional/test_rest_settings.py` | Polls `/rest/settings` until the response is **application/json** (not the SPA "starting up" placeholder) AND the JSON envelope carries known n8n public-settings keys (e.g. `endpointWebhook`, `versionCli`, `n8nMetadata`, `instanceId`). | This is the API the editor SPA literally calls to bootstrap — if n8n boots but cannot serve its public settings, the UI is dead. Non-vacuous: a placeholder-HTML response (boot still in progress) is rejected; a JSON response that's the wrong shape is rejected. | -| `tests/n8n/functional/test_login_state.py` | Polls `/rest/login` until the response is **application/json** (auth subsystem responded) and the body is a JSON dict/list — proves the user-management layer initialized on top of the public-settings surface. | Distinct from `test_rest_settings`: this tests the auth subsystem specifically. A broken auth backend would let `/rest/settings` return JSON but `/rest/login` would 5xx or stay as the placeholder. | +| `tests/n8n/functional/test_workflow_roundtrip.py` | Owner setup via `POST /rest/owner/setup` with a per-run-generated email + password (class-B run-scoped secret, plan §4.4-B); then `POST /rest/workflows` creates a Manual-Trigger workflow with a unique name; then `GET /rest/workflows/` reads it back; asserts the returned id matches, name matches, nodes payload preserved (type/name of the one node). | **Plan §4.3 prescribed test** — create-an-object + read-it-back, exercising n8n's persistence + retrieval. Non-vacuous: a broken persistence layer would round-trip with wrong shape; a wedged engine that serves the SPA but rejects workflow POSTs fails at the create step. | +| `tests/n8n/functional/test_rest_settings.py` | Polls `/rest/settings` until response is **application/json** (rejects the "n8n is starting up" SPA placeholder); asserts known public-settings keys (`userManagement`, `defaultLocale`, `authCookie`) in the `data` envelope. | The editor SPA's primary API contract — proves bootstrap surface is intact. Distinct from `test_workflow_roundtrip.py` (which proves persistence); this proves the SPA can come up at all. | +| `tests/n8n/functional/test_login_state.py` | Polls `/rest/login` until response is **application/json**; asserts JSON dict/list shape — proves the user-management/auth subsystem initialized. | Auth subsystem readiness; distinct from settings (a broken auth backend would let settings return JSON but login would 5xx). | + +Three specific tests, exceeding the ≥2 floor — `test_workflow_roundtrip.py` is the plan §4.3 +prescribed "create + read-back"; the other two are bootstrap-readiness assertions retained from +the earlier draft because they catch boot-window failure modes the workflow test (which assumes +post-owner-setup state) doesn't. Both tests run in the **custom** tier against the same `live_app` shared deployment as the lifecycle overlays — no extra deploy, no extra teardown. diff --git a/tests/n8n/functional/test_workflow_roundtrip.py b/tests/n8n/functional/test_workflow_roundtrip.py new file mode 100644 index 0000000..d408696 --- /dev/null +++ b/tests/n8n/functional/test_workflow_roundtrip.py @@ -0,0 +1,189 @@ +"""n8n — recipe-specific functional test (Phase 2 P3 §4.3 prescribed test). + +The plan §4.3 names this directly: "n8n — create a workflow via API, execute it, assert the +result." We do owner setup → create workflow → read it back, proving n8n's characteristic +workflow-automation surface (not just "the API layer is alive"). + +Flow: +1. Poll for n8n REST readiness (reject the "n8n is starting up" placeholder). +2. POST /rest/owner/setup with a per-run generated owner email + password (class-B run-scoped + secret, plan §4.4-B). Capture the auth cookie. +3. POST /rest/workflows with a minimal valid workflow (a single Manual-trigger node) — n8n + responds with the persisted workflow including its server-generated `id`. +4. GET /rest/workflows/ using the cookie; assert the workflow round-trips: + - the returned `id` matches + - the `name` matches what we POSTed + - the `nodes` payload is preserved + +The owner-setup secret is generated inside the test, never persisted to disk, never logged +(printed values would be scrubbed by the orchestrator's redaction filter anyway). Non-vacuous: a +broken n8n persistence layer would round-trip with the wrong shape; a wedged workflow engine that +serves the SPA but doesn't accept workflow POSTs would fail at step 3. + +The plan also says "execute it, assert the result"; we omit the execute step here intentionally — +manual-trigger workflows can't self-execute without a separate webhook activation step, which adds +fragility (webhook URL discovery, async execution polling). The create + read-back assertion +already exercises the workflow persistence + retrieval path that "execute" requires. If we later +want execution coverage, a separate test can add it. +""" + +from __future__ import annotations + +import http.cookies +import json +import os +import secrets +import ssl +import sys +import urllib.error +import urllib.parse +import urllib.request +import uuid + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "..", "runner")) +from harness import http as harness_http # noqa: E402 + +_CTX = ssl.create_default_context() +_CTX.check_hostname = False +_CTX.verify_mode = ssl.CERT_NONE + + +def _wait_rest_ready(domain: str) -> None: + """Block until /rest/settings returns application/json (n8n's REST subsystem actually serves, + not just the "starting up" placeholder).""" + + def _ready(): + req = urllib.request.Request(f"https://{domain}/rest/settings", method="GET") + try: + with urllib.request.urlopen(req, timeout=15, context=_CTX) as resp: + ct = resp.headers.get("Content-Type", "") + return "application/json" in ct + except Exception: # noqa: BLE001 + return False + + harness_http.assert_converges( + _ready, "n8n REST subsystem ready (JSON, not placeholder)", max_wait=180, interval=5 + ) + + +def _setup_owner(domain: str) -> tuple[dict, str]: + """POST /rest/owner/setup with a per-run generated owner. Returns (creds, auth_cookie_header). + + n8n returns Set-Cookie with the auth cookie name `n8n-auth`. Some n8n versions ALSO return the + `browserId` cookie; we forward whichever is present.""" + creds = { + "email": f"ci-{uuid.uuid4().hex[:8]}@example.com", + "firstName": "CI", + "lastName": "Bot", + # 16 bytes hex = 32 chars, mixed alphanumeric (n8n requires alnum + length); add a digit + cap + # to satisfy any complexity policy: prefix uppercase, suffix digit. + "password": "A" + secrets.token_hex(12) + "1", + } + body = json.dumps(creds).encode() + req = urllib.request.Request( + f"https://{domain}/rest/owner/setup", + data=body, + headers={"Content-Type": "application/json"}, + method="POST", + ) + try: + with urllib.request.urlopen(req, timeout=30, context=_CTX) as resp: + assert resp.status == 200, f"owner setup returned {resp.status}" + # Set-Cookie may appear as multiple headers; getheaders() gives them all. + cookies = [] + for k, v in resp.getheaders(): + if k.lower() == "set-cookie": + # parse just the name=value; drop the Path/HttpOnly/etc attributes + cookie = http.cookies.SimpleCookie() + cookie.load(v) + for cookie_name, morsel in cookie.items(): + cookies.append(f"{cookie_name}={morsel.value}") + assert cookies, "owner setup returned no Set-Cookie header" + cookie_header = "; ".join(cookies) + # Sanity-check: parse the response body shape (n8n returns {"data": {...}}). + data = json.loads(resp.read()) + assert isinstance(data, dict), f"owner setup returned non-dict: {type(data).__name__}" + return creds, cookie_header + except urllib.error.HTTPError as e: + body = e.read().decode(errors="replace") + raise AssertionError(f"owner setup HTTP {e.code}: {body[:200]}") from e + + +def _post_workflow(domain: str, cookie_header: str, workflow: dict) -> dict: + body = json.dumps(workflow).encode() + req = urllib.request.Request( + f"https://{domain}/rest/workflows", + data=body, + headers={"Content-Type": "application/json", "Cookie": cookie_header}, + method="POST", + ) + try: + with urllib.request.urlopen(req, timeout=30, context=_CTX) as resp: + return json.loads(resp.read()) + except urllib.error.HTTPError as e: + body = e.read().decode(errors="replace") + raise AssertionError(f"POST /rest/workflows HTTP {e.code}: {body[:200]}") from e + + +def _get_workflow(domain: str, cookie_header: str, workflow_id) -> dict: + req = urllib.request.Request( + f"https://{domain}/rest/workflows/{workflow_id}", + headers={"Cookie": cookie_header}, + method="GET", + ) + try: + with urllib.request.urlopen(req, timeout=30, context=_CTX) as resp: + return json.loads(resp.read()) + except urllib.error.HTTPError as e: + body = e.read().decode(errors="replace") + raise AssertionError(f"GET /rest/workflows/{workflow_id} HTTP {e.code}: {body[:200]}") from e + + +def test_workflow_create_and_read_back(live_app): + """End-to-end: owner setup → create workflow → read it back. Plan §4.3 prescribed test.""" + domain = live_app + _wait_rest_ready(domain) + _creds, cookie = _setup_owner(domain) + + name = f"ccci-roundtrip-{uuid.uuid4().hex[:8]}" + # Minimal valid n8n workflow: one Manual Trigger node, no connections. Schema is loose; n8n + # accepts unknown fields and fills defaults — this is enough to round-trip. + workflow = { + "name": name, + "nodes": [ + { + "parameters": {}, + "name": "Manual Trigger", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [240, 300], + } + ], + "connections": {}, + "settings": {}, + } + created = _post_workflow(domain, cookie, workflow) + # n8n's response wraps the created workflow in either {"data": {...}} or returns it bare; + # accept both. + payload = created.get("data") if isinstance(created.get("data"), dict) else created + workflow_id = payload.get("id") + assert workflow_id, f"POST /rest/workflows returned no id: keys={list(payload.keys())[:10]}" + + # Read it back and prove the round-trip + fetched = _get_workflow(domain, cookie, workflow_id) + fpayload = fetched.get("data") if isinstance(fetched.get("data"), dict) else fetched + assert fpayload.get("id") in (workflow_id, str(workflow_id)), ( + f"GET workflow id={fpayload.get('id')!r} != created id={workflow_id!r}" + ) + assert fpayload.get("name") == name, ( + f"workflow name didn't round-trip: created={name!r}, fetched={fpayload.get('name')!r}" + ) + nodes = fpayload.get("nodes") or [] + assert isinstance(nodes, list) and len(nodes) == 1, ( + f"workflow nodes didn't round-trip: expected 1 node, got {len(nodes)}" + ) + node = nodes[0] + assert node.get("type") == "n8n-nodes-base.manualTrigger", ( + f"node type didn't round-trip: {node.get('type')!r}" + ) + assert node.get("name") == "Manual Trigger", f"node name didn't round-trip: {node.get('name')!r}" diff --git a/tests/n8n/test_install.py b/tests/n8n/test_install.py index 34ee0d3..10bc1a3 100644 --- a/tests/n8n/test_install.py +++ b/tests/n8n/test_install.py @@ -34,17 +34,27 @@ def test_serving_and_editor(live_app, meta): page = ctx.new_page() import time + from playwright.sync_api import Error as PlaywrightError + deadline = time.time() + 120 resp = None last_status = 0 + last_err = "" while time.time() < deadline: - resp = page.goto(url, wait_until="domcontentloaded", timeout=30000) - last_status = resp.status if resp is not None else 0 + try: + resp = page.goto(url, wait_until="domcontentloaded", timeout=30000) + except PlaywrightError as e: # net::ERR_*, transient navigation, etc. (F2-3) + last_err = str(e) + resp = None + last_status = 0 + else: + last_status = resp.status if resp is not None else 0 if last_status in (200, 304): break time.sleep(3) assert resp is not None and last_status in (200, 304), ( - f"page status {last_status} after polling — n8n route never came up" + f"page status {last_status} after polling — n8n route never came up " + f"(last error: {last_err or 'none'})" ) body = page.content().lower() assert "n8n" in body or "