-
Notifications
You must be signed in to change notification settings - Fork 1k
Prevent infinite console.log()/console.info() loops with Wrangler E2E tests
#8163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-wrangler-8163You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/8163/npm-package-wrangler-8163Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-wrangler-8163 dev path/to/script.jsAdditional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-workers-bindings-extension-8163 -O ./cloudflare-workers-bindings-extension.0.0.0-v9e5c4e441.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v9e5c4e441.vsixcreate-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-create-cloudflare-8163 --no-auto-update@cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-kv-asset-handler-8163miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-miniflare-8163@cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-pages-shared-8163@cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-unenv-preset-8163@cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-vite-plugin-8163@cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-vitest-pool-workers-8163@cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-workers-editor-shared-8163@cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-workers-shared-8163@cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13371164498/npm-package-cloudflare-workflows-shared-8163Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
petebacondarwin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of e2e tests failed but are probably classic flakes.
But there is a linting error to resolve: https://github.com/cloudflare/workers-sdk/actions/runs/13369830840/job/37335707533?pr=8163#step:5:941
| import type { DevToolsEvent } from "../src/api"; | ||
|
|
||
| const OPTIONS = [{ remote: false }, { remote: true }] as const; | ||
| const OPTIONS = [{ remote: true }, { remote: false }] as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? I think it was originally the other way round to get a fast fail.
One of the Wrangler E2E tests is checking to see if inspector messages >1MB can be succesfully sent (a regression test for a bug introduced during phase 1 of the start dev worker work). However, since we don't really want to actually log a 1MB message during tests, we had been mocking the output of
console.log()/console.info(). In Vitest v3, the way we'd done this seems to have been causing an infinite loop, so instead I've replaced the mock implementation with an empty function. We don't really care about any console output in the specific test, so that shouldn't cause any issues.