[wrangler] test: fix E2E tests #4907

petebacondarwin · 2024-02-02T14:58:27Z

petebacondarwin · 2024-02-02T15:09:01Z

Why not use the get-port package for this?

RamIdeas · 2024-02-02T14:39:35Z

Nit: could probably use createDeferred here

RamIdeas · 2024-02-02T14:38:54Z

Non-blocking but... this is a bit gross and we should investigate fixing the need for this in shellac

-Original file line number
+Diff line change
@@ -0,0 +1,7 @@
+    ---
+    "wrangler": patch
+    ---
+    fix: mark R2 object and bucket not found errors as unreportable
+    Previously, running `wrangler r2 objects {get,put}` with an object or bucket that didn't exist would ask if you wanted to report that error to Cloudflare. There's nothing we can do to fix this, so this change prevents the prompt in this case.

-Original file line number
+Diff line change
@@ -0,0 +1,7 @@
+    ---
+    "wrangler": patch
+    ---
+    fix: ensure `wrangler dev --log-level` flag applied to all logs
+    Previously, `wrangler dev` may have ignored the `--log-level` flag for some startup logs. This change ensures the `--log-level` flag is applied immediately.

-Original file line number
+Diff line change
@@ Expand Up / @@ -13,7 +13,7 @@ jobs: @@
         concurrency:
           group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.node }}
           cancel-in-progress: true
-        timeout-minutes: 30
+        timeout-minutes: 15
         if: github.repository_owner == 'cloudflare' && (github.event_name != 'pull_request' || (github.event_name == 'pull_request' && contains(github.event.*.labels.*.name, 'e2e' )))
         name: "E2E Test"
         strategy:
@@ Expand Down Expand Up / @@ -73,9 +73,20 @@ jobs: @@
             id: "find-wrangler"
             run: echo "dir=$(ls $HOME/wrangler-*.tgz)" >> $GITHUB_OUTPUT;
-          - name: Run tests
-            id: e2e-1
-            continue-on-error: true
+          - name: Run tests (unix)
+            if: matrix.os == 'macos-13' || matrix.os == 'ubuntu-22.04'
+            run: |
+              pnpm add ${{ steps.find-wrangler.outputs.dir}} --global
+              pnpm run --filter wrangler test:e2e
+            env:
+              CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
+              CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
+              WRANGLER: wrangler
+              NODE_OPTIONS: "--max_old_space_size=8192"
+              WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
+          - name: Run tests (windows)
+            if: matrix.os == 'windows-2022'
             run: pnpm run --filter wrangler test:e2e
             env:
               CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -1531,6 +1531,7 @@ export class Miniflare { @@
     		// Get a `Fetcher` to that worker (NOTE: the `ProxyServer` Durable Object
     		// shares its `env` with Miniflare's entry worker, so has access to routes)
     		const bindingName = CoreBindings.SERVICE_USER_ROUTE_PREFIX + workerName;
     		const fetcher = proxyClient.env[bindingName];
     		if (fetcher === undefined) {
     			// `#findAndAssertWorkerIndex()` will throw if a "worker" doesn't exist
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     			{ value: stringifiedResult, unbufferedStream },
     			this.revivers
     		);
     		// We get an empty stack trace if we thread the caller through here,
     		// specifying `this.#parseAsyncResponse` is good enough though, we just
     		// get an extra `processTicksAndRejections` entry
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wrangler] test: fix E2E tests #4907

Uh oh!

Diff view

Diff view

There are no files selected for viewing

petebacondarwin Feb 2, 2024

Uh oh!

Uh oh!

petebacondarwin Feb 2, 2024

Uh oh!

RamIdeas Feb 2, 2024

Uh oh!

RamIdeas Feb 2, 2024

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -309,5 +309,6 @@ export function parseWithReadableStreams<RS>( @@
     		},
     		...revivers,
     	};
     	return parse(stringified.value, streamRevivers);
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -63,8 +63,7 @@ describe("c3 integration", () => { @@
     	it("deploy the worker", async () => {
     		const { stdout, stderr } = await runInWorker`$ ${WRANGLER} deploy`;
     		expect(normalize(stdout)).toMatchInlineSnapshot(`
-    			"🚧 New Workers Standard pricing is now available. Please visit the dashboard to view details and opt-in to new pricing: https://dash.cloudflare.com/CLOUDFLARE_ACCOUNT_ID/workers/standard/opt-in.
-    			Total Upload: xx KiB / gzip: xx KiB
+    			"Total Upload: xx KiB / gzip: xx KiB
     			Uploaded smoke-test-worker (TIMINGS)
     			Published smoke-test-worker (TIMINGS)
     			  https://smoke-test-worker.SUBDOMAIN.workers.dev
@@ Expand All / @@ -86,7 +85,7 @@ describe("c3 integration", () => { @@
     		const { stdout, stderr } = await runInWorker`$$ ${WRANGLER} delete`;
     		expect(normalize(stdout)).toMatchInlineSnapshot(`
     			"? Are you sure you want to delete smoke-test-worker? This action cannot be undone.
-    			🤖 Using default value in non-interactive context: yes
+    			🤖 Using fallback value in non-interactive context: yes
     			Successfully deleted smoke-test-worker"
     		`);
     		expect(stderr).toMatchInlineSnapshot('""');
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,15 +4,23 @@ import { existsSync } from "node:fs"; @@
     import * as nodeNet from "node:net";
     import path from "node:path";
     import { setTimeout } from "node:timers/promises";
-    import getPort from "get-port";
     import shellac from "shellac";
-    import { fetch } from "undici";
+    import { Agent, fetch, setGlobalDispatcher } from "undici";
     import { afterEach, beforeEach, describe, expect, it } from "vitest";
     import { normalizeOutput } from "./helpers/normalize";
     import { retry } from "./helpers/retry";
     import { dedent, makeRoot, seed } from "./helpers/setup";
     import { WRANGLER } from "./helpers/wrangler-command";
+    // Use `Agent` with lower timeouts so `fetch()`s inside `retry()`s don't block for a long time
+    setGlobalDispatcher(
+    	new Agent({
+    		connectTimeout: 10_000,
+    		headersTimeout: 10_000,
+    		bodyTimeout: 10_000,
+    	})
+    );
     type MaybePromise<T = void> = T | Promise<T>;
     const waitForPortToBeBound = async (port: number) => {
@@ Expand All / @@ -31,11 +39,7 @@ const waitUntilOutputContains = async ( @@
     		(stdout) => !stdout.includes(substring),
     		async () => {
     			await setTimeout(intervalMs);
-    			return (
-    				normalizeOutput(session.stdout) +
-    				"\n\n\n" +
-    				normalizeOutput(session.stderr)
-    			);
+    			return session.stdout + "\n\n\n" + session.stderr;
     		}
     	);
     };
@@ Expand All / @@ -46,6 +50,20 @@ interface SessionData { @@
     	stderr: string;
     }
+    function getPort() {
+    	return new Promise<number>((resolve, reject) => {
+    		const server = nodeNet.createServer((socket) => socket.destroy());
+    		server.listen(0, () => {
+    			const address = server.address();
+    			assert(typeof address === "object" && address !== null);
+    			server.close((err) => {
+    				if (err) reject(err);
+    				else resolve(address.port);
+    			});
+    		});
+    	});
+    }
     async function runDevSession(
     	workerPath: string,
     	flags: string,
@@ Expand All / @@ -65,7 +83,11 @@ async function runDevSession( @@
     		// Must use the `in` statement in the shellac script rather than `.in()` modifier on the `shellac` object
     		// otherwise the working directory does not get picked up.
+    		let promiseResolve: (() => void) | undefined;
+    		const promise = new Promise<void>((resolve) => (promiseResolve = resolve));
     		const bg = await shellac.env(process.env).bg`
+    		await ${() => promise}
     		in ${workerPath} {
     			exits {
             $ ${WRANGLER} dev ${flags}
@@ Expand All / @@ -82,12 +104,18 @@ async function runDevSession( @@
     		};
     		bg.process.stdout.on("data", (chunk) => (sessionData.stdout += chunk));
     		bg.process.stderr.on("data", (chunk) => (sessionData.stderr += chunk));
+    		// Only start `wrangler dev` once we've registered output listeners so we don't miss messages
+    		promiseResolve?.();
     		await session(sessionData);
     		return bg.promise;
     	} finally {
-    		if (pid) process.kill(pid);
+    		try {
+    			if (pid) process.kill(pid);
+    		} catch {
+    			// Ignore errors if we failed to kill the process (i.e. ESRCH if it's already terminated)
+    		}
     	}
     }
@@ Expand Down Expand Up / @@ -489,14 +517,14 @@ describe("writes debug logs to hidden file", () => { @@
     			async (session) => {
     				await waitForPortToBeBound(session.port);
-    				await waitUntilOutputContains(session, "🐛 Writing debug logs to");
+    				await waitUntilOutputContains(session, "Writing logs to");
     				await setTimeout(1000); // wait a bit to ensure the file is written to disk
     			}
     		);
     		const filepath = finalA.stdout.match(
-    			/🐛 Writing debug logs to "(.+\.log)"/
+    			/🪵 {2}Writing logs to "(.+\.log)"/
     		)?.[1];
     		assert(filepath);
@@ Expand All / @@ -511,13 +539,13 @@ describe("writes debug logs to hidden file", () => { @@
     		});
     		const filepath = finalA.stdout.match(
-    			/🐛 Writing debug logs to "(.+\.log)"/
+    			/🪵 {2}Writing logs to "(.+\.log)"/
     		)?.[1];
     		expect(filepath).toBeUndefined();
     	});
-    	it("rewrites address-in-use error logs", async () => {
+    	it.skip("rewrites address-in-use error logs", async () => {
     		// 1. start worker A on a (any) port
     		await a.runDevSession("", async (sessionA) => {
     			const normalize = (text: string) =>
@@ Expand Down @@

[wrangler] test: fix E2E tests #4907

Uh oh!

[wrangler] test: fix E2E tests #4907

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

petebacondarwin Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

petebacondarwin Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

RamIdeas Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

RamIdeas Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!