Skip to content

[fixtures] Skip dev-registry tests on Windows - #15018

Merged
petebacondarwin merged 1 commit into
mainfrom
fix/skip-dev-registry-tests-on-windows
Aug 4, 2026
Merged

[fixtures] Skip dev-registry tests on Windows#15018
petebacondarwin merged 1 commit into
mainfrom
fix/skip-dev-registry-tests-on-windows

Conversation

@petebacondarwin

Copy link
Copy Markdown
Contributor

The fixtures/dev-registry suite is consistently failing on Windows in CI and is blocking unrelated work. This skips it on Windows as a temporary measure to unblock the pipeline, while keeping full coverage on macOS and Linux.

The fixture has five top-level describe blocks, so rather than wrapping the whole file in a platform conditional (as fixtures/interactive-dev-tests/tests/index.test.ts does), this aliases the vitest import and derives a locally-skipped describe:

const describe = baseDescribe.skipIf(process.platform === "win32");

That covers all five blocks in a five-line diff with no re-indentation, which keeps the eventual revert trivial.

Verification:

  • All 27 tests live inside those five describe blocks — no top-level it/test escapes the skip.
  • Forcing the condition to true locally reports 27 skipped and exits 0, so Windows goes green rather than erroring on an empty test file.
  • With the real check in place, vitest list still enumerates all 27 tests on macOS.
  • oxfmt --check and check:type both pass. Lint is not applicable — .oxlintrc.jsonc ignores fixtures/**.

Note that this is a blanket file-level skip, so any new Windows regression in the dev registry will also go unnoticed on that platform, not just the current failure. The TODO marks it as temporary but nothing enforces cleanup, so this should be reverted once the root cause is fixed.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: this is a test-only change that skips an existing suite on one platform. There is no product behaviour change to cover. The skip itself was verified locally as described above.
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this only affects which platforms an internal test fixture runs on, with no user-facing impact.

A picture of a cute animal (not mandatory, but encouraged)

These tests are consistently failing on Windows in CI and are blocking other
work. Skip them on Windows as a temporary measure until the underlying issue
is fixed. There is still value in running them on macOS and Linux.
@petebacondarwin petebacondarwin added ci:no-changeset-required Skip pull request checks for a valid changeset ci:no-tests Skip pull request checks for tests labels Aug 4, 2026
@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Aug 4, 2026
@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a637a3b

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

@ask-bonk

ask-bonk Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

All five describe blocks are top-level and use the local aliased describe. There are no top-level it/test calls escaping the skip. Let me verify the approach matches the referenced pattern and confirm skipIf returns a properly-typed describe.

The change is correct:

  • baseDescribe.skipIf(...) is a valid vitest API returning a describe-compatible function
  • The local const describe shadows the import within the module scope, and all five describe blocks are declared after line 24, so they resolve to the shadowed const (no TDZ issue since they execute after the const initialization)
  • All 27 tests are nested inside the five blocks
  • On non-Windows the condition is false, so tests run normally

The author's own PR description already thoroughly documents the tradeoffs (blanket skip masks future Windows regressions, TODO not enforced). These are acknowledged design decisions for a temporary unblock, not bugs. The implementation is correct and minimal.

No logic bugs, security issues, backward-compat violations, or incorrect API behavior found.

LGTM

github run

@dario-piotrowicz dario-piotrowicz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @petebacondarwin 🙏

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Aug 4, 2026
@pkg-pr-new

pkg-pr-new Bot commented Aug 4, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@15018

@cloudflare/build-output-utils

npm i https://pkg.pr.new/@cloudflare/build-output-utils@15018

@cloudflare/config

npm i https://pkg.pr.new/@cloudflare/config@15018

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@15018

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@15018

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@15018

miniflare

npm i https://pkg.pr.new/miniflare@15018

@cloudflare/pages-functions

npm i https://pkg.pr.new/@cloudflare/pages-functions@15018

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@15018

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@15018

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@15018

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@15018

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@15018

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@15018

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@15018

wrangler

npm i https://pkg.pr.new/wrangler@15018

commit: a637a3b

@petebacondarwin
petebacondarwin marked this pull request as ready for review August 4, 2026 15:22
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@petebacondarwin

Copy link
Copy Markdown
Contributor Author

Let's merge this one for today, so that we can get the release out and unblock other PRs.
In the meantime we can look at #15017 as a possible mitigation.

@petebacondarwin
petebacondarwin enabled auto-merge (squash) August 4, 2026 15:23
@petebacondarwin
petebacondarwin merged commit b3594f9 into main Aug 4, 2026
71 of 80 checks passed
@petebacondarwin
petebacondarwin deleted the fix/skip-dev-registry-tests-on-windows branch August 4, 2026 15:24
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Aug 4, 2026
petebacondarwin added a commit that referenced this pull request Aug 4, 2026
Reverts the temporary skip from #15018, which disabled the whole suite on
Windows to unblock other work "until the underlying issue is fixed". The
preceding commit fixes it: the timeouts were a workerd abort during teardown
(cloudflare/workerd#6913), triggered by killing a dev session that a still
running session was forwarding tail events to, and the fixture no longer sets
that up.

Windows is the only platform the dev registry flake ever appeared on, so
leaving these skipped there would mean the suite never guards the thing it
exists to guard.
petebacondarwin added a commit that referenced this pull request Aug 4, 2026
Measures the baseline Windows flake rate under the same 6x harness. Reverts
both the teardown fix and #15018's Windows skip, so the original cyclic
tail_consumers are back and the tests actually execute on Windows.
petebacondarwin added a commit that referenced this pull request Aug 4, 2026
Reverts the temporary skip from #15018, which disabled the whole suite on
Windows to unblock other work "until the underlying issue is fixed". The
preceding commit fixes it: the timeouts were a workerd abort during teardown
(cloudflare/workerd#6913), triggered by killing a dev session that a still
running session was forwarding tail events to, and the fixture no longer sets
that up.

Windows is the only platform the dev registry flake ever appeared on, so
leaving these skipped there would mean the suite never guards the thing it
exists to guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:no-changeset-required Skip pull request checks for a valid changeset ci:no-tests Skip pull request checks for tests

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants