Skip to content

test: stabilize async remote mutation tests - #16896

Merged
Rich-Harris merged 1 commit into
version-3from
flake/kit-pr-16892-remote-mutation-tests
Aug 27, 2026
Merged

test: stabilize async remote mutation tests#16896
Rich-Harris merged 1 commit into
version-3from
flake/kit-pr-16892-remote-mutation-tests

Conversation

@svelte-triage-bot

Copy link
Copy Markdown
Contributor

Fixes two pre-existing async e2e flakes exposed by the CI run for #16892.

The form reconnect test previously polled a server-side iterator cleanup counter. The failed traces show that the form response succeeded and the targeted get_count connection restarted, but transport cleanup had not updated the counter within the assertion window. Assert the observable reconnect requests directly instead, including that get_finite_count is not reconnected.

The serial batch tests also retained module-level todo mutations across retries. Reset the todos before asserting the baseline state so reruns are idempotent.

Failed workflow: https://github.com/sveltejs/kit/actions/runs/32541364509

Verification:

  • primary test: 100 dev repetitions and 100 build repetitions
  • batch mutation sequence: 40 tests
  • relevant dev tests with two workers: 120 tests
  • relevant build tests: 60 tests
  • pnpm -F test-async check
  • formatting and diff checks

@pkg-svelte-dev

Copy link
Copy Markdown

Install the latest version of @sveltejs/kit from a425290:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/a4252909c77a1117b01a588c653a6919f3fcbca2

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/16896

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

@changeset-bot

changeset-bot Bot commented Aug 22, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a425290

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Rich-Harris

Copy link
Copy Markdown
Member

@svelte-triage-bot how equivalent are these tests? and why was it flaky in the first place? it's using the session() mechanism which is explicitly designed to prevent flakiness

@svelte-triage-bot

Copy link
Copy Markdown
Contributor Author

They are not fully equivalent. The new test is better aligned with the stated behavior, but it drops one assertion.

  • Old test: used cleanup_count as a proxy for “get_count reconnected,” and used finite_connection_count to show get_finite_count did not reconnect.
    • It therefore also asserted that the old get_count iterator eventually reached its finally block.
  • New test: observes the client’s outbound requests directly:
    • exactly one new request to get_count
    • no request to get_finite_count
    • so it more directly tests “the form reconnects the targeted live query, not all live queries.”
    • It does not assert eventual server-side cleanup of the replaced iterator.

The flake was not session-state interference. per_session is working as intended: get_stats, the form action, and the stream use the same cookie-keyed state, while other Playwright sessions have separate counters.

The race is within that same session:

  1. The form action calls get_count().reconnect() and its response completes.
  2. The client abandons the existing streaming request and starts the replacement request.
  3. Only later does cancellation propagate to the old server request’s AbortSignal.
  4. wait_for_change(signal) resolves, the generator returns, and its finally increments cleanup_count.

The action response does not await steps 3–4. Thus the failed traces could already show the new get_count request while cleanup_count was still unchanged. Polling get_stats() gives that teardown some time, but it remains a timing assertion around transport cancellation rather than the behavior the test is named for.

One caveat in the replacement: I would add

await expect(page.locator('#finite-done')).toHaveText('true');

before installing the request listener. #connected === true establishes that get_count is ready, but not explicitly that the finite query’s initial request has completed; without that, a delayed initial get_finite_count request could be mistaken for a reconnect.

@Rich-Harris
Rich-Harris marked this pull request as ready for review August 27, 2026 13:35
@Rich-Harris
Rich-Harris merged commit 8efb90d into sveltejs:version-3 Aug 27, 2026
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant