Add e2e tests to CI #3344

penalosa · 2023-05-25T17:54:33Z

What this PR solves / how to test:

e2e testing in CI! This adds a run of the e2e tests to the pull request and main branch workflows, as well as adding basic tests for wrangler dev. It will be removed from the pull request workflow before this PR gets merged, but it's there for now to demonstrate how the tests work. Currently, about half the tests fail because of OS limitations of workerd—I'm in two minds about whether we should keep those failing tests in to remind us, or skip them for now.

Author has included the following, where applicable:

Tests
~~[ ] Changeset (Changeset guidelines)~~

Reviewer is to perform the following, as applicable:

Checked for inclusion of relevant tests
Checked for inclusion of a relevant changeset
Checked for creation of associated docs updates
Manually pulled down the changes and spot-tested

changeset-bot · 2023-05-25T17:54:37Z

⚠️ No Changeset found

Latest commit: e5af057

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2023-05-25T18:03:28Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5266760836/npm-package-wrangler-3344

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/3344/npm-package-wrangler-3344

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5266760836/npm-package-wrangler-3344 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5266760836/npm-package-cloudflare-pages-shared-3344

Note that these links will no longer work once the GitHub Actions artifact expires.

.github/workflows/pullrequests.yml

codecov · 2023-05-25T18:07:47Z

Codecov Report

Merging #3344 (e5af057) into main (05099f4) will decrease coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3344      +/-   ##
==========================================
- Coverage   75.32%   75.20%   -0.12%     
==========================================
  Files         183      183              
  Lines       11022    11052      +30     
  Branches     2893     2904      +11     
==========================================
+ Hits         8302     8312      +10     
- Misses       2720     2740      +20

see 5 files with indirect coverage changes

packages/wrangler/e2e/dev.test.ts

mrbbot · 2023-05-25T21:29:41Z

packages/wrangler/e2e/dev.test.ts

+			await retry(() =>
+				expect(
+					fetch(`http://127.0.0.1:${port}`).then((r) => r.text())
+				).resolves.toMatchInlineSnapshot('"Hello World!"')


I guess snapshots don't really work with retries... 😅

Suggested change

).resolves.toMatchInlineSnapshot('"Hello World!"')

).resolves.toMatchInlineSnapshot('"Updated Worker!"')

This is actually a bit worrying, from a maintenance perspective.
The problem with retry is that it will pass if you put in the "old" data and wrangler has not yet updated the worker.

packages/wrangler/e2e/dev.test.ts

packages/wrangler/e2e/helpers/retry.ts

packages/wrangler/e2e/dev.test.ts

petebacondarwin

LGTM once the "modify" tests are fixed.

.github/workflows/main.yml

packages/wrangler/e2e/dev.test.ts

petebacondarwin · 2023-05-31T11:59:19Z

packages/wrangler/e2e/helpers/retry.ts

+	if (n === 0) {
+		await promise();
+	} else {
+		try {
+			await promise();
+		} catch {
+			await setTimeout(2_000);
+			return retry(promise, n - 1);
+		}
+	}


Just for fun this could also be written without recursion:

async function retry(promise: ..., n = 20) { for (let i = 0; i < n; i++) { try { return promise(); } catch { await setTimeout(2000); } } throw new Error("Timed out waiting for promise to resolve"); }

Not interested? 😿

I kinda prefer the recursive way, since it should throw the proper error from the promise on the last run

petebacondarwin · 2023-05-31T12:04:27Z

packages/wrangler/e2e/dev.test.ts

