fix(e2e): harness reliability — measured failure-class fixes#5655
Merged
norman-abramovitz merged 30 commits intoJul 19, 2026
Conversation
auth.setup.ts resolves org/space GUIDs via the authenticated backend proxy after login and writes them to e2e/.auth/cf-guids.json, keyed by CF endpoint name. Adds CF_GUIDS_FILE to auth.constants.ts.
resolveEndpointGuids now checks the file written by auth.setup.ts first, falling back to the cf CLI (with a warning) only when a persisted entry is missing. This avoids the parallel-worker race on the shared ~/.cf/config.json that cf target caused. Also memoizes SecretsHelper.load() per process — it was re-parsing YAML and re-shelling cf up to 8x per test.
Wrap the endpoints-list fetch/parse and the resolveAndPersistCfGuids call site in try/catch, and guard against a non-array response body. An unguarded throw there previously escaped the admin setup test before storageState was written, cascade-failing every downstream test. GUID resolution is a bonus, not a login requirement. Also joins CF_GUIDS_FILE against process.cwd() before writing, to match the read side in secrets-helpers.ts.
Dotted names from createCustomName() are reused as route hosts; CF rejects dotted hosts with 422 Host must be alphanumeric.
createAndMapRoute's routes were never deleted — app delete does not cascade to routes, leaving orphaned route debris. Track route GUIDs on TestApp, delete them before the app in cleanupTestApp, and log failures with resource names instead of a guid-only message.
Standalone script (scripts/e2e-clean.mjs) that deletes every CF resource carrying the stratos-e2e-test label, direct against the CF API: apps, routes, service instances, spaces, then orgs, polling each async delete's job to completion before moving to the next resource type. Every resource is re-checked for the label before deletion; any miss aborts the whole sweep rather than risk deleting something broader. Wired as `make e2e clean`, deliberately not part of global-teardown so runs stay fast — an operator sweeps between runs.
A 404 on delete means the resource vanished between our list and delete calls — the end state (gone) is what we wanted, so count it as cleaned rather than FAILED. Also warn instead of silently claiming "deleted" when a 202 response has no job Location header, and drop the header comment's reference to a TESTING.md section that doesn't exist.
Setup only connected the admin identity, leaving the user identity without a CNSI token row. Proxied requests riding that identity then hit an empty-body 400 at the token lookup.
Verify the endpoint registry is non-empty and the user identity has a connected CF token before handing the fixture to a test. Fails loudly and by name instead of letting the cause surface later as an unexplained empty-body 400.
reuseExistingServer:true silently reuses a leftover backend process (and its stale sqlite DB) if a prior run's teardown didn't complete. Compare the DB file's mtime to the run's start time and log an unmistakable marker so this is visible instead of silent.
Isolates the one spec that mutates shared endpoint-registry state so it can be sequenced separately from the parallel, self-contained pool. Updates the legacy-files ratchet list path to match.
Any selection touching e2e/tests/dependent/ (tier, group, impact, or explicit files) now splits into two passes: everything else first, dependent/ last. A selection with only one side stays a single pass. Handles tag-based selections (e.g. smoke) the same way.
Explains what belongs in dependent/, that the runner always orders it last, and the measured rationale: a failed endpoint-registry restore cascaded ~200 downstream failures in a full run.
Sibling describe blocks in application-deploy.spec.ts referenced this helper as a function declaration scoped to a different describe closure, causing a ReferenceError in 4 tests. No signature change.
application-wall.spec.ts passed the whole connectedEndpointsUserPage fixture object into ApplicationsPage, whose constructor dereferences .locator on it, throwing "page.locator is not a function" in 4 tests. Pass .page instead.
Playwright's test timeout is a Promise.race; the losing promise (a hung cfApi/connectedEndpointsUserPage network call) is never cancelled, so a stuck worker waits out TCP-level timeouts on top of the test timeout (measured: 32.5 minutes for one hang). Wrap the cfApi fixture's initFromStorageState/endpoints GET and the connectedEndpointsUserPage guard's own GETs in an explicit 15s deadline race so a stuck call throws a named E2E_DEADLINE_EXCEEDED error instead. Also add a 6h globalTimeout as a run-level backstop (full 3-browser suite runs ~4h; leaves headroom for future tiers).
Config-driven capability declaration for the secrets profile schema,
replacing UI-probe skip guards that can't tell 'not installed' apart
from 'broken selector' or 'slow page'.
- capabilities?: { [name]: boolean } on the secrets profile schema
- requireCapability(name) test.skip guard, no UI probing
- absent map or absent key both skip with a named reason (unknown
is never treated as available)
- template documents the new optional section
Converts metrics-registration.spec.ts's probe-based skip guards to
requireCapability('metrics') per the capability-flag pilot.
Data from a full chromium run showed metrics was the only one of the
three candidate capabilities (autoscaler, sso, metrics) whose probes
actually ran the underlying flow rather than skip on config/feature
absence. Verified metrics=true runs both tests (0 skipped, 2 known-
baseline failures traced to a stale locator and a stale route — both
pre-existing, unrelated to this change); metrics=false skips both
with the named reason; an absent capabilities map skips the same way.
Mechanical sweep of the remaining probe-guard sites is a follow-up.
Compares two or more Playwright results.json files and classifies every test that failed in any run as deterministic, flaky, or (for exactly two ordered runs) fixed/regressed. Reports per-run totals and an error-class summary. Feeds the suite-fix verification harness and a known-flakes list for docs.
playwright.config.ts sets fullyParallel:true globally; the setup project didn't override it, so 'authenticate as admin' and 'authenticate as user' could run in separate workers on a cold DB. Both call registerDefaultCloudFoundry(), whose idempotence check is check-then-act (GET-then-POST) — concurrent callers can both observe 'not registered' and both POST, duplicating the CF endpoint and breaking every list page with 'Request failed'. Set fullyParallel:false on the setup project (not mode:'serial', which would skip user setup entirely if admin setup fails) so the two setup tests run one at a time. Corrected the in-code comment that claimed idempotence alone made the race safe.
connectedEndpointsAdminPage did a raw page.request.get with no deadline and no guards, while connectedEndpointsUserPage had withDeadline plus registry-empty and no-CF-token guards. Admin-state specs are the majority of the suite, so the same failure mode (unbounded wait, unexplained 'No CF endpoint found' cascade) hit them too. Extracted the shared logic into resolveConnectedCfEndpoint(page, label) and used it from both fixtures, instead of duplicating the guard block a second time.
SecretsHelper.resolveEndpointGuids consumed e2e/.auth/cf-guids.json unconditionally. If this run's setup failed to resolve GUIDs (fail-open) or the org/space was deleted since (e.g. by the clean sweep), tests silently used last-run GUIDs and produced confusing 404s. resolveAndPersistCfGuids now stamps the file with a __meta block (profile, apiUrl, writtenAt) alongside the per-endpoint entries. resolveEndpointGuids warns loudly, via warnIfGuidsStale, when E2E_RUN_START predates writtenAt or the stamped profile/apiUrl mismatch the active ones — still consumes the file either way; this stays fail-open, just louder.
TESTING.md claimed the dependent/-last ordering applied to an 'explicit file list' selection, which scripts/e2e-run.mjs doesn't support (only --tier/--group). Also documented that the ordering is enforced only by the tiered runner: a bare 'npx playwright test' or 'make test e2e' interleaves dependent/ with the parallel pool, and 'make check e2e' scopes to core/ and no longer covers the endpoint lifecycle spec, which now lives in dependent/. In global-setup.ts, the 'unknown' boot-status branch said 'no db file found' even when the file exists but E2E_RUN_START is unset — split into a distinct message naming the missing anchor instead of the file. Also noted the mtime-vs-run-start check is heuristic: a reused backend writing between config eval and this check can misreport.
An unresolvable E2E_PROFILE previously fell back to top-level secrets, silently sweeping an operator-unselected CF on a typo'd profile name. Now errors and lists available profiles, mirroring SecretsHelper.loadUncached. Legacy top-level-config files (no 'profiles:' key) are unaffected.
_e2e_toggle only recognized literal 'yes', so DRYRUN=1/true/on silently ran a real deletion sweep instead of previewing it. Makefile now passes DRYRUN through unfiltered as an env var; e2e-clean.mjs accepts yes/true/1/on (case-insensitive) as dry-run, empty/absent as a real run, and rejects any other value with exit 2 naming the accepted spellings.
connectAllEndpoints(user) threw on any non-2xx, aborting 'authenticate as user' before storageState was written — since every browser project depends on setup, this skipped the whole suite. Wrapped in try/catch with a loud warn; the per-test E2E_NO_CF_TOKEN guard is the designed loud failure for tests that actually need the missing token. Also removed connectAllEndpoints' silent admin-cred fallback for the user identity: a profile missing creds.nonAdmin now throws E2E_NO_NONADMIN_CREDS instead of minting the 'user' token with admin credentials, which would make permission-scoped tests pass vacuously. This composes with the fail-open change above: the throw is caught and warned, and per-test guards fail loudly instead. Admin-identity path is unchanged.
Clarify that SESSION_STORE_EXPIRY only takes effect on a fresh backend boot; a reused jetstream process keeps its prior value. Points at the BACKEND BOOT MARKER line for the fresh-vs-reused signal.
The capability-flag pilot (requireCapability + SecretsCapabilities) gated the metrics-registration tests on an operator-maintained config key. Rejected: e2e specs must stand on their own — the metrics registration option is built into the register-endpoint dialog, so the tests should simply run and fail honestly if it breaks. If a spec needs environment-specific data it reads the secrets file directly. Removes requireCapability() and its two call sites, the SecretsCapabilities interface and profile field, the related exports, and the template's capabilities documentation block. The prior UI-probe skip guards stay removed (brittle, false-negative prone).
nabramovitz
force-pushed
the
norm/fix/e2e-harness
branch
from
July 19, 2026 15:06
02ec509 to
ce86d7f
Compare
norman-abramovitz
approved these changes
Jul 19, 2026
norman-abramovitz
left a comment
norman-abramovitz
left a comment
Contributor
There was a problem hiding this comment.
LGTM - More testing is needed but this forms basis of the e2e correction worik
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was stacked on #5653 (tiered e2e dispatch); now rebased onto develop
after that merge, so the diff is just this PR's 30 commits.
This addresses the failure classes measured in a full-suite chromium
baseline run (725 tests): GUID-resolution races against the cf CLI,
422s from dotted route hostnames, empty-body 400s from a missing
identity-user CF token, an endpoint-registry wipe cascading into ~200
downstream failures, and individual tests hanging for 30+ minutes.
What changed:
(
e2e/.auth/cf-guids.json) through the proxy; tests read the fileinstead of racing the cf CLI. Stale-file detection warns when the
profile or API URL changed.
"Host must be" errors from route creation.
already-gone resources as success, and a standalone
make e2e cleansweeps everything carrying the e2e label(apps, routes, service instances, spaces, orgs) with a DRYRUN preview
mode and a hard fail on unknown profiles.
endpoint during setup, and both fixtures guard with named errors
(
E2E_ENDPOINT_REGISTRY_EMPTY,E2E_NO_CF_TOKEN) instead of lettinga wiped registry or missing token cascade into hundreds of opaque
failures. The user identity is never minted with admin credentials —
if the profile lacks
creds.nonAdmin, affected tests fail loudlyrather than silently testing admin permissions.
endpoints.spec.tsmoved to adependent/group the runner alwayssequences last; it deliberately unregisters/re-registers endpoints
and previously wrecked every spec scheduled after it.
definition, a page-object constructor passed the wrong type, and
15-second deadline races around fixture waits plus a 6h global
timeout, replacing open-ended hangs.
boot marker that reports fresh-boot vs reuse per run.
scripts/e2e-flake-diff.mjscompares two results.json files andclassifies tests as deterministic / flaky / fixed / regressed across
runs (test identity is file + title chain, so it works across
projects).
Verification: the empty-400 class went 14 to 0 in an isolated re-run of
the affected specs; the 422 class is zero across a full pass; the
deadline guard converts a measured 32.5-minute hang into a 15-second
named failure; a full post-fix chromium run completed in 1.2h (baseline
1.7h) with no test exceeding 20 minutes and zero occurrences of the
GUID-race, 422, ReferenceError, or constructor classes. Suites that
depended on the admin-credential fallback now fail with
E2E_NO_CF_TOKENuntil the profile provides non-admin CF credentials;that surfacing is intentional.