From 93e83b156a41b3f2ea038efec14683317a8b12b3 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 5 Mar 2024 10:15:11 +0000 Subject: [PATCH] Remove unnecessary stdout from test output (#5136) * Remove unnecessary stdout from test output * test: make sure mocks are cleared between vitest tests and use MockAgent rather than mocking undici --- fixtures/dev-env/tests/index.test.ts | 6 +- .../tests/get-bindings-proxy.bindings.test.ts | 7 +- .../tests/get-platform-proxy.env.test.ts | 7 +- fixtures/no-bundle-import/src/index.test.ts | 7 +- fixtures/worker-app/tests/index.test.ts | 2 - .../src/__tests__/workers.test.ts | 26 ++-- .../src/helpers/__tests__/command.test.ts | 116 +++++++++--------- .../tests/index.test.ts | 6 +- vitest.shared.ts | 1 + 9 files changed, 102 insertions(+), 76 deletions(-) diff --git a/fixtures/dev-env/tests/index.test.ts b/fixtures/dev-env/tests/index.test.ts index aa2747da0aa5..7745b1ddf49e 100644 --- a/fixtures/dev-env/tests/index.test.ts +++ b/fixtures/dev-env/tests/index.test.ts @@ -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); @@ -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: ` diff --git a/fixtures/get-bindings-proxy/tests/get-bindings-proxy.bindings.test.ts b/fixtures/get-bindings-proxy/tests/get-bindings-proxy.bindings.test.ts index 3a354cc66c59..6cb9169c9ff8 100644 --- a/fixtures/get-bindings-proxy/tests/get-bindings-proxy.bindings.test.ts +++ b/fixtures/get-bindings-proxy/tests/get-bindings-proxy.bindings.test.ts @@ -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"; @@ -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 diff --git a/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts b/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts index 1f86b6191abe..2da8df13bf82 100644 --- a/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts +++ b/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts @@ -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"; @@ -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 diff --git a/fixtures/no-bundle-import/src/index.test.ts b/fixtures/no-bundle-import/src/index.test.ts index 5c795513cbb7..75e0e8d248dd 100644 --- a/fixtures/no-bundle-import/src/index.test.ts +++ b/fixtures/no-bundle-import/src/index.test.ts @@ -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()); diff --git a/fixtures/worker-app/tests/index.test.ts b/fixtures/worker-app/tests/index.test.ts index 664376125767..27712af13d0c 100644 --- a/fixtures/worker-app/tests/index.test.ts +++ b/fixtures/worker-app/tests/index.test.ts @@ -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`); }); @@ -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`); }); diff --git a/packages/create-cloudflare/src/__tests__/workers.test.ts b/packages/create-cloudflare/src/__tests__/workers.test.ts index 48af88fe038e..17048c45ef56 100644 --- a/packages/create-cloudflare/src/__tests__/workers.test.ts +++ b/packages/create-cloudflare/src/__tests__/workers.test.ts @@ -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, @@ -43,10 +43,6 @@ const mockWorkersTypesDirectory = ( describe("getLatestTypesEntrypoint", () => { const ctx = createTestContext(); - afterEach(() => { - vi.clearAllMocks(); - }); - test("happy path", async () => { mockWorkersTypesDirectory(); @@ -94,10 +90,6 @@ describe("addWorkersTypesToTsConfig", () => { ); }); - afterEach(() => { - vi.clearAllMocks(); - }); - test("happy path", async () => { await addWorkersTypesToTsConfig(ctx); @@ -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 () => { diff --git a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts index a60a20ab5bd9..71deb6921a82 100644 --- a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts +++ b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts @@ -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"; @@ -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) => { @@ -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"); }); }); diff --git a/packages/edge-preview-authenticated-proxy/tests/index.test.ts b/packages/edge-preview-authenticated-proxy/tests/index.test.ts index ccc63d4c73f6..f126619729ad 100644 --- a/packages/edge-preview-authenticated-proxy/tests/index.test.ts +++ b/packages/edge-preview-authenticated-proxy/tests/index.test.ts @@ -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"; @@ -24,6 +24,7 @@ describe("Preview Worker", () => { experimental: { disableExperimentalWarning: true, }, + logLevel: "none", }); tmpDir = await fs.realpath( @@ -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" } @@ -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}` @@ -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` diff --git a/vitest.shared.ts b/vitest.shared.ts index 78af35d607a6..4b1a037c6840 100644 --- a/vitest.shared.ts +++ b/vitest.shared.ts @@ -16,5 +16,6 @@ export default defineConfig({ poolOptions: { useAtomics: true, }, + restoreMocks: true, }, });