Repository navigation
UN-4345 [FEAT] Frontend v1 E2E harness and the frontend-v1-e2e group - #2355
Deepak-Kesavan wants to merge 9 commits into
Conversation
Adds tests/e2e/ui-v1: a self-contained Playwright suite (own config, lock, Biome, strict TS) with login/org, seed, v1Preview and distribution tag fixtures, and five-state, accessibility, theme-matrix, redirect and old-version helpers. P0-T03 dependencies go through contract/registry.ts (stubbed; built ahead of P0-T03). Adds the frontend-v1-e2e rig group and the CI install steps for it. tests/e2e/ui and the ui group are untouched.
$host drops the port, so Django's CSRF same-origin check compared the browser's Origin (http://localhost:3000) with http://localhost and rejected every unsafe request the SPA made. The org handshake failed and browser-driven specs never got past the session loader.
- five-states: don't await open() while the loading response is held, so an open that waits for networkidle (app.goto) cannot deadlock; hand unstubbed rest to route.fallback() so a spec's own routes still apply. - seed: a row created but not resolvable to a delete URL fails teardown. - old-version: assert v0 rendered only after the SPA has mounted. - Self-tests for the impact threshold, the networkidle open, the pass-through path and that a custom setTheme replaces the default.
 [Medium impact] The PR appears safe to merge with its documented temporary stubs and optional test group.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… clear the Sonar findings loginAuth0 now starts where the Login button does (/api/v1/login on the app origin, with the app as Referer so the callback returns there), bounds every IdP wait and gives the session fixture its own budget, so a failed sign-in skips or errors with its reason instead of a generic timeout. Sonar: drop the redundant RoutePattern alias, keep the stub registry from being a provably-empty literal, String.raw and \w in the regex helpers, no nested template, reverse cleanups in place with CSRF headers read once, accept only string or number ids, explicit assertions in the a11y self-tests, and NOSONAR on the sequential test.step loops.
…-tests Cloud CI exports UNSTRACT_RIG_EXTRA_MANIFESTS for the whole job, so load_groups() merged groups.cloud.yaml into the synthetic manifests the test_cli tests patch in. The first cloud requires_platform group (frontend-v1-e2e-cloud, which must depend on e2e-smoke) then failed validation against manifests that never define e2e-smoke. Clear the variable per test; the overlay tests set it themselves.
…its id The frontend's /:orgName segment is organization.name from the /organization/<id>/set response (useSessionValid), which differs from the id on cloud. The session now carries both: orgId for API calls, orgName for app.goto and the registry route checks. A harness self-test pins id != name. Also note that the P0-T01 scaffold renders no v1 root yet, so the marker stays assumed.
|
Unstract test resultsPer-group results
Critical paths
|
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
A few small things from a review pass over the harness. Overall it looks solid: seed teardown, the held-response release in the five-states helper, the $http_host fix and the CI caching all checked out.
| const creds = credentialsFor(role); | ||
| test.skip( | ||
| !creds, | ||
| `no credentials for role "${role}": set UNSTRACT_E2E_${role.toUpperCase()}_USER and _PASSWORD`, |
There was a problem hiding this comment.
The env var named here doesn't match the one credentialsFor() reads. That function sanitises the role (role.toUpperCase().replace(/[^A-Z0-9]/g, "_"), line 47), this message only uppercases it. For role: "org-admin" the skip says to set UNSTRACT_E2E_ORG-ADMIN_USER, but the lookup is UNSTRACT_E2E_ORG_ADMIN_USER, so following the hint never takes effect.
Suggest pulling the key into one helper used by both:
/** The `<ROLE>` in UNSTRACT_E2E_<ROLE>_USER / _PASSWORD. */
function roleEnvKey(role: Role): string {
return role.toUpperCase().replace(/[^A-Z0-9]/g, "_");
}and set UNSTRACT_E2E_${roleEnvKey(role)}_USER and _PASSWORD here.
| export interface Session { | ||
| role: Role; | ||
| mode: AuthMode; | ||
| /** For API calls: `/api/v1/organization/<orgId>/...`. */ |
There was a problem hiding this comment.
Nit: org-scoped API calls go to /api/v1/unstract/<orgId>/... (as seed.ts builds its prefix); /api/v1/organization/<id> is only the list/set handshake. As written this could point spec authors at the wrong path.
| /** For API calls: `/api/v1/organization/<orgId>/...`. */ | |
| /** For API calls: `/api/v1/unstract/<orgId>/...`. */ |
| const value = JSON.stringify(pages); | ||
| if (mechanism.kind === "cookie") { | ||
| await context.addCookies([ | ||
| { name: mechanism.name, value, url: FRONTEND_URL }, |
There was a problem hiding this comment.
Not urgent since V1_PREVIEW is null today, but worth settling when P0-T03 defines the mechanism: for the cookie kind, the value is raw JSON (["UN13"]). Double quotes and commas aren't valid in a cookie-value under RFC 6265, and some server-side parsers mangle them. If the mechanism ends up being a cookie, it may be worth agreeing on an encoding (e.g. encodeURIComponent, or a comma-free format) on both sides.
What
Adds the frontend v1 E2E harness (P0-T09, UN-4345) and the
frontend-v1-e2erig group, so every v1 page task can test itself in a real browser and report apart from v0.Why
The page definition of done needs E2E, five states, Light/Dark/Unstract and a clean accessibility scan for every page, reported under frontend v1 and never mixed into v0 results (constraint 12, D10).
How
tests/e2e/ui-v1/: a self-contained suite, the same pattern astests/e2e/ui. It has its ownpackage.json+bun.lock,playwright.config.ts, a stricttsconfig.jsonandbiome.json. It is all TypeScript and imports nothing fromtests/e2e/ui/.fixtures/):session/app/role: login as a seeded user of a given role (OSS mock-login, or auth0's hosted form) plus org selection.seed: rows created through the org-scoped API and deleted after the test. A leak fails the test.v1Preview: renders an unregistered page.DIST/TAG: distribution tags. The config runs only tests tagged forUNSTRACT_DISTRIBUTION.helpers/):expectFiveStates: rest, loading, empty, error and no-permission by intercepting the screen's API. A held response is always released.expectAccessible:@axe-core/playwright, WCAG 2.0/2.1 A+AA.forEachTheme: theme matrix with a pluggablesetTheme.expectV0RedirectsToV1expectOldVersionRoundTrip: page 11 only.contract/registry.tsis the only place the harness reads P0-T03 from: the registry, the old-version preference key and the preview mechanism. All three are stubbed.contract/v1-ui.tsholds the assumed DOM markers.specs/P0-T09.demo.spec.ts:page.routestub that follows the v1-ui markers. It runs five states, an enforcing a11y scan and the theme matrix. It proves the helpers, not the version switch.frontend-v1-e2eintests/groups.yaml(requires_platform,depends_on: [e2e-smoke],optional: truefor now). Theuigroup is untouched.ci-test.yaml, e2e job): Bun, a cachedbun install --frozen-lockfileintests/e2e/ui-v1, and a cached Playwright Chromium. The leg runs the whole e2e tier, so the group is always selected and the caches keep that cheap. Theuigroup's CI is unchanged.tests/e2e/ui-v1, frontend/ untouched, exact pins, all over two weeks old):@axe-core/playwright4.13.0,@playwright/test1.62.1 (same asui),@biomejs/biome2.3.13 andtypescript7.0.2 (both match frontend's lock),@types/node22.20.3.Needed from P0-T03 (lane A)
import.meta.env, no tsconfig path aliases, no asset imports. Playwright transpiles TS itself and applies none of those. Emitting JSON is an alternative.Open questions
seriousand above by default (UNSTRACT_E2E_A11Y_IMPACToverrides it). The DoD says "accessibility scan clean"; P8-T02 owns the final level. @deepak to confirm.[data-ui-version="v1"], state test idsv1-state-*, and theme hookdata-theme+ class on<html>.Flip when P0-T01 / P0-T03 land
contract/registry.ts: re-export the real registry and setREGISTRY_SOURCE = "frontend". The redirect and old-version self-tests then run.contract/registry.ts: the realOLD_VERSION_PREFERENCEkey.contract/registry.ts:V1_PREVIEWset to P0-T03's override, plus the demo key/route inspecs/harness/preview.spec.ts.specs/P0-T09.demo.spec.tspart 2: point at the real dev-only demo route, then deletesupport/demo-page.ts.contract/v1-ui.ts: the real v1 root marker and the core state component ids (P1). The P0-T01 scaffold (UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary #2380) renders no DOM:frontend/src/v1/index.tsisexport {}until the version switch (UN-4271, P0-T03) mounts v1. So the marker comes from that mount, and[data-ui-version="v1"]stays assumed until then.helpers/themes.ts:defaultSetThemeset to P0-T04's mechanism. P0-T04 putsdata-themeon the v1 root, not on<html>, and adds a#v1-portal. The default theme hook then targets the v1 root, and the theme and a11y checks also cover#v1-portal, so overlays rendered there are themed and scanned.optional: trueonfrontend-v1-e2eonce CI shows it green.Can this PR break any functionality?
No app code changes. The new group is optional. Two shared changes:
Host(above). That changes behaviour only in the compose test overlay and only lets requests through that were wrongly rejected.tests/e2e/uiand theuigroup are unchanged.Related
How to test
Through the rig, exactly as the CI e2e job runs it:
Local run (compose runtime,
latestimages, an isolated project on shifted ports):e2e-smokeuifrontend-v1-e2ee2e-workflow/e2e-prompt-studio/e2e-login/e2e-coowners/e2e-api-deploymente2e-etlfrontend-v1-e2ereports as its own group. The 2 skips are the registry-driven check and the preview self-test, both waiting on P0-T03 (each skip carries its reason).uiis red locally only because it has nonode_moduleshere, the same as in CI. It is optional and untouched by this PR.e2e-etlfailed an ETL execution on the publishedlatestimages. This PR has no backend change, and main's CI is green.tox -e groups -- frontend-v1-e2ealone also pulls inunit-sdk1as a dependency. That group fails locally because the rig exportsUNSTRACT_LLM_MOCK_RESPONSEfor any selection with a platform group, which leaks into the sdk1 streaming tests. This is pre-existing and not touched here;tox -e e2e, like CI, avoids it.Also fixed: the e2e frontend proxy rejected every SPA write
tests/compose/frontend-nginx.test.confforwardedHost $host, which drops the port. Django's CSRF same-origin check then compared the browser'sOrigin: http://localhost:3000withhttp://localhostand rejected every unsafe request:CSRF Failed: Origin checking failed. The SPA's org handshake (POST /organization/<id>/set) got a 403, and the app sat on "Please wait while we prepare your session" forever. It is now$http_host. This affected the v0uisuite too; it went unnoticed becauseuinever had its dependencies installed in CI.Checklist
tsc --noEmitclean intests/e2e/ui-v1