Skip to content

[wrangler] Quieten warnings and stray output in tests - #15209

Merged
petebacondarwin merged 3 commits into
mainfrom
pbd/test-output-noise
Aug 17, 2026
Merged

petebacondarwin merged 3 commits into
mainfrom
pbd/test-output-noise

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Removes three sources of noise from the test output. No behaviour change — test and test-config only.

__dirname in ESM configs. Vite warns when loading an ESM config that uses __dirname, once per site:

- `__dirname` (vitest.config.mts:8:33). Use `import.meta.dirname` instead

Switched to import.meta.dirname across 14 Vite/Vitest configs (31 sites). Safe: these packages require Node >= 22, and import.meta.dirname has been available since 20.11.

A vi.mock call inside a describe. versions.upload.test.ts mocked child_process from within a describe body, which Vitest hoists to module scope anyway:

Warning: A vi.mock("child_process") call in ".../versions.upload.test.ts" is not at the top level of
the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it
to the top level to reflect its actual execution order. This will become an error in a future version.

Moved to module scope, so the code now reflects both when it runs and what it affects — the mock applies file-wide, not just to that describe. Vitest says this will become a hard error, so it needs doing regardless.

Errors leaking to stderr. ai.local.test.ts asserted on vi.spyOn(logger, "error"), which observes the call but does not capture the output, so a real error was printed on every run:

✘ [ERROR] Authentication error (code 1031): Your API token may have expired or lacks the required
permissions. Please refresh your token by running `wrangler login`.

Switched to mockConsoleMethods() with inline snapshots, matching the pattern used across the rest of the suite. This also tightens the two negative cases: they previously asserted only that logger.error was not called, and now assert that nothing at all was written to stderr.

Similarly, two console.warn spies in the context-exports fixture called through to the real implementation, printing the warnings they assert on. A third spy was never asserted against at all, so it is dropped.

Verified by running the affected files before and after: the warnings are gone and the tests still pass.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: test-only change with no user-facing impact.

@changeset-bot

changeset-bot Bot commented Aug 15, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a061429

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

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Aug 15, 2026
@petebacondarwin petebacondarwin added the ci:no-changeset-required Skip pull request checks for a valid changeset label Aug 15, 2026
@workers-devprod
workers-devprod requested review from a team and penalosa and removed request for a team August 15, 2026 18:05
@workers-devprod

workers-devprod commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@ask-bonk

ask-bonk Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

The message matches the snapshot. The change is sound. Let me review the whole change set holistically once more.

Summary of my analysis:

  1. vitest.config.mts: __dirname → import.meta.dirname. Node >= 22 confirmed, import.meta.dirname available since 20.11. Correct.

  2. project-create.test.ts: Moved vi.mock to module scope. Vitest hoists it anyway, so behavior is identical. Correct. (Note: the mock targets "getConfigCache" which is not a real module path — it's a pre-existing no-op that this PR merely relocated, not in scope.)

  3. ai.local.test.ts: Switched from vi.spyOn(logger, "error") to mockConsoleMethods() with inline snapshots. The error message matches source, and the negative cases now assert empty stderr, which is stronger. vi.restoreAllMocks() in the test's afterEach coexists fine with mockConsoleMethods()'s own hooks since spies are recreated each beforeEach. Correct.

All three changes are correct, well-scoped, and test-only. No logic bugs, security issues, or behavior regressions. The changes actually improve test hygiene. No actionable issues.

LGTM

github run

@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

@pkg-pr-new

pkg-pr-new Bot commented Aug 15, 2026 •

Copy link
Copy Markdown
@cloudflare/autoconfig

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

@cloudflare/build-output-utils

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

@cloudflare/config

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

create-cloudflare

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

@cloudflare/deploy-helpers

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-functions

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-auth

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

commit: a061429

@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 17, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

Extends the same fixes to every other instance in the repo:

- Hoist the `child_process` `vi.mock` in `versions.upload` to module
  scope. Written inside a `describe`, it misrepresented its scope: the
  call is hoisted anyway and mocks the module for the whole file. Vitest
  warns this "will become an error in a future version".

- Replace `__dirname` with `import.meta.dirname` in 14 ESM Vite/Vitest
  configs (31 sites). Vite warns on each when loading the config. Node
  >=22 is already required, so `import.meta.dirname` is always available.

- Stop two `console.warn` spies in the context-exports fixture from
  calling through, so the warnings they assert on no longer print. Drop a
  third spy that was never asserted against.
@petebacondarwin
petebacondarwin merged commit 1552bce into main Aug 17, 2026
68 of 70 checks passed
@petebacondarwin
petebacondarwin deleted the pbd/test-output-noise branch August 17, 2026 12:11
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Aug 17, 2026
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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants