test(cycle): the walk asserts the refusals it collects - #702
Merged
Conversation
The pinned-list comment attributed the annotation-draft 404s to the schema editor and said the second read of each pair follows a write; both are wrong. The pair comes from the annotator's add-a-class dialog, gated on addingClass, and the second read is a remount or page.reload() — a saved draft is written straight into the cache rather than invalidated, so a write never triggers a refetch on its own. The size-line wait relied on connection-model being visible to guarantee the seed landed on a curated model, but the seeding effect falls back to Custom model with an empty id when the catalog offers nothing that answers a point prompt, and DownloadSizeLine renders nothing for an empty id — so that installation would die on a bare 15-second timeout with nothing to explain it. Add an explicit assertion that the select shows a curated model before waiting on the price probe only a curated model triggers. Also trims the apiRefusals comment from four sentences to three, cutting an opening clause that only summarized what a later sentence already said properly.
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
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.
The cycle walk collected every API call the application made that the API refused into a
Set, and read that set exactly once — to suppress a console error whose location matched a member. Nothing asserted it. Its two neighbouring collectors are both asserted in the walk's final step,badRequestsagainst an empty array andabortedApiCallsagainst an exact two-element list, soapiRefusalswas the only one of the three that existed solely to make an assertion pass, and the only one that grew without bound. A genuine 500 in a walked route reached the suite as the same thing as an expected 404: added to the set, exempted from the console assertion, never looked at, and the step titled "the whole walk produced a clean console" reported a clean console.This adds a second collection alongside the set. The set stays keyed by URL, because that is what a console message's location carries and the suppression genuinely needs it; the new
refusedApiCallslist is keyed by method, path and status, because that is what a person can check, and it is pinned in the final step with an exacttoEqual.The conditional entry, and why it is read rather than assumed
Six of the seven expected refusals are 404s for documents that do not exist yet, and they are the same on every installation. The seventh is not.
GET /inference/download-sizeprices the model the connection form seeds itself with, and reading that price needs the optional local-inference runtime, so it answers 500 wherever the extra is absent — which is what thebrowser cycle (chromium)job installs — and 200 wherever it is present.Rather than guess at the environment, the walk reads what the form rendered.
DownloadSizeLineshowssize-knownorsize-unavailable, and the expectation splices the500in exactly when the second one is what appeared. That keeps the entry bounded rather than merely conditional: the method, path, status and position stay pinned literals, so a probe that started answering 404 or 422 would still rendersize-unavailable, still set the flag, and still fail the assertion.Making that readable required one behavioural change to the walk. The seeded model's probe previously raced the walk's own typing — on one machine it fired as the dialog tore down after submit rather than when the dialog opened — so where the refusal landed, and whether it landed at all, was timing. The walk now waits for the size line before touching the model select, which orders the probe without changing what is under test.
Test plan
The gate for this change is the walk failing when it should. Four runs, in order, each on a base install where the curated model's probe genuinely answers 500:
1 passed (19.8s)— the defect1 passed (21.5s)— the defect, amplifiedinference_download_sizetemporarily raising for every model1 failed1 passed (24.1s)Run C is the one that matters, and it named the route and the status:
The temporary raise was never committed, and the diff touches one file.
Both halves of the inference matrix were run against the same pinned array.
uv sync --lockedproduces the installation the browser cycle job installs, and there the list is seven entries with the500at index 4.uv sync --locked --extra local-inferenceproduces the other half, and there the flag readsfalse, the entry is spliced out, and the list is six. Neither half required a different array.Full gate on the rebased branch:
One axis this branch cannot verify
The assertion has never executed on a CI runner, because it did not exist until this branch and this branch has not run there. Two mechanisms could legitimately produce a different exact list on a slower machine, and both are measurement rather than regression if they appear. The autosave debounce in
SchemaTabis 400 ms and the draft write populates the cache rather than invalidating it, so whether a givenGET .../schema/drafts/curatedanswers 404 or 200 depends on whether that timer fired before the walk's tab switch and reload; if it lands earlier on a runner, the list is short by one. Separately, the active schema and the curated draft are requested in the same render, so they complete in whatever order the server's threadpool finishes them, and the pair could invert.If the browser cycle job goes red on either, the repair is to re-measure and pin what it reports. Widening the assertion —
expect.arrayContaining, sorting both sides, or dropping the conditional entry — would restore precisely the blind spot this change closes. Worth noting that the cycle config sets one retry in CI, so an inversion would surface as a flaky pass, which must not be read as a pass.Found, not fixed
A full gate run on this branch failed at exit 1 while all 1149
ui-coretests passed, because a@radix-ui/react-focus-scopeteardown timer fired after jsdom had torn down the document that owned its element. It did not reproduce in isolation on this branch or on unmodifiedmainatda3304a, both of which came back clean at exit 0, so it is a contention-sensitive race rather than a standing red or anything this branch introduced — the diff here is one Playwright spec that theui-corevitest run never loads. It is filed with its stack and both reproduction attempts as the focus-scope teardown race (#701) and is not addressed here.The seeded model's probe fires once more than it needs to, when the create dialog resets to its curated default as it closes after a connection is submitted. It is harmless, it predates this branch, and pinning the list documents it rather than changing it.
Closes #697