chore: improvements to remote-bindings tests to reduce flakes (#10380)

* chore: improvements to remote-bindings tests to reduce flakes

* retry wrangler-remote-resources checks a few times

* move the getPlatformProxy remote-bindings tests to the e2e CI job

* move `startDev` into its own module so it is possible to mock it

* move `startRemoteProxySession` into its own module so it is possible to mock it

* add missing types to `Binding` type

* add missing `remote` property and update jsdoc for Config type

* Allow config watching in unit tests

The tests were previously failing because the controller had not been been torn down before the end of the test, rather than the actual watching being a problem in itself.
When the tests end, the current working directory gets reverted from the temporary directory that was created for it. Since the controller had not been torn down, the config file change triggered a config change event which then tries to read the config from the wrong current working directory, causing an exception.

* Remove wrangler-remote-resources e2e test

This is now covered by a combination of:
- `packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts` - actually tests that we can create, deploy and connect to the remote dev session worker.
- `packages/wrangler/src/__tests__/dev/remote-bindings.test.ts` - tests that we generate the correct config for the remote session proxy worker and local miniflare session.

The removed e2e test was very flaky and slow as it required creating and deleting a lot of resources in Cloudflare.
The new test runs entirely without any dependency on Cloudflare and is super fast (<20 secs).

* add Wrangler remote bindings unit tests

* ensure configcontroller is torn down before tmp directory is deleted

* wait for config watching to be updated

* ignore changes if the watcher is closed

* Allow the "too large" upload test more time to pass

Sometimes timesout on Windows

* abort if this config controller is no longer viable

* remove unnecessary `console.dir()` calls in tests

* update ubuntu in CI tests

* run wrangler tests before fixtures

* Update packages/wrangler/e2e/helpers/e2e-wrangler-test.ts

Co-authored-by: Dario Piotrowicz <[email protected]>

* run all linux tests on ubuntu 24

---------

Co-authored-by: Dario Piotrowicz <[email protected]>

Author: petebacondarwin

.github/workflows/e2e.yml

@@ -25,7 +25,7 @@ jobs:
             description: v20, Windows
             node: 20.19.1
             # we need to use an amd image to run the containers tests, since we build for linux/amd64
-          - os: ubuntu-22.04
+          - os: ubuntu-24.04
             description: v20, Linux
             node: 20.19.1
 
@@ -61,6 +61,14 @@ jobs:
           TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
           CI_OS: ${{ matrix.os }}
 
+      - name: Run getPlatformProxy() remote-bindings e2e tests
+        run: pnpm run test:e2e -F @fixture/get-platform-proxy-remote-bindings
+        env:
+          TEST_CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
+          TEST_CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
+          NODE_OPTIONS: "--max_old_space_size=8192"
+          CI_OS: ${{ matrix.os }}
+
       - name: Upload turbo logs
         if: always()
         uses: actions/upload-artifact@v4
@@ -83,7 +91,8 @@ jobs:
           - os: windows-2022
             description: v20, Windows
             node: 20.19.1
-          - os: ubuntu-22.04-arm
+            # we need to use an amd image to run the containers tests, since we build for linux/amd64
+          - os: ubuntu-24.04
             description: v20, Linux
             node: 20.19.1
     runs-on: ${{ matrix.os }}
@@ -128,25 +137,3 @@ jobs:
         with:
           name: turbo-runs-${{ matrix.os }}-${{ matrix.node }}
           path: .turbo/runs
-
-  # E2E tests cannot run on pull requests created from forks due to security concerns.
-  # In such cases we require a member of the `workers-sdk` maintainers group to manually
-  # run these tests on behalf of the PR.
-  #
-  # If a PR from a fork specifically requests running the e2e tests, by applying the `e2e`
-  # label, we want CI to fail with a descriptive message, instead of skipping the `e2e-tests`
-  # job altogether, which gives the false optics that CI is green. Having CI intentionally
-  # fail for such use cases will act as a reminder that tests need to be manually run, and
-  # will prevent us from accidentally merging fork PRs that are green, but never ran the
-  # e2e tests.
-  e2e-test-forks:
-    name: "E2E tests on forks"
-    concurrency:
-      group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.node }}
-    if: github.event_name == 'pull_request' && contains(github.event.*.labels.*.name, 'e2e' ) && github.event.pull_request.head.repo.full_name != github.repository
-    runs-on: ${{ matrix.os }}
-    steps:
-      - name: Force Fail
-        run: |
-          echo: "E2E tests cannot run on pull requests created from forks due to security reasons. Please reach out to a workers-sdk maintainer to run the E2E tests on behalf of this PR."
-          exit 1

.github/workflows/test-and-check.yml

@@ -112,7 +112,7 @@ jobs:
             # Browser rendering is disabled here because of https://pptr.dev/troubleshooting#issues-with-apparmor-on-ubuntu
             filter: '--filter="./packages/*" --filter="./fixtures/*" --filter="./packages/vite-plugin-cloudflare/playground" --filter="!./packages/kv-asset-handler" --filter="!./fixtures/browser-rendering"'
           # Skipped until we upgrade to undici v7, because of https://github.com/nodejs/undici/issues/4285
-          # - os: ubuntu-24.04-arm
+          # - os: ubuntu-24.04
           #   label: v24, Linux
           #   node_version: 24
           #   filter: '--filter="./packages/*" --filter="./fixtures/*" --filter="./packages/vite-plugin-cloudflare/playground" --filter="!./packages/kv-asset-handler" --filter="!./fixtures/interactive-dev-tests"'
@@ -147,12 +147,22 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ github.token }}
 
-      - name: Run tests
+      - name: Run tests (Wrangler only)
+        # We are running Wrangler separately to be able to get early feedback on changes.
+        # There is no point in running the fixtures if Wrangler is broken.
         if: steps.changes.outputs.everything_but_markdown == 'true'
-        run: pnpm run test:ci --concurrency 1 ${{ matrix.filter }} --log-order=stream
+        run: pnpm run test:ci --log-order=stream --filter=wrangler
+        env:
+          NODE_OPTIONS: "--max_old_space_size=8192"
+          WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
+          TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
+          CI_OS: ${{ matrix.description }}
+          NODE_DEBUG: "@cloudflare:vite-plugin"
+
+      - name: Run tests (not Wrangler)
+        if: steps.changes.outputs.everything_but_markdown == 'true'
+        run: pnpm run test:ci --concurrency 1 ${{ matrix.filter }} --log-order=stream --filter=!wrangler
         env:
-          TEST_CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
-          TEST_CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
           NODE_OPTIONS: "--max_old_space_size=8192"
           WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
           TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html

fixtures/get-platform-proxy-remote-bindings/package.json

