-
Notifications
You must be signed in to change notification settings - Fork 778
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
chore: include banner in output of tests for commands with json/pretty mode #8109
base: main
Are you sure you want to change the base?
Conversation
|
@@ -21,6 +21,9 @@ import type { | |||
import type { RequestInit } from "undici"; | |||
import type WebSocket from "ws"; | |||
|
|||
// we want to include the banner to make sure it doesn't show up in |
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.
Do you mean:
// we want to include the banner to make sure it doesn't show up in | |
// we want to include the banner to make sure it shows up in |
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.
Also I have no clue about what "json mode" is?
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.
i've update the comments and PR description, but tldr we want to test for the absence of the banner, so we need to enable banner printing in the first place
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/13285046202/npm-package-wrangler-8109 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/8109/npm-package-wrangler-8109 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-wrangler-8109 dev path/to/script.js Additional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-workers-bindings-extension-8109 -O ./cloudflare-workers-bindings-extension.0.0.0-vf63a9adb1.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vf63a9adb1.vsix create-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-create-cloudflare-8109 --no-auto-update @cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-kv-asset-handler-8109 miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-miniflare-8109 @cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-pages-shared-8109 @cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-unenv-preset-8109 @cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-vite-plugin-8109 @cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-vitest-pool-workers-8109 @cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-workers-editor-shared-8109 @cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-workers-shared-8109 @cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13285046202/npm-package-cloudflare-workflows-shared-8109 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
Out of curiosity: instead of each command having to think about |
yeah? i don't know if it would be huge benefit at the moment as the switches are so inconsistent: sometimes we use |
The wrangler banner is normally mocked in tests so it isn't included in the console output that we check. However, we do want to test for the absence of the banner when a particular command has a 'json' mode where the output should be json only.
This PR adds/updates tests for d1 and tail commands that have a 'json' option.
There may be some commands that have an implicit guarantee that they output json without a separate flag, but I don't have a good way of identifying those -
secret list
now has tests for it but there may be others?This should ensure issues like #7487 and #8101 don't happen again 😅