Skip to content

Commit

Permalink
Remove unnecessary stdout from test output (#5136)
Browse files Browse the repository at this point in the history
* Remove unnecessary stdout from test output

* test: make sure mocks are cleared between vitest tests and use MockAgent rather than mocking undici
  • Loading branch information
petebacondarwin authored Mar 5, 2024
1 parent f8808b2 commit 93e83b1
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 76 deletions.
6 changes: 5 additions & 1 deletion fixtures/dev-env/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ beforeEach(() => {
mf = undefined;
res = undefined as any;
ws = undefined;
// Hide stdout messages from the test logs
vi.spyOn(console, "log").mockImplementation(() => {});
});
afterEach(async () => {
await Promise.allSettled(fireAndForgetPromises);
Expand Down Expand Up @@ -330,7 +332,9 @@ describe("startDevWorker: ProxyController", () => {
});

test("User worker exception", async () => {
const consoleErrorSpy = vi.spyOn(console, "error");
const consoleErrorSpy = vi
.spyOn(console, "error")
.mockImplementation(() => {});

const run = await fakeStartUserWorker({
script: `
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
Fetcher,
R2Bucket,
} from "@cloudflare/workers-types";
import { describe, expect, it } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { unstable_dev } from "wrangler";
import { getBindingsProxy } from "./shared";
import type { KVNamespace } from "@cloudflare/workers-types";
Expand All @@ -31,6 +31,11 @@ const wranglerTomlFilePath = path.join(__dirname, "..", "wrangler.toml");
describe("getBindingsProxy - bindings", () => {
let devWorkers: UnstableDevWorker[];

beforeEach(() => {
// Hide stdout messages from the test logs
vi.spyOn(console, "log").mockImplementation(() => {});
});

// Note: we're skipping the service workers and durable object tests
// so there's no need to start separate workers right now, the
// following beforeAll and afterAll should be un-commented when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
Fetcher,
R2Bucket,
} from "@cloudflare/workers-types";
import { describe, expect, it } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { unstable_dev } from "wrangler";
import { getPlatformProxy } from "./shared";
import type { KVNamespace } from "@cloudflare/workers-types";
Expand All @@ -31,6 +31,11 @@ const wranglerTomlFilePath = path.join(__dirname, "..", "wrangler.toml");
describe("getPlatformProxy - bindings", () => {
let devWorkers: UnstableDevWorker[];

beforeEach(() => {
// Hide stdout messages from the test logs
vi.spyOn(console, "log").mockImplementation(() => {});
});

// Note: we're skipping the service workers and durable object tests
// so there's no need to start separate workers right now, the
// following beforeAll and afterAll should be un-commented when
Expand Down
7 changes: 6 additions & 1 deletion fixtures/no-bundle-import/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ describe("Worker", () => {
let worker: UnstableDevWorker;

beforeAll(async () => {
worker = await unstable_dev(path.resolve(__dirname, "index.js"));
worker = await unstable_dev(path.resolve(__dirname, "index.js"), {
logLevel: "none",
experimental: {
disableExperimentalWarning: true,
},
});
}, 30_000);

afterAll(() => worker.stop());
Expand Down
2 changes: 0 additions & 2 deletions fixtures/worker-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ describe("'wrangler dev' correctly renders pages", () => {
headers: { Origin: `http://${ip}:${port}` },
});
const text = await response.text();
console.log(text);
expect(text).toContain(`HOST:prod.example.org`);
expect(text).toContain(`ORIGIN:https://prod.example.org`);
});
Expand All @@ -120,7 +119,6 @@ describe("'wrangler dev' correctly renders pages", () => {
headers: { Origin: `http://foo.com` },
});
const text = await response.text();
console.log(text);
expect(text).toContain(`HOST:prod.example.org`);
expect(text).toContain(`ORIGIN:http://foo.com`);
});
Expand Down
26 changes: 12 additions & 14 deletions packages/create-cloudflare/src/__tests__/workers.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { existsSync, readdirSync } from "fs";
import { getWorkerdCompatibilityDate } from "helpers/command";
import { readFile, writeFile } from "helpers/files";
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import { beforeEach, describe, expect, test, vi } from "vitest";
import {
addWorkersTypesToTsConfig,
getLatestTypesEntrypoint,
Expand Down Expand Up @@ -43,10 +43,6 @@ const mockWorkersTypesDirectory = (
describe("getLatestTypesEntrypoint", () => {
const ctx = createTestContext();

afterEach(() => {
vi.clearAllMocks();
});

test("happy path", async () => {
mockWorkersTypesDirectory();

Expand Down Expand Up @@ -94,10 +90,6 @@ describe("addWorkersTypesToTsConfig", () => {
);
});

afterEach(() => {
vi.clearAllMocks();
});

test("happy path", async () => {
await addWorkersTypesToTsConfig(ctx);

Expand Down Expand Up @@ -140,12 +132,18 @@ describe("updateWranglerToml", () => {
const ctx = createTestContext();

const mockCompatDate = "2024-01-17";
vi.mocked(getWorkerdCompatibilityDate).mockReturnValue(
Promise.resolve(mockCompatDate)
);

afterEach(() => {
vi.clearAllMocks();
beforeEach(() => {
vi.mocked(getWorkerdCompatibilityDate).mockReturnValue(
Promise.resolve(mockCompatDate)
);
vi.mocked(existsSync).mockImplementation(() => true);
mockWorkersTypesDirectory();

// Mock the read of tsconfig.json
vi.mocked(readFile).mockImplementation(
() => `{ "compilerOptions": { "types": ["@cloudflare/workers-types"]} }`
);
});

test("placeholder replacement", async () => {
Expand Down
116 changes: 61 additions & 55 deletions packages/create-cloudflare/src/helpers/__tests__/command.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync } from "fs";
import { spawn } from "cross-spawn";
import { detectPackageManager } from "helpers/packages";
import { fetch, Response } from "undici";
import { getGlobalDispatcher, MockAgent, setGlobalDispatcher } from "undici";
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import whichPMRuns from "which-pm-runs";
import { createTestContext } from "../../__tests__/helpers";
Expand All @@ -11,67 +12,49 @@ import {
npmInstall,
runCommand,
} from "../command";
import type { ChildProcess } from "child_process";

// We can change how the mock spawn works by setting these variables
let spawnResultCode = 0;
let spawnStdout: string | undefined = undefined;
let spawnStderr: string | undefined = undefined;
let fetchResult: Response | undefined = undefined;

vi.mock("cross-spawn");
vi.mock("fs");
vi.mock("which-pm-runs");

describe("Command Helpers", () => {
afterEach(() => {
vi.clearAllMocks();
spawnResultCode = 0;
spawnStdout = undefined;
spawnStderr = undefined;
fetchResult = undefined;
});

beforeEach(() => {
// Mock out the child_process.spawn function
vi.mock("cross-spawn", () => {
const mockedSpawn = vi.fn().mockImplementation(() => {
return {
on: vi.fn().mockImplementation((event, cb) => {
if (event === "close") {
cb(spawnResultCode);
}
}),
stdout: {
on(event: "data", cb: (data: string) => void) {
spawnStdout !== undefined && cb(spawnStdout);
},
vi.mocked(spawn).mockImplementation(() => {
return {
on: vi.fn().mockImplementation((event, cb) => {
if (event === "close") {
cb(spawnResultCode);
}
}),
stdout: {
on(_event: "data", cb: (data: string) => void) {
spawnStdout !== undefined && cb(spawnStdout);
},
stderr: {
on(event: "data", cb: (data: string) => void) {
spawnStderr !== undefined && cb(spawnStderr);
},
},
stderr: {
on(_event: "data", cb: (data: string) => void) {
spawnStderr !== undefined && cb(spawnStderr);
},
};
});

return { spawn: mockedSpawn };
},
} as unknown as ChildProcess;
});

// Mock out the undici fetch function
vi.mock("undici", async () => {
const actual = (await vi.importActual("undici")) as Record<
string,
unknown
>;
const mockedFetch = vi.fn().mockImplementation(() => {
return fetchResult;
});

return { ...actual, fetch: mockedFetch };
});

vi.mock("which-pm-runs");
vi.mocked(whichPMRuns).mockReturnValue({ name: "npm", version: "8.3.1" });

vi.mock("fs", () => ({
existsSync: vi.fn(() => false),
}));
vi.mocked(existsSync).mockImplementation(() => false);
});

const expectSilentSpawnWith = (cmd: string) => {
Expand Down Expand Up @@ -177,32 +160,55 @@ describe("Command Helpers", () => {
});

describe("getWorkerdCompatibilityDate()", () => {
const originalDispatcher = getGlobalDispatcher();
let agent: MockAgent;

beforeEach(() => {
// Mock out the undici Agent
agent = new MockAgent();
agent.disableNetConnect();
setGlobalDispatcher(agent);
});

afterEach(() => {
agent.assertNoPendingInterceptors();
setGlobalDispatcher(originalDispatcher);
});

test("normal flow", async () => {
fetchResult = new Response(
JSON.stringify({
"dist-tags": { latest: "2.20250110.5" },
})
);
agent
.get("https://registry.npmjs.org")
.intercept({ path: "/workerd" })
.reply(
200,
JSON.stringify({
"dist-tags": { latest: "2.20250110.5" },
})
);
const date = await getWorkerdCompatibilityDate();
expect(fetch).toHaveBeenCalledWith("https://registry.npmjs.org/workerd");
expect(date).toBe("2025-01-10");
});

test("empty result", async () => {
fetchResult = new Response(
JSON.stringify({
"dist-tags": { latest: "" },
})
);
agent
.get("https://registry.npmjs.org")
.intercept({ path: "/workerd" })
.reply(
200,
JSON.stringify({
"dist-tags": { latest: "" },
})
);
const date = await getWorkerdCompatibilityDate();
expect(fetch).toHaveBeenCalledWith("https://registry.npmjs.org/workerd");
expect(date).toBe("2023-05-18");
});

test("command failed", async () => {
fetchResult = new Response("Unknown error");
agent
.get("https://registry.npmjs.org")
.intercept({ path: "/workerd" })
.replyWithError(new Error("Unknown error"));
const date = await getWorkerdCompatibilityDate();
expect(fetch).toHaveBeenCalledWith("https://registry.npmjs.org/workerd");
expect(date).toBe("2023-05-18");
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { randomBytes } from "crypto";
import fs from "fs/promises";
import os from "os";
import path from "path";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import { unstable_dev } from "wrangler";
import type { UnstableDevWorker } from "wrangler";

Expand All @@ -24,6 +24,7 @@ describe("Preview Worker", () => {
experimental: {
disableExperimentalWarning: true,
},
logLevel: "none",
});

tmpDir = await fs.realpath(
Expand Down Expand Up @@ -108,6 +109,7 @@ compatibility_date = "2023-01-01"
);
});
it("should reject invalid exchange_url", async () => {
vi.spyOn(console, "error").mockImplementation(() => {});
const resp = await worker.fetch(
`https://preview.devprod.cloudflare.dev/exchange?exchange_url=not_an_exchange_url`,
{ method: "POST" }
Expand Down Expand Up @@ -190,6 +192,7 @@ compatibility_date = "2023-01-01"
.split("=")[1];
});
it("should reject invalid prewarm url", async () => {
vi.spyOn(console, "error").mockImplementation(() => {});
const resp = await worker.fetch(
`https://random-data.preview.devprod.cloudflare.dev/.update-preview-token?token=TEST_TOKEN&prewarm=not_a_prewarm_url&remote=${encodeURIComponent(
`http://127.0.0.1:${remote.port}`
Expand All @@ -201,6 +204,7 @@ compatibility_date = "2023-01-01"
);
});
it("should reject invalid remote url", async () => {
vi.spyOn(console, "error").mockImplementation(() => {});
const resp = await worker.fetch(
`https://random-data.preview.devprod.cloudflare.dev/.update-preview-token?token=TEST_TOKEN&prewarm=${encodeURIComponent(
`http://127.0.0.1:${remote.port}/prewarm`
Expand Down
1 change: 1 addition & 0 deletions vitest.shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ export default defineConfig({
poolOptions: {
useAtomics: true,
},
restoreMocks: true,
},
});

0 comments on commit 93e83b1

Please sign in to comment.