C3: Improve e2e testing reliability #3686

jculvey · 2023-07-28T22:21:54Z

What this PR solves / how to test:

Improves the reliability of c3 e2e tests by putting a more stable cleanup script in place that runs before each test run. Also does some refactoring to have a re-usable composite action for checkout and install. Lays some additional ground work for pnpm tests which still need some work

[ x ] 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-07-28T22:21:56Z

⚠️ No Changeset found

Latest commit: a3957bf

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-07-28T22:25:50Z

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/5869937533/npm-package-wrangler-3686

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

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

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

codecov · 2023-07-28T22:30:18Z

Codecov Report

Merging #3686 (0ec691b) into main (2ee9815) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3686   +/-   ##
=======================================
  Coverage   75.17%   75.17%           
=======================================
  Files         194      194           
  Lines       11410    11410           
  Branches     3013     3013           
=======================================
  Hits         8577     8577           
  Misses       2833     2833

macrombilux · 2023-08-14T21:22:16Z

https://app.codecov.io/gh/cloudflare/workers-sdk/pull/3686?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare

dario-piotrowicz

I just noticed that the PR got merged as I was reviewing it 😭

(I figured I'd at least publish the comment I had....)

dario-piotrowicz · 2023-08-15T18:17:50Z

.github/actions/install-dependencies/action.yml

+    - name: ESlint and Typescript caching
+      uses: actions/cache@v3
+      id: eslint-cache
+      with:
+        path: |
+          .eslintcache
+          tsconfig.tsbuildinfo
+        key: ${{ matrix.os }}-eslint-tsbuildinfo-${{ hashFiles('**/*.ts','**/*.js', 'package.json', 'tsconfig.json') }}


is this supposed to be in the install-dependencies actions?

dario-piotrowicz · 2023-08-15T18:26:52Z

packages/create-cloudflare/src/frameworks/versionMap.json

@@ -6,7 +6,7 @@
 	"hono": "0.2.6",
 	"next": "13.4.2",
 	"nuxt": "3.4.2",
-	"qwik": "1.1.x",
+	"qwik": "1.2.7",


out of curiosity, since before it was 1.1.x why not going to 1.2.x?

Didn't have a ton of time to test multiple 1.2.x versions so I just bumped to latest. I also didn't like that it was sort of a one off, I'd like to re-evaluate our version pinning and use .x for everything or pin to specific consistently.

dario-piotrowicz · 2023-08-15T18:31:19Z

packages/create-cloudflare/src/helpers/packages.ts

-	const { name, version } = whichPmRuns() ?? { name: "npm", version: "0.0.0" };
+	let { name, version } = whichPmRuns() ?? { name: "npm", version: "0.0.0" };
+
+	if (process.env.TEST_PM) {


just to understand things better, I don't see you setting this anywhere, so I assume that it isn't currently used right?

In package.json:
"test:e2e:npm": "npm run build && TEST_PM=npm vitest run --config ./vitest-e2e.config.ts",

ah ok, sorry, somehow I missed that 🤦

dario-piotrowicz · 2023-08-15T18:35:41Z

packages/create-cloudflare/src/helpers/command.ts

+
+	const nodeModulesPath = path.join(ctx.project.path, "node_modules");
+	rmSync(nodeModulesPath, { recursive: true });
+	const lockfilePath = path.join(ctx.project.path, "package-lock.json");


is this correct? this code runs for pnpm and yarn but those two don't produce a package-lock.json, I think that they produce respectively a pnpm-lock.yaml and a yarn.lock

(maybe all possible lock files should get deleted? 🤷)

Right, the idea here is that if you're using yarn or pnpm, it will delete the lockfile and node_modules, then install with the appropriate package manager, thus creating fresh (and hopefully working) dependencies, and a new lockfile to describe them.

The issue is that some create scripts use npm regardless of which PM you choose, and these need to be reset before we try to install wrangler with pnpm or yarn. Otherwise, it fails

I see thanks 🙂👍

jculvey requested review from a team as code owners July 28, 2023 22:21

jculvey force-pushed the c3-e2e-package-managers branch 3 times, most recently from a28ae4f to fdfacd0 Compare July 28, 2023 22:56

jculvey force-pushed the c3-e2e-package-managers branch 5 times, most recently from 29b5529 to c6e2d01 Compare August 11, 2023 21:19

jculvey force-pushed the c3-e2e-package-managers branch from c56b595 to 8aa7ed7 Compare August 14, 2023 21:16

jculvey changed the title ~~WIP: C3: Add e2e coverage for pnpm and yarn~~ C3: Improve e2e testing reliability Aug 15, 2023

jculvey force-pushed the c3-e2e-package-managers branch 2 times, most recently from 22fcc2d to fb66ef6 Compare August 15, 2023 03:14

jculvey added 12 commits August 15, 2023 12:30

C3: Add e2e coverage for pnpm and yarn

24f3ee5

Skipping some tests for debug purposes

891b691

Fixing env variable passing to e2e process

082d6dd

Change how we force package managers in e2e

5001f47

Create a job to cleanup e2e pages projects before running

928a8de

Add required env variables to cleanup job

d36df89

Consolidate temp directory creation logic

0a8a8df

debug cli tmp project creation

7f79500

Skip react and next for debug

bf1bf40

Re-enable all tests

4d787e1

Running tests serially for debug

b6232a7

Fix bug in cleanup

bb54748

jculvey added 7 commits August 15, 2023 12:30

Debug: limit to npm

78d9489

Update test account in c3 tests

5ccc2cd

Lower the concurrency to help with rate limiting

2eba1b5

Fixing pnpm issues for react and next

b9025d8

Configure dummy git user and limit runs to npm

def8aba

Add more logging

e662ee6

Parse deployment url from c3 run output. Fix hono expectations

a3957bf

jculvey force-pushed the c3-e2e-package-managers branch from 0ec691b to a3957bf Compare August 15, 2023 17:30

penalosa approved these changes Aug 15, 2023

View reviewed changes

penalosa merged commit d885fbe into main Aug 15, 2023
15 checks passed

penalosa deleted the c3-e2e-package-managers branch August 15, 2023 18:33

dario-piotrowicz reviewed Aug 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C3: Improve e2e testing reliability #3686

C3: Improve e2e testing reliability #3686

jculvey commented Jul 28, 2023 •

edited

Loading

changeset-bot bot commented Jul 28, 2023 •

edited

Loading

github-actions bot commented Jul 28, 2023 •

edited

Loading

codecov bot commented Jul 28, 2023 •

edited

Loading

macrombilux commented Aug 14, 2023

dario-piotrowicz left a comment

dario-piotrowicz Aug 15, 2023

dario-piotrowicz Aug 15, 2023

jculvey Aug 15, 2023

dario-piotrowicz Aug 15, 2023

jculvey Aug 15, 2023

dario-piotrowicz Aug 15, 2023

dario-piotrowicz Aug 15, 2023 •

edited

Loading

jculvey Aug 15, 2023

dario-piotrowicz Aug 15, 2023

C3: Improve e2e testing reliability #3686

C3: Improve e2e testing reliability #3686

Conversation

jculvey commented Jul 28, 2023 • edited Loading

changeset-bot bot commented Jul 28, 2023 • edited Loading

⚠️ No Changeset found

github-actions bot commented Jul 28, 2023 • edited Loading

codecov bot commented Jul 28, 2023 • edited Loading

Codecov Report

macrombilux commented Aug 14, 2023

dario-piotrowicz 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

dario-piotrowicz Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jculvey commented Jul 28, 2023 •

edited

Loading

changeset-bot bot commented Jul 28, 2023 •

edited

Loading

github-actions bot commented Jul 28, 2023 •

edited

Loading

codecov bot commented Jul 28, 2023 •

edited

Loading

dario-piotrowicz Aug 15, 2023 •

edited

Loading