Skip to content

Commit

Permalink
polish: improve consistency of warnings and errors
Browse files Browse the repository at this point in the history
Related to cloudflare#377
  • Loading branch information
petebacondarwin committed Apr 26, 2022
1 parent 0c582be commit fbad3f3
Show file tree
Hide file tree
Showing 45 changed files with 701 additions and 511 deletions.
7 changes: 7 additions & 0 deletions .changeset/dirty-knives-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

polish: improve consistency of warnings and errors

Related to #377
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"cfetch",
"clipboardy",
"cloudflared",
"Codespaces",
"esbuild",
"eslintcache",
"execa",
Expand Down
20 changes: 10 additions & 10 deletions packages/wrangler/src/__tests__/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ describe("normalizeAndValidateConfig()", () => {
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- 🚨 NO LONGER SUPPORTED: \\"site.entry-point\\":
- NO LONGER SUPPORTED: \\"site.entry-point\\":
The \`site.entry-point\` config field is no longer used.
The entry-point should be specified via the command line or the \`main\` config field."
`);
Expand Down Expand Up @@ -534,9 +534,9 @@ describe("normalizeAndValidateConfig()", () => {
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- ⚠️ DEPRECATION: \\"type\\":
- DEPRECATION: \\"type\\":
DO NOT USE THIS. Most common features now work out of the box with wrangler, including modules, jsx, typescript, etc. If you need anything more, use a custom build.
- ⚠️ DEPRECATION: \\"webpack_config\\":
- DEPRECATION: \\"webpack_config\\":
DO NOT USE THIS. Most common features now work out of the box with wrangler, including modules, jsx, typescript, etc. If you need anything more, use a custom build."
`);
});
Expand Down Expand Up @@ -759,15 +759,15 @@ describe("normalizeAndValidateConfig()", () => {
expect(normalizePath(diagnostics.renderWarnings()))
.toMatchInlineSnapshot(`
"Processing project/wrangler.toml configuration:
- ⚠️ DEPRECATION: \\"build.upload.format\\":
- DEPRECATION: \\"build.upload.format\\":
The format is inferred automatically from the code.
- ⚠️ DEPRECATION: \\"build.upload.main\\":
- DEPRECATION: \\"build.upload.main\\":
Delete the \`build.upload.main\` and \`build.upload.dir\` fields.
Then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"src/index.ts\\"
\`\`\`
- ⚠️ DEPRECATION: \\"build.upload.dir\\":
- DEPRECATION: \\"build.upload.dir\\":
Use the top level \\"main\\" field or a command-line argument to specify the entry-point for the Worker.
- DEPRECATION: The \`build.upload.rules\` config field is no longer used, the rules should be specified via the \`rules\` config field. Delete the \`build.upload\` field from the configuration file, and add this:
\`\`\`
Expand Down Expand Up @@ -1509,9 +1509,9 @@ describe("normalizeAndValidateConfig()", () => {
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- ⚠️ DEPRECATION: \\"zone_id\\":
- DEPRECATION: \\"zone_id\\":
This is unnecessary since we can deduce this from routes directly.
- ⚠️ DEPRECATION: \\"experimental_services\\":
- DEPRECATION: \\"experimental_services\\":
The \\"experimental_services\\" field is no longer supported. Instead, use [[unsafe.bindings]] to enable experimental features. Add this to your wrangler.toml:
\`\`\`
[[unsafe.bindings]]
Expand Down Expand Up @@ -2733,9 +2733,9 @@ describe("normalizeAndValidateConfig()", () => {
"Processing wrangler configuration:
- \\"env.ENV1\\" environment configuration
- ⚠️ DEPRECATION: \\"zone_id\\":
- DEPRECATION: \\"zone_id\\":
This is unnecessary since we can deduce this from routes directly.
- ⚠️ DEPRECATION: \\"experimental_services\\":
- DEPRECATION: \\"experimental_services\\":
The \\"experimental_services\\" field is no longer supported. Instead, use [[unsafe.bindings]] to enable experimental features. Add this to your wrangler.toml:
\`\`\`
[[unsafe.bindings]]
Expand Down
59 changes: 30 additions & 29 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ describe("wrangler dev", () => {
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn.replaceAll(currentDate, "<current-date>"))
.toMatchInlineSnapshot(`
"No compatibility_date was specified. Using today's date: <current-date>.
Add one to your wrangler.toml file:
\`\`\`
compatibility_date = \\"<current-date>\\"
\`\`\`
or pass it in your terminal:
\`\`\`
--compatibility-date=<current-date>
\`\`\`
See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
"No compatibility_date was specified. Using today's date: <current-date>.
Add one to your wrangler.toml file:
\`\`\`
compatibility_date = \\"<current-date>\\"
\`\`\`
or pass it in your terminal:
\`\`\`
--compatibility-date=<current-date>
\`\`\`
See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});
Expand Down Expand Up @@ -79,9 +79,9 @@ describe("wrangler dev", () => {

expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`
"Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`main\` config field.
[32m%s[0m If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
"Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`main\` config field.
✖ [32mIf you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new.[0m"
`);
});

Expand Down Expand Up @@ -389,7 +389,7 @@ describe("wrangler dev", () => {
watch_dir: "src",
});
expect(std.out).toMatchInlineSnapshot(
`"running: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""`
`"Running custom build: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""`
);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
Expand All @@ -411,7 +411,7 @@ describe("wrangler dev", () => {
`);

expect(std.out).toMatchInlineSnapshot(
`"running: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"`
`"Running custom build: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"`
);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
Expand All @@ -431,13 +431,14 @@ describe("wrangler dev", () => {
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""`
);
expect(std.out).toMatchInlineSnapshot(
`"running: node -e \\"console.log('custom build');\\""`
);
expect(std.out).toMatchInlineSnapshot(`
"Running custom build: node -e \\"console.log('custom build');\\"
"
`);
expect(std.err).toMatchInlineSnapshot(`
"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"
[32m%s[0m If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"
✖ [32mIf you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new.[0m"
`);
expect(std.warn).toMatchInlineSnapshot(`""`);
});
Expand Down Expand Up @@ -469,9 +470,9 @@ describe("wrangler dev", () => {
);
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`
"Setting upstream-protocol to http is not currently implemented.
If this is required in your project, please add your use case to the following issue:
https://github.com/cloudflare/wrangler2/issues/583."
"Setting upstream-protocol to http is not currently implemented.
If this is required in your project, please add your use case to the following issue:
https://github.com/cloudflare/wrangler2/issues/583."
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});
Expand Down Expand Up @@ -595,11 +596,11 @@ describe("wrangler dev", () => {
expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("localhost");
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`
"WARNING: You have Durable Object bindings, which are not defined locally in the worker being developed.
Be aware that changes to the data stored in these Durable Objects will be permanent and affect the live instances.
Remote Durable Objects that are affected:
- {\\"name\\":\\"NAME_2\\",\\"class_name\\":\\"CLASS_2\\",\\"script_name\\":\\"SCRIPT_A\\"}
- {\\"name\\":\\"NAME_4\\",\\"class_name\\":\\"CLASS_4\\",\\"script_name\\":\\"SCRIPT_B\\"}"
"WARNING: You have Durable Object bindings that are not defined locally in the worker being developed.
Be aware that changes to the data stored in these Durable Objects will be permanent and affect the live instances.
Remote Durable Objects that are affected:
- {\\"name\\":\\"NAME_2\\",\\"class_name\\":\\"CLASS_2\\",\\"script_name\\":\\"SCRIPT_A\\"}
- {\\"name\\":\\"NAME_4\\",\\"class_name\\":\\"CLASS_4\\",\\"script_name\\":\\"SCRIPT_B\\"}"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe("guess worker format", () => {
);
expect(guess).toBe("service-worker");
expect(std.warn).toMatchInlineSnapshot(
`"The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using \\"service-worker\\" format..."`
`"The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using \\"service-worker\\" format..."`
);
});
});
8 changes: 7 additions & 1 deletion packages/wrangler/src/__tests__/helpers/mock-console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import * as util from "node:util";
* assert on the values they're called with in our tests.
*/

let logSpy: jest.SpyInstance,
let debugSpy: jest.SpyInstance,
logSpy: jest.SpyInstance,
errorSpy: jest.SpyInstance,
warnSpy: jest.SpyInstance;

const std = {
get debug() {
return normalizeOutput(debugSpy);
},
get out() {
return normalizeOutput(logSpy);
},
Expand All @@ -35,11 +39,13 @@ function captureCalls(spy: jest.SpyInstance): string {

export function mockConsoleMethods() {
beforeEach(() => {
debugSpy = jest.spyOn(console, "debug").mockImplementation();
logSpy = jest.spyOn(console, "log").mockImplementation();
errorSpy = jest.spyOn(console, "error").mockImplementation();
warnSpy = jest.spyOn(console, "warn").mockImplementation();
});
afterEach(() => {
debugSpy.mockRestore();
logSpy.mockRestore();
errorSpy.mockRestore();
warnSpy.mockRestore();
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/__tests__/https-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ describe("getHttpsOptions()", () => {
`"Generating new self-signed certificate..."`
);
expect(std.warn).toMatchInlineSnapshot(`
"Unable to cache generated self-signed certificate in home/.wrangler/local-cert.
ERROR: Cannot write file"
"Unable to cache generated self-signed certificate in home/.wrangler/local-cert.
ERROR: Cannot write file"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});
Expand Down
13 changes: 6 additions & 7 deletions packages/wrangler/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ describe("wrangler", () => {
-h, --help Show help [boolean]
-v, --version Show version number [boolean]
--legacy-env Use legacy environments [boolean]
Unknown argument: invalid-command"
✖ Unknown argument: invalid-command"
`);
});
});
Expand Down Expand Up @@ -845,22 +844,22 @@ describe("wrangler", () => {
it("should throw an error if the deprecated command is used with positional arguments", async () => {
await expect(runWrangler("preview GET")).rejects
.toThrowErrorMatchingInlineSnapshot(`
"⚠️ DEPRECATION:
"DEPRECATION:
The \`wrangler preview\` command has been deprecated.
Try using \`wrangler dev\` to to try out a worker during development.
"
`);
await expect(runWrangler(`preview GET "SomeBody"`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
"⚠️ DEPRECATION:
"DEPRECATION:
The \`wrangler preview\` command has been deprecated.
Try using \`wrangler dev\` to to try out a worker during development.
"
`);
});
});

describe("subcommand implicit help ran on imcomplete command execution", () => {
describe("subcommand implicit help ran on incomplete command execution", () => {
function endEventLoop() {
return new Promise((resolve) => setImmediate(resolve));
}
Expand Down Expand Up @@ -970,15 +969,15 @@ describe("wrangler", () => {
it("should print a deprecation message for 'generate'", async () => {
await runWrangler("generate").catch((err) => {
expect(err.message).toMatchInlineSnapshot(`
"⚠️ DEPRECATION:
"DEPRECATION:
\`wrangler generate\` has been deprecated, please refer to https://github.com/cloudflare/wrangler2/blob/main/docs/deprecations.md#generate for alternatives"
`);
});
});
it("should print a deprecation message for 'build'", async () => {
await runWrangler("build").catch((err) => {
expect(err.message).toMatchInlineSnapshot(`
"⚠️ DEPRECATION:
"DEPRECATION:
\`wrangler build\` has been deprecated, please refer to https://github.com/cloudflare/wrangler2/blob/main/docs/deprecations.md#build for alternatives"
`);
});
Expand Down
9 changes: 9 additions & 0 deletions packages/wrangler/src/__tests__/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,17 @@ import {
} from "../cfetch/internal";
import { confirm, prompt } from "../dialogs";
import { mockFetchInternal, mockFetchKVGetValue } from "./helpers/mock-cfetch";
import { useMockIsTTY } from "./helpers/mock-istty";
import { MockWebSocket } from "./helpers/mock-web-socket";

// For tests let's just agree to pretend we are not running in a TTY environment.
// This ensures consistency between running tests:
// - locally with `run test-watch`,
// - locally (in parallel) using `run test`,
// - and also in the CI actions.
// eslint-disable-next-line react-hooks/rules-of-hooks
useMockIsTTY().setIsTTY(false);

jest.mock("ws", () => {
return {
__esModule: true,
Expand Down
Loading

0 comments on commit fbad3f3

Please sign in to comment.