@@ -3,7 +3,7 @@
 	"private": true,
 	"description": "A test for the getPlatformProxy utility used with remote bindings",
 	"scripts": {
-		"test:ci": "vitest run",
+		"test:e2e": "vitest run",
 		"type:tests": "tsc --noEmit -p tests/tsconfig.json"
 	},
 	"devDependencies": {

fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts

@@ -27,23 +27,37 @@ if (auth) {
 				`pnpm wrangler deploy remote-worker.js --name ${remoteWorkerName} --compatibility-date 2025-06-19`,
 				execOptions
 			);
-
-			if (!new RegExp(`Deployed\\s+${remoteWorkerName}\\b`).test(deployOut)) {
-				throw new Error(`Failed to deploy ${remoteWorkerName}`);
-			}
+			const deployedUrl = deployOut.match(
+				/(?<url>https:\/\/tmp-e2e-.+?\..+?\.workers\.dev)/
+			)?.groups?.url;
+			assert(deployedUrl, "Failed to find deployed worker URL");
 
 			const stagingDeployOut = execSync(
 				`pnpm wrangler deploy remote-worker.staging.js --name ${remoteStagingWorkerName} --compatibility-date 2025-06-19`,
 				execOptions
 			);
-
-			if (
-				!new RegExp(`Deployed\\s+${remoteStagingWorkerName}\\b`).test(
-					stagingDeployOut
-				)
-			) {
-				throw new Error(`Failed to deploy ${remoteStagingWorkerName}`);
-			}
+			const stagingDeployedUrl = stagingDeployOut.match(
+				/(?<url>https:\/\/tmp-e2e-.+?\..+?\.workers\.dev)/
+			)?.groups?.url;
+			assert(stagingDeployedUrl, "Failed to find deployed staging worker URL");
+
+			// Wait for the deployed workers to be available
+			await Promise.all([
+				vi.waitFor(
+					async () => {
+						const response = await fetch(deployedUrl);
+						expect(response.status).toBe(200);
+					},
+					{ timeout: 5000, interval: 500 }
+				),
+				vi.waitFor(
+					async () => {
+						const response = await fetch(stagingDeployedUrl);
+						expect(response.status).toBe(200);
+					},
+					{ timeout: 5000, interval: 500 }
+				),
+			]);
 
 			const kvAddOut = execSync(
 				`pnpm wrangler kv namespace create ${remoteKvName}`,
@@ -65,17 +79,31 @@ if (auth) {
 		}, 35_000);
 
 		afterAll(() => {
-			execSync(`pnpm wrangler delete --name ${remoteWorkerName}`, execOptions);
-			execSync(
-				`pnpm wrangler delete --name ${remoteStagingWorkerName}`,
-				execOptions
-			);
-			execSync(
-				`pnpm wrangler kv namespace delete --namespace-id=${remoteKvId}`,
-				execOptions
-			);
+			try {
+				execSync(
+					`pnpm wrangler delete --name ${remoteWorkerName}`,
+					execOptions
+				);
+			} catch {}
+			try {
+				execSync(
+					`pnpm wrangler delete --name ${remoteStagingWorkerName}`,
+					execOptions
+				);
+			} catch {}
+			try {
+				execSync(
+					`pnpm wrangler kv namespace delete --namespace-id=${remoteKvId}`,
+					execOptions
+				);
+			} catch {}
 
-			rmSync("./.tmp", { recursive: true, force: true });
+			rmSync("./.tmp", {
+				recursive: true,
+				force: true,
+				maxRetries: 10,
+				retryDelay: 100,
+			});
 		}, 35_000);
 
 		describe("normal usage", () => {

fixtures/get-platform-proxy-remote-bindings/turbo.json

@@ -2,7 +2,7 @@
 	"$schema": "http://turbo.build/schema.json",
 	"extends": ["//"],
 	"tasks": {
-		"test:ci": {
+		"test:e2e": {
 			"env": [
 				"VITEST",
 				"NODE_DEBUG",

fixtures/vitest-pool-workers-remote-bindings/run-tests.mjs

@@ -5,9 +5,11 @@
  * This script is used to deploy the remote workers and run the fixture using said workers
  * with the appropriate vitest configurations.
  */
-import { execSync } from "child_process";
-import { randomUUID } from "crypto";
-import { cpSync, readFileSync, rmSync, writeFileSync } from "fs";
+import assert from "node:assert";
+import { execSync } from "node:child_process";
+import { randomUUID } from "node:crypto";
+import { cpSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { setTimeout } from "node:timers/promises";
 
 const env = getAuthenticatedEnv();
 if (!env) {
@@ -58,12 +60,21 @@ const deployOut = execSync(
 		stdio: "pipe",
 		cwd: "./.tmp",
 		env,
+		encoding: "utf8",
 	}
 );
+
 if (!new RegExp(`Deployed\\s+${remoteWorkerName}\\b`).test(`${deployOut}`)) {
 	throw new Error(`Failed to deploy ${remoteWorkerName}`);
 }
 
+const urlMatcher = new RegExp(
+	`(?<url>https:\\/\\/${remoteWorkerName}\\..+?\\.workers\\.dev)`
+);
+
+const deployedUrl = deployOut.match(urlMatcher)?.groups?.url;
+assert(deployedUrl, "Failed to find deployed worker URL");
+
 writeFileSync(
 	"./.tmp/remote-wrangler.staging.json",
 	JSON.stringify(
@@ -84,6 +95,7 @@ const deployStagingOut = execSync(
 		stdio: "pipe",
 		cwd: "./.tmp",
 		env,
+		encoding: "utf8",
 	}
 );
 if (
@@ -93,20 +105,50 @@ if (
 ) {
 	throw new Error(`Failed to deploy ${remoteStagingWorkerName}`);
 }
+const deployedStagingUrl = deployStagingOut.match(
+	/(?<url>https:\/\/tmp-e2e-.+?\..+?\.workers\.dev)/
+)?.groups?.url;
+assert(deployedStagingUrl, "Failed to find deployed staging worker URL");
+
+await setTimeout(2000);
+
+// Wait for the workers to become available
+await Promise.all([
+	waitFor(async () => {
+		const response = await fetch(deployedUrl);
+		assert.equal(response.status, 200);
+	}),
+	waitFor(async () => {
+		const response = await fetch(deployedStagingUrl);
+		assert.equal(response.status, 200);
+	}),
+]);
 
 try {
-	execSync("pnpm test:vitest --config ./.tmp/vitest.workers.config.ts", {
-		env,
-	});
-	execSync(
-		"pnpm test:vitest --config ./.tmp/vitest.workers.config.staging.ts",
-		{
-			env,
+	try {
+		try {
+			console.log("Running vitest-pool-workers remote bindings tests...");
+			execSync("pnpm test:vitest --config ./.tmp/vitest.workers.config.ts", {
+				env,
+				stdio: "inherit",
+			});
+			console.log(
+				"Running vitest-pool-workers remote bindings staging tests..."
+			);
+			execSync(
+				"pnpm test:vitest --config ./.tmp/vitest.workers.config.staging.ts",
+				{
+					env,
+					stdio: "inherit",
+				}
+			);
+		} finally {
+			execSync(`pnpm wrangler delete --name ${remoteWorkerName}`, { env });
 		}
-	);
+	} finally {
+		execSync(`pnpm wrangler delete --name ${remoteStagingWorkerName}`, { env });
+	}
 } finally {
-	execSync(`pnpm wrangler delete --name ${remoteWorkerName}`, { env });
-	execSync(`pnpm wrangler delete --name ${remoteStagingWorkerName}`, { env });
 	rmSync("./.tmp", { recursive: true, force: true });
 }
 
@@ -133,3 +175,22 @@ function getAuthenticatedEnv() {
 		"Skipping vitest-pool-workers remote bindings tests because the environment is not authenticated with Cloudflare."
 	);
 }
+
+/**
+ * Wait for the callback to execute successfully.
+ *
+ * If the callback throws an error or returns a rejected promise it will continue to wait until it succeeds or times out.
+ */
+export async function waitFor(callback) {
+	const start = Date.now();
+	while (true) {
+		try {
+			return callback();
+		} catch (error) {
+			if (Date.now() < start + 15_000) {
+				throw error;
+			}
+		}
+		await setTimeout(1000);
+	}
+}

packages/vite-plugin-cloudflare/e2e/helpers.ts

@@ -200,13 +200,17 @@ export function runCommand(
 	while (attempts > 0) {
 		debuglog("Running command:", command);
 		try {
-			childProcess.execSync(command, {
+			const output = childProcess.execSync(command, {
 				cwd,
-				stdio: debuglog.enabled ? "inherit" : "ignore",
+				stdio: "pipe",
 				env: testEnv,
 				timeout,
+				encoding: "utf8",
 			});
-			break;
+			if (debuglog.enabled) {
+				process.stdout.write(output);
+			}
+			return output;
 		} catch (e) {
 			attempts--;
 			if (attempts > 0) {

packages/wrangler/e2e/dev-env.test.ts

@@ -56,6 +56,9 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("switching runtimes", () => {
                                     apiToken: process.env.CLOUDFLARE_API_TOKEN,
                                 }
                             },
+                            server: {
+                                port: 0,
+                            },
                         },
                     });