+			await retry(() =>
+				expect(
+					fetch(`http://127.0.0.1:${port}`).then((r) => r.text())
+				).resolves.toMatchInlineSnapshot('"Hello World!"')


This is actually a bit worrying, from a maintenance perspective.
The problem with retry is that it will pass if you put in the "old" data and wrangler has not yet updated the worker.

petebacondarwin · 2023-06-13T19:08:57Z

.github/workflows/e2e.yml

+          NODE_ENV: "production"
+
+      - name: Run tests
+        run: cd packages/wrangler && npm run test:e2e


Can you not use the npm workspace rather than cd-ing?

Suggested change

run: cd packages/wrangler && npm run test:e2e

run: npm run -w wrangler test:e2e

petebacondarwin · 2023-06-13T19:10:25Z

packages/wrangler/package.json

@@ -63,7 +63,7 @@
 		"test": "npm run assert-git-version && jest",
 		"test:ci": "npm run test -- --coverage",
 		"test:debug": "npm run test -- --silent=false --verbose=true",
-		"test:e2e": "vitest --single-thread ./e2e",
+		"test:e2e": "vitest --single-thread --test-timeout 120000 --dir ./e2e run",


Why the --test-timeout here and in the vite.config.ts file too?

petebacondarwin · 2023-06-13T19:18:13Z

packages/wrangler/e2e/helpers/retry.ts

+	if (n === 0) {
+		await promise();
+	} else {
+		try {
+			await promise();
+		} catch {
+			await setTimeout(2_000);
+			return retry(promise, n - 1);
+		}
+	}


Not interested? 😿

petebacondarwin · 2023-06-13T19:29:22Z

packages/wrangler/e2e/helpers/run.ts

-const WRANGLER = process.env.WRANGLER ?? `npx wrangler@beta`;
-
-const RUN = `${CF_ID} ${WRANGLER}`;
+const RUN = process.env.WRANGLER ?? `npx --prefer-offline wrangler@beta`;


Rather than hitting the beta deployed version, I think we could do:

Suggested change

const RUN = process.env.WRANGLER ?? `npx --prefer-offline wrangler@beta`;

const pathToWrangler = path.resolve(__dirname, "../../..");

const RUN = process.env.WRANGLER ?? `node "${pathToWrangler}"`;

And that might even avoid having to set the WRANGLER package in the CI job - and even avoid all that packaging and copying/moving in the job too.

Or were you trying to be even more "pure" in terms of e2e?

Yeah, the idea was that it should mimic running a package installed from npm

petebacondarwin · 2023-06-13T19:32:03Z

Interestingly I ran this locally and got:

> wrangler@3.1.0 test:e2e
> vitest --single-thread --test-timeout 120000 --dir ./e2e run


 RUN  v0.31.4 /Users/pbacondarwin/dev/wrangler2/packages/wrangler

 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
     ⠇ can modify worker during dev session (local)
     · can modify worker during dev session (remote)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
 ❯ e2e/dev.test.ts (2)
 ❯ e2e/dev.test.ts (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
     ✓ can modify worker during dev session (local) 4054ms
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
 ❯ e2e/dev.test.ts (2)
 ❯ e2e/dev.test.ts (2)
   ❯ basic dev tests (2)
     ✓ can modify worker during dev session (local) 4054ms
⎔ Detected changes, restarted server.

 ❯ e2e/dev.test.ts (2)
 ✓ e2e/dev.test.ts (2) 10899ms
 ✓ e2e/r2.test.ts (7) 17519ms
 ✓ e2e/deployments.test.ts (8) 91675ms
 ✓ e2e/deploy.test.ts (4) 71216ms

  Snapshots  2 failed
 Test Files  4 passed (4)
      Tests  21 passed (21)
   Start at  20:26:54
   Duration  193.96s (transform 98ms, setup 0ms, collect 2.47s, tests 191.31s, environment 0ms, prepare 43ms)

Note it says that two snapshots failed, but does not tell me which...

penalosa · 2023-06-13T20:23:44Z

Note it says that two snapshots failed, but does not tell me which...

I think these are snapshot failures that are caught by retry()

penalosa requested a review from a team May 25, 2023 17:54

jculvey reviewed May 25, 2023

View reviewed changes

.github/workflows/pullrequests.yml Outdated Show resolved Hide resolved

mrbbot reviewed May 25, 2023

View reviewed changes

petebacondarwin requested changes May 31, 2023

View reviewed changes

penalosa force-pushed the penalosa/e2e-ci branch 5 times, most recently from d861067 to 966dc4a Compare June 13, 2023 11:20

penalosa added the e2e Run e2e tests on a PR label Jun 13, 2023

penalosa force-pushed the penalosa/e2e-ci branch 6 times, most recently from 697381c to 9c4386c Compare June 13, 2023 12:21

Add e2e tests to CI

e49a53f

penalosa force-pushed the penalosa/e2e-ci branch from 9c4386c to e49a53f Compare June 13, 2023 13:39

petebacondarwin self-requested a review June 13, 2023 19:04

petebacondarwin reviewed Jun 13, 2023

View reviewed changes

PR comments

e8e864c

petebacondarwin approved these changes Jun 13, 2023

View reviewed changes

Vite config isn't being read, so use CLI args

e5af057

penalosa merged commit 5cfb94c into main Jun 14, 2023

penalosa deleted the penalosa/e2e-ci branch June 14, 2023 14:14

mrbbot mentioned this pull request Jun 28, 2023

Implement wrangler dev --(experimental-)local E2E tests #2365

Closed

6 tasks

lrapoport-cf pushed a commit that referenced this pull request Aug 11, 2023

Add e2e tests to CI (#3344)

a520190

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add e2e tests to CI #3344

Add e2e tests to CI #3344

penalosa commented May 25, 2023 •

edited

Loading

changeset-bot bot commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

codecov bot commented May 25, 2023 •

edited

Loading

mrbbot May 25, 2023

petebacondarwin May 31, 2023

petebacondarwin left a comment

petebacondarwin May 31, 2023

petebacondarwin Jun 13, 2023

penalosa Jun 13, 2023

petebacondarwin May 31, 2023

petebacondarwin Jun 13, 2023

petebacondarwin Jun 13, 2023

petebacondarwin Jun 13, 2023

petebacondarwin Jun 13, 2023

petebacondarwin Jun 13, 2023

penalosa Jun 13, 2023

petebacondarwin commented Jun 13, 2023

penalosa commented Jun 13, 2023 •

edited

Loading

	).resolves.toMatchInlineSnapshot('"Hello World!"')
	).resolves.toMatchInlineSnapshot('"Updated Worker!"')

	run: cd packages/wrangler && npm run test:e2e
	run: npm run -w wrangler test:e2e

	const RUN = process.env.WRANGLER ?? `npx --prefer-offline wrangler@beta`;
	const pathToWrangler = path.resolve(__dirname, "../../..");
	const RUN = process.env.WRANGLER ?? `node "${pathToWrangler}"`;

Add e2e tests to CI #3344

Add e2e tests to CI #3344

Conversation

penalosa commented May 25, 2023 • edited Loading

changeset-bot bot commented May 25, 2023 • edited Loading

⚠️ No Changeset found

github-actions bot commented May 25, 2023 • edited Loading

codecov bot commented May 25, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin commented Jun 13, 2023

penalosa commented Jun 13, 2023 • edited Loading

penalosa commented May 25, 2023 •

edited

Loading

changeset-bot bot commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

codecov bot commented May 25, 2023 •

edited

Loading

penalosa commented Jun 13, 2023 •

edited

Loading