further e2e test stability improvements (#10731)

petebacondarwin · web-flow · commit 5f8155a4b47c · 2025-09-23T21:42:33.000+01:00
Includes fixing private packages check
diff --git a/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts b/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts
@@ -19,6 +19,16 @@ import {
 } from "./wrangler";
 import type { WranglerCommandOptions } from "./wrangler";
 
+// eslint-disable-next-line @typescript-eslint/consistent-type-imports
+export function importWrangler(): Promise<typeof import("../../src/cli")> {
+	return import(WRANGLER_IMPORT.href);
+}
+
+// eslint-disable-next-line @typescript-eslint/consistent-type-imports
+export function importMiniflare(): Promise<typeof import("miniflare")> {
+	return import(MINIFLARE_IMPORT.href);
+}
+
 /**
  * Use this class in your e2e tests to create a temp directory, seed it with files
  * and then run various Wrangler commands.
@@ -42,16 +52,6 @@ export class WranglerE2ETestHelper {
 		await removeFiles(this.tmpPath, files);
 	}
 
-	// eslint-disable-next-line @typescript-eslint/consistent-type-imports
-	importWrangler(): Promise<typeof import("../../src/cli")> {
-		return import(WRANGLER_IMPORT.href);
-	}
-
-	// eslint-disable-next-line @typescript-eslint/consistent-type-imports
-	importMiniflare(): Promise<typeof import("miniflare")> {
-		return import(MINIFLARE_IMPORT.href);
-	}
-
 	runLongLived(
 		wranglerCommand: string,
 		{
@@ -76,7 +76,6 @@ export class WranglerE2ETestHelper {
 		wranglerCommand: string,
 		{ cwd = this.tmpPath, ...options }: WranglerCommandOptions = {}
 	) {
-		console.log(`Running wrangler command: ${wranglerCommand}`);
 		return runWrangler(wranglerCommand, { cwd, ...options });
 	}
 
diff --git a/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts b/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts
@@ -2,13 +2,19 @@ import path from "node:path";
 import dedent from "ts-dedent";
 import { beforeEach, describe, expect, it, onTestFinished } from "vitest";
 import { CLOUDFLARE_ACCOUNT_ID } from "../helpers/account-id";
-import { WranglerE2ETestHelper } from "../helpers/e2e-wrangler-test";
+import {
+	importMiniflare,
+	importWrangler,
+	WranglerE2ETestHelper,
+} from "../helpers/e2e-wrangler-test";
 import { generateResourceName } from "../helpers/generate-resource-name";
-import type { startRemoteProxySession } from "../../src/api";
 import type { RawConfig } from "../../src/config";
 import type { RemoteProxyConnectionString, WorkerOptions } from "miniflare";
 import type { ExpectStatic } from "vitest";
 
+const { startRemoteProxySession } = await importWrangler();
+const { Miniflare } = await importMiniflare();
+
 type TestCase<T = void> = {
 	name: string;
 	scriptPath: string;
@@ -522,8 +528,6 @@ async function runTestCase<T>(
 	helper: WranglerE2ETestHelper,
 	{ disableRemoteBindings } = { disableRemoteBindings: false }
 ) {
-	const { startRemoteProxySession } = await helper.importWrangler();
-	const { Miniflare } = await helper.importMiniflare();
 	await helper.seed(path.resolve(__dirname, "./workers"));
 	// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 	const setupResult = (await testCase.setup?.(helper))!;
diff --git a/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts b/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts
@@ -2,15 +2,25 @@ import assert from "node:assert";
 import { resolve } from "node:path";
 import { beforeAll, describe, expect, test } from "vitest";
 import { CLOUDFLARE_ACCOUNT_ID } from "../helpers/account-id";
-import { WranglerE2ETestHelper } from "../helpers/e2e-wrangler-test";
+import {
+	importMiniflare,
+	importWrangler,
+	WranglerE2ETestHelper,
+} from "../helpers/e2e-wrangler-test";
 import { generateResourceName } from "../helpers/generate-resource-name";
 import type {
-	Miniflare,
 	MiniflareOptions,
+	Miniflare as MiniflareType,
 	RemoteProxyConnectionString,
 	Response,
 } from "miniflare";
 
+const { Miniflare } = await importMiniflare();
+const {
+	startRemoteProxySession: startRemoteProxySession,
+	maybeStartOrUpdateRemoteProxySession: maybeStartOrUpdateRemoteProxySession,
+} = await importWrangler();
+
 // Note: the tests in this file are simple ones that check basic functionalities of the remote bindings programmatic APIs
 //       various other aspects of these APIs (e.g. different bindings, reloading capabilities) are indirectly tested when
 //       generally testing remote bindings
@@ -21,14 +31,6 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)(
 		const remoteWorkerName = generateResourceName();
 		const helper = new WranglerE2ETestHelper();
 
-		const { Miniflare } = await helper.importMiniflare();
-
-		const {
-			startRemoteProxySession: startRemoteProxySession,
-			maybeStartOrUpdateRemoteProxySession:
-				maybeStartOrUpdateRemoteProxySession,
-		} = await helper.importWrangler();
-
 		beforeAll(async () => {
 			await helper.seed(resolve(__dirname, "./workers"));
 			const { cleanup } = await helper.worker({
@@ -254,7 +256,7 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)(
 	}
 );
 
-async function timedDispatchFetch(mf: Miniflare): Promise<Response | null> {
+async function timedDispatchFetch(mf: MiniflareType): Promise<Response | null> {
 	try {
 		return await mf.dispatchFetch("http://localhost/", {
 			signal: AbortSignal.timeout(5000),
diff --git a/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts b/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts
@@ -3,9 +3,14 @@ import { resolve } from "node:path";
 import { setTimeout } from "node:timers/promises";
 import { beforeAll, describe, expect, it } from "vitest";
 import { CLOUDFLARE_ACCOUNT_ID } from "../helpers/account-id";
-import { WranglerE2ETestHelper } from "../helpers/e2e-wrangler-test";
+import {
+	importWrangler,
+	WranglerE2ETestHelper,
+} from "../helpers/e2e-wrangler-test";
 import { generateResourceName } from "../helpers/generate-resource-name";
 
+const { unstable_startWorker: startWorker } = await importWrangler();
+
 describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("startWorker - remote bindings", () => {
 	const remoteWorkerName = generateResourceName();
 	const helper = new WranglerE2ETestHelper();
@@ -47,8 +52,7 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("startWorker - remote bindings", () => {
 					}),
 				});
 
-				const { unstable_startWorker } = await helper.importWrangler();
-				const worker = await unstable_startWorker({
+				const worker = await startWorker({
 					config: `${helper.tmpPath}/wrangler.json`,
 					dev: {
 						experimentalRemoteBindings,
@@ -82,9 +86,7 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("startWorker - remote bindings", () => {
 					}),
 				});
 
-				const { unstable_startWorker } = await helper.importWrangler();
-
-				const worker = await unstable_startWorker({
+				const worker = await startWorker({
 					config: `${helper.tmpPath}/wrangler.json`,
 					dev: {
 						experimentalRemoteBindings,
diff --git a/packages/wrangler/e2e/start-worker-auth-opts.test.ts b/packages/wrangler/e2e/start-worker-auth-opts.test.ts
@@ -1,27 +1,29 @@
 import assert from "node:assert";
 import path from "node:path";
 import dedent from "ts-dedent";
-import { beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import { CLOUDFLARE_ACCOUNT_ID } from "./helpers/account-id";
-import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test";
+import {
+	importWrangler,
+	WranglerE2ETestHelper,
+} from "./helpers/e2e-wrangler-test";
 import type { Worker } from "../src/api/startDevWorker";
 import type { MockInstance } from "vitest";
 
-type Wrangler = Awaited<ReturnType<WranglerE2ETestHelper["importWrangler"]>>;
-
-describe("startWorker - auth options", () => {
-	let consoleErrorMock: MockInstance<typeof console.error>;
-
-	beforeAll(() => {
-		consoleErrorMock = vi.spyOn(console, "error").mockImplementation(() => {});
-	});
+const { unstable_startWorker: startWorker } = await importWrangler();
 
+describe("startWorker - auth options", { sequential: true }, () => {
+	let worker: Worker | undefined;
 	describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("with remote bindings", () => {
 		let helper: WranglerE2ETestHelper;
-		let wrangler: Wrangler;
-		let startWorker: Wrangler["unstable_startWorker"];
+
+		let consoleErrorMock: MockInstance<typeof console.error>;
 
 		beforeEach(async () => {
+			consoleErrorMock = vi.spyOn(console, "error").mockImplementation(() => {
+				// suppress error output during tests - we are going to check for specific error messages in the tests themselves
+			});
+
 			helper = new WranglerE2ETestHelper();
 			const aiWorkerScript = dedent`
 			export default {
@@ -45,13 +47,11 @@ describe("startWorker - auth options", () => {
 			await helper.seed({
 				"src/index.js": aiWorkerScript,
 			});
-			wrangler = await helper.importWrangler();
-			startWorker = wrangler.unstable_startWorker;
 		});
 
-		test("starting a worker with startWorker with the valid auth information and updating it with invalid information", async (t) => {
-			t.onTestFinished(async () => await worker?.dispose());
+		afterEach(() => worker?.dispose());
 
+		test("starting a worker with startWorker with the valid auth information and updating it with invalid information", async () => {
 			const validAuth = vi.fn(() => {
 				assert(process.env.CLOUDFLARE_API_TOKEN);
 
@@ -63,7 +63,7 @@ describe("startWorker - auth options", () => {
 				};
 			});
 
-			const worker = await startWorker({
+			worker = await startWorker({
 				entrypoint: path.resolve(helper.tmpPath, "src/index.js"),
 				bindings: {
 					AI: {
@@ -80,7 +80,7 @@ describe("startWorker - auth options", () => {
 				},
 			});
 
-			await assertValidWorkerAiResponse(worker);
+			await assertValidWorkerAiResponse();
 
 			expect(validAuth).toHaveBeenCalledOnce();
 
@@ -101,14 +101,12 @@ describe("startWorker - auth options", () => {
 				},
 			});
 
-			await assertInvalidWorkerAiResponse(worker);
+			await assertInvalidWorkerAiResponse();
 
 			expect(incorrectAuth).toHaveBeenCalledOnce();
 		});
 
-		test("starting a worker with startWorker with invalid auth information and updating it with valid auth information", async (t) => {
-			t.onTestFinished(async () => await worker?.dispose());
-
+		test("starting a worker with startWorker with invalid auth information and updating it with valid auth information", async () => {
 			const incorrectAuth = vi.fn(() => {
 				return {
 					accountId: CLOUDFLARE_ACCOUNT_ID,
@@ -118,7 +116,7 @@ describe("startWorker - auth options", () => {
 				};
 			});
 
-			const worker = await startWorker({
+			worker = await startWorker({
 				entrypoint: path.resolve(helper.tmpPath, "src/index.js"),
 				bindings: {
 					AI: {
@@ -135,7 +133,7 @@ describe("startWorker - auth options", () => {
 				},
 			});
 
-			await assertInvalidWorkerAiResponse(worker);
+			await assertInvalidWorkerAiResponse();
 
 			expect(incorrectAuth).toHaveBeenCalledOnce();
 
@@ -158,12 +156,13 @@ describe("startWorker - auth options", () => {
 				},
 			});
 
-			await assertValidWorkerAiResponse(worker);
+			await assertValidWorkerAiResponse();
 
 			expect(validAuth).toHaveBeenCalledOnce();
 		});
 
-		async function assertValidWorkerAiResponse(worker: Worker) {
+		async function assertValidWorkerAiResponse() {
+			assert(worker, "Worker is not defined");
 			const responseText = await fetchTimedTextFromWorker(worker);
 
 			// We've fixed the auth information so now we can indeed get
@@ -179,7 +178,8 @@ describe("startWorker - auth options", () => {
 			);
 		}
 
-		async function assertInvalidWorkerAiResponse(worker: Worker) {
+		async function assertInvalidWorkerAiResponse() {
+			assert(worker, "Worker is not defined");
 			const responseText = await fetchTimedTextFromWorker(worker);
 
 			// The remote connection is not established so we can't successfully
@@ -196,12 +196,8 @@ describe("startWorker - auth options", () => {
 	});
 
 	describe("without remote bindings (no auth is needed)", () => {
-		test("starting a worker via startWorker without any remote bindings (doesn't cause wrangler to try to get the auth information)", async (t) => {
-			t.onTestFinished(async () => await worker?.dispose());
-
+		test("starting a worker via startWorker without any remote bindings (doesn't cause wrangler to try to get the auth information)", async () => {
 			const helper = new WranglerE2ETestHelper();
-			const wrangler = await helper.importWrangler();
-			const startWorker = wrangler.unstable_startWorker;
 
 			const simpleWorkerScript = dedent`
 			export default {
@@ -223,7 +219,7 @@ describe("startWorker - auth options", () => {
 				};
 			});
 
-			const worker = await startWorker({
+			worker = await startWorker({
 				entrypoint: path.resolve(helper.tmpPath, "src/index.js"),
 				dev: {
 					auth: someAuth,
@@ -255,6 +251,7 @@ async function fetchTimedTextFromWorker(
 	let responseText: string | null = null;
 
 	try {
+		assert(worker, "Worker is not defined");
 		await vi.waitFor(
 			async () => {
 				responseText = await (
diff --git a/packages/wrangler/e2e/startWorker.test.ts b/packages/wrangler/e2e/startWorker.test.ts
diff --git a/tools/deployments/validate-private-packages.ts b/tools/deployments/validate-private-packages.ts
