Skip to content
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

feat: support outputing ND-JSON files via an environment variable #6383

Merged
merged 7 commits into from
Aug 7, 2024
5 changes: 5 additions & 0 deletions .changeset/strong-rats-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": minor
---

feat: support outputting ND-JSON files via an environment variable
245 changes: 245 additions & 0 deletions packages/wrangler/src/__tests__/output.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
import { readdirSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { clearOutputFilePath, writeOutput } from "../output";
import { runInTempDir } from "./helpers/run-in-tmp";
import type { OutputEntry } from "../output";

const originalProcessEnv = process.env;
const {
WRANGLER_OUTPUT_FILE_DIRECTORY: _,
WRANGLER_OUTPUT_FILE_PATH: __,
...processEnvNoVars
} = originalProcessEnv;

describe("writeOutput()", () => {
runInTempDir({ homedir: "home" });
afterEach(clearOutputFilePath);

it("should do nothing with no env vars set", () => {
try {
process.env = processEnvNoVars;
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});
// No files written
expect(readdirSync(".")).toEqual(["home"]);
} finally {
process.env = originalProcessEnv;
}
});

it("should write to the file given by WRANGLER_OUTPUT_FILE_PATH", () => {
try {
const WRANGLER_OUTPUT_FILE_PATH = "output.json";
process.env = { ...processEnvNoVars, WRANGLER_OUTPUT_FILE_PATH };
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});
const outputFile = readFileSync(WRANGLER_OUTPUT_FILE_PATH, "utf8");
expect(outputFile).toContainEntries([
{
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
},
]);
} finally {
process.env = originalProcessEnv;
}
});

it("should write to the file given by WRANGLER_OUTPUT_FILE_PATH, ignoring WRANGLER_OUTPUT_FILE_DIRECTORY", () => {
try {
const WRANGLER_OUTPUT_FILE_PATH = "output.json";
process.env = {
...processEnvNoVars,
WRANGLER_OUTPUT_FILE_PATH,
WRANGLER_OUTPUT_FILE_DIRECTORY: ".",
};
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});
const outputFile = readFileSync(WRANGLER_OUTPUT_FILE_PATH, "utf8");
expect(outputFile).toContainEntries([
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this custom matcher necessary? To understand what it's doing, you'd have to go looking for the definition whereas if it was a plain function call (expect(parseEntries(outputFile)).toEqual([...) you could just cmd+click (parseEntries) to get to the implementation

Copy link
Contributor

Choose a reason for hiding this comment

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

I think custom matchers are great for common use-cases to a whole codebase and should be introduced globally so they can be defined clearly, learnt and relied upon. Implementing them as one-offs per suite I think is obfuscating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just a helper function to avoid duplicating the same logic throughout and keeping the tests themselves nice and readable.

It is only used in this file (where it is defined) and it makes no sense outside of this file. It is nicer than using a vanilla function because Vitest treats it specially, for example it will format the errors nicely and do diffing of the actual and expected results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you Cmd-Click on the function it takes you to the type definition below, which is exactly next to the implementation. So I don't think it is too hard to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhat ironically, when you create global custom matchers like this it is common to put the type in a .d.ts file - e.g. https://github.com/cloudflare/workers-sdk/blob/f6300a71adb83e5f92cd9c9ddd56a85132e03f98/packages/create-cloudflare/vitest.d.ts

In this case it is actually much harder to find the original implementation of the matcher. For example the toExist() matcher implementation is over here: https://github.com/cloudflare/workers-sdk/blob/cc335ee1c1dbd58009714c4b49e23e9dcf121d1e/packages/create-cloudflare/e2e-tests/setup.ts

{
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
},
]);
} finally {
process.env = originalProcessEnv;
}
});

it("should write multiple entries to the file given by WRANGLER_OUTPUT_FILE_PATH", () => {
try {
const WRANGLER_OUTPUT_FILE_PATH = "output.json";
process.env = { ...processEnvNoVars, WRANGLER_OUTPUT_FILE_PATH };
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});
writeOutput({
type: "deployment",
version: 1,
worker_name: "Worker",
worker_tag: "ABCDE12345",
deployment_id: "1234",
});

const outputFile = readFileSync(WRANGLER_OUTPUT_FILE_PATH, "utf8");
expect(outputFile).toContainEntries([
{
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
},
{
type: "deployment",
version: 1,
worker_name: "Worker",
worker_tag: "ABCDE12345",
deployment_id: "1234",
},
]);
} finally {
process.env = originalProcessEnv;
}
});

it("should write to a random file in WRANGLER_OUTPUT_FILE_DIRECTORY", () => {
try {
process.env = {
...processEnvNoVars,
WRANGLER_OUTPUT_FILE_DIRECTORY: "output",
};
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});

const outputFilePaths = readdirSync("output");
expect(outputFilePaths.length).toEqual(1);
expect(outputFilePaths[0]).toMatch(/wrangler-output-.+\.json/);
const outputFile = readFileSync(
join("output", outputFilePaths[0]),
"utf8"
);
expect(outputFile).toContainEntries([
{
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
},
]);
} finally {
process.env = originalProcessEnv;
}
});

it("should write multiple entries to the same random file in WRANGLER_OUTPUT_FILE_DIRECTORY", () => {
try {
process.env = {
...processEnvNoVars,
WRANGLER_OUTPUT_FILE_DIRECTORY: "output",
};
writeOutput({
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
});
writeOutput({
type: "deployment",
version: 1,
worker_name: "Worker",
worker_tag: "ABCDE12345",
deployment_id: "1234",
});

const outputFilePaths = readdirSync("output");
expect(outputFilePaths.length).toEqual(1);
expect(outputFilePaths[0]).toMatch(/wrangler-output-.+\.json/);
const outputFile = readFileSync(
join("output", outputFilePaths[0]),
"utf8"
);
expect(outputFile).toContainEntries([
{
type: "wrangler-session",
version: 1,
wrangler_version: "0.0.0.0",
command_line_args: ["--help"],
log_file_path: "some/log/path.log",
},
{
type: "deployment",
version: 1,
worker_name: "Worker",
worker_tag: "ABCDE12345",
deployment_id: "1234",
},
]);
} finally {
process.env = originalProcessEnv;
}
});
});

expect.extend({
toContainEntries(received: string, expected: OutputEntry[]) {
const actual = received
.trim()
.split("\n")
.map((line) => JSON.parse(line));

const stamped = expected.map((entry) => ({
...entry,
timestamp: expect.any(String),
}));

return {
pass: this.equals(actual, stamped),
message: () => `Entries are${this.isNot ? "" : " not"} as expected.`,
actual,
expected: stamped,
};
},
});

interface CustomMatchers {
toContainEntries: (expected: OutputEntry[]) => unknown;
}

declare module "vitest" {
interface Assertion extends CustomMatchers {}
interface AsymmetricMatchersContaining extends CustomMatchers {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ describe("versions deploy", () => {
mswListVersions,
mswGetVersion(),
mswPostNewDeployment,
mswPatchNonVersionedScriptSettings
mswPatchNonVersionedScriptSettings,
...mswSuccessDeploymentScriptMetadata
);
});

Expand Down
45 changes: 26 additions & 19 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,35 +306,44 @@ Update them to point to this script instead?`;
return domains.map((domain) => renderRoute(domain));
}

export default async function deploy(
props: Props
): Promise<{ sourceMapSize?: number }> {
export default async function deploy(props: Props): Promise<{
sourceMapSize?: number;
deploymentId: string | null;
workerTag: string | null;
}> {
// TODO: warn if git/hg has uncommitted changes
const { config, accountId, name } = props;
let workerTag: string | null = null;
let deploymentId: string | null = null;

if (!props.dispatchNamespace && accountId && name) {
try {
const serviceMetaData = await fetchResult(
`/accounts/${accountId}/workers/services/${name}`
);
const { default_environment } = serviceMetaData as {
const serviceMetaData = await fetchResult<{
default_environment: {
script: { last_deployed_from: "dash" | "wrangler" | "api" };
script: {
tag: string;
last_deployed_from: "dash" | "wrangler" | "api";
};
};
};
}>(`/accounts/${accountId}/workers/services/${name}`);
const {
default_environment: { script },
} = serviceMetaData;
workerTag = script.tag;

if (default_environment.script.last_deployed_from === "dash") {
if (script.last_deployed_from === "dash") {
logger.warn(
`You are about to publish a Workers Service that was last published via the Cloudflare Dashboard.\nEdits that have been made via the dashboard will be overridden by your local code and config.`
);
if (!(await confirm("Would you like to continue?"))) {
return {};
return { deploymentId, workerTag };
}
} else if (default_environment.script.last_deployed_from === "api") {
} else if (script.last_deployed_from === "api") {
logger.warn(
`You are about to publish a Workers Service that was last updated via the script API.\nEdits that have been made via the script API will be overridden by your local code and config.`
);
if (!(await confirm("Would you like to continue?"))) {
return {};
return { deploymentId, workerTag };
}
}
} catch (e) {
Expand Down Expand Up @@ -436,15 +445,13 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
? `/accounts/${accountId}/workers/services/${scriptName}/environments/${envName}`
: `/accounts/${accountId}/workers/scripts/${scriptName}`;

let deploymentId: string | null = null;

const { format } = props.entry;

if (!props.dispatchNamespace && prod && accountId && scriptName) {
const yes = await confirmLatestDeploymentOverwrite(accountId, scriptName);
if (!yes) {
cancel("Aborting deploy...");
return {};
return { deploymentId, workerTag };
}
}

Expand Down Expand Up @@ -828,7 +835,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

if (props.dryRun) {
logger.log(`--dry-run: exiting now.`);
return {};
return { deploymentId, workerTag };
}
assert(accountId, "Missing accountId");

Expand All @@ -839,7 +846,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
// Early exit for WfP since it doesn't need the below code
if (props.dispatchNamespace !== undefined) {
deployWfpUserWorker(props.dispatchNamespace, deploymentId);
return {};
return { deploymentId, workerTag };
}

// deploy triggers
Expand All @@ -850,7 +857,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

logVersionIdChange();

return { sourceMapSize };
return { sourceMapSize, deploymentId, workerTag };
}

function deployWfpUserWorker(
Expand Down
Loading
Loading