Add basic smoke tests #2796

penalosa · 2023-02-24T19:56:45Z

What this PR solves / how to test:

Adds (basic) e2e smoke tests as a pre-release check for Wrangler. Run with CLOUDFLARE_ACCOUNT_ID=$TEST_ACCOUNT_ID npm -w wrangler run test:e2e.

By default, this runs npx wrangler@beta, which can be customised using the WRANGLER environment variable

Associated docs issues/PR:

[insert associated docs issue(s)/PR(s)]

Author has included the following, where applicable:

Tests
Changeset

Reviewer has performed the following, where 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

Fixes # [insert issue number].

changeset-bot · 2023-02-24T19:56:48Z

⚠️ No Changeset found

Latest commit: effaf9e

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-02-24T19:59:49Z

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/4472098604/npm-package-wrangler-2796

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

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

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

petebacondarwin · 2023-02-27T12:54:40Z

packages/wrangler/scripts/tsconfig.json

@@ -0,0 +1,9 @@
+{


This feels a bit brittle. We now have one TS file in this folder that is built with this TS config, but then a load of others that use the top level.
If this script is fundamentally different, then perhaps we should have two folders?
Otherwise I think we should try to keep all these source files compiling with the same config.

petebacondarwin · 2023-02-27T12:56:22Z

packages/wrangler/package.json

@@ -65,7 +65,8 @@
 		"start": "npm run bundle && cross-env NODE_OPTIONS=--enable-source-maps ./bin/wrangler.js",
 		"test": "npm run assert-git-version && jest --silent=false --verbose=true --forceExit",
 		"test-watch": "npm run test -- --runInBand --testTimeout=50000 --watch",
-		"test:ci": "npm run test -- --verbose=true --coverage"
+		"test:ci": "npm run test -- --verbose=true --coverage",
+		"test:smoke": "esbuild --bundle scripts/smoke-test.ts --outfile=scripts/smoke-test.mjs --platform=node --format=esm && zx scripts/smoke-test.mjs && rm scripts/smoke-test.mjs"


I feel like we should not be polluting the scripts folder with generated files, even if they are gitignored. Can we put these in some dist-... folder that is gitignored?

Alternatively, I note that other scripts are just compiled on demand using:

node -r esbuild-register ...

mrbbot

Nice! The zx package looks pretty cool, but it's pretty big. Did you consider any alternatives?

packages/wrangler/scripts/smoke-test.ts

penalosa · 2023-03-01T13:54:54Z

@cloudflare/wrangler This has changed a fair amount—I've moved everything to vitest and shellac, and into an e2e folder

mrbbot · 2023-03-01T14:37:40Z

packages/wrangler/e2e/helpers/normalise.ts

+	return npmStripTimings(
+		removeWorkersDev(
+			removeUUID(
+				normalizeErrorMarkers(
+					replaceByte(
+						stripTrailingWhitespace(
+							normalizeSlashes(
+								normalizeTempDirs(stripTimings(removeVersionHeader(stdout)))
+							)
+						)
+					)
+				)
+			)
+		)
+	);


That's a lot of nesting but I love the composability 😅 , maybe something like this would be nicer?

Suggested change

return npmStripTimings(

removeWorkersDev(

removeUUID(

normalizeErrorMarkers(

replaceByte(

stripTrailingWhitespace(

normalizeSlashes(

normalizeTempDirs(stripTimings(removeVersionHeader(stdout)))

)

)

)

)

)

)

);

const functions = [

removeWorkersDev,

removeUUID,

...

];

for (const f of functions) stdout = f(stdout);

return stdout;

I am in favor of some sort of refactor.

Is this also replacing the other normalizeOutput function for the rest of the tests? If not we should refactor that one the same way.

No, this is intentionally kept separate from the jest tests

mrbbot · 2023-03-01T14:40:53Z

packages/wrangler/e2e/publish.test.ts

+    `;
+		expect(stdout).toMatchInlineSnapshot(`
+			"Using npm as package manager.
+			✨ Created ${workerName}/wrangler.toml


How does interpolation interact with toMatchInlineSnapshot()? Do you have to add the ${...}s back when updating the snapshot?

Yeah, it is generally not encouraged to have dynamic snapshots like this.

I've added an explicit replacers option to runIn which should make this simpler

packages/wrangler/e2e/publish.test.ts

packages/wrangler/e2e/r2.test.ts

packages/wrangler/e2e/util.ts

JacobMGEvans

Discussing file naming internally.

packages/wrangler/e2e/util.ts

packages/wrangler/e2e/vite.config.ts

codecov · 2023-03-03T18:22:34Z

Codecov Report

Merging #2796 (effaf9e) into main (8b81abd) will decrease coverage by 0.41%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2796      +/-   ##
==========================================
- Coverage   73.94%   73.54%   -0.41%     
==========================================
  Files         167      166       -1     
  Lines       10295    10397     +102     
  Branches     2748     2779      +31     
==========================================
+ Hits         7613     7646      +33     
- Misses       2682     2751      +69

see 21 files with indirect coverage changes

packages/wrangler/e2e/helpers/setup.ts

packages/wrangler/e2e/helpers/normalise.ts

packages/wrangler/e2e/publish.test.ts

packages/wrangler/e2e/r2.test.ts

packages/wrangler/e2e/tsconfig.json

packages/wrangler/e2e/helpers/normalise.ts

…command, and just generally be more resilient

…enalosa/smoke-tests

mrbbot

Apologies for the delayed review, looks really good! :) Once comment about Windows compatibility, but other than that... ✅

mrbbot · 2023-03-21T11:31:38Z

packages/wrangler/e2e/helpers/run.ts

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


This won't work on Windows. Is there any way we can specify environment variables with shellac? Does it inherit process.env if we modified that?

This is definitely a concern. I'll merge this for now, but we should loop back on Windows support soon.

penalosa requested a review from a team as a code owner February 24, 2023 19:56

petebacondarwin reviewed Feb 27, 2023

View reviewed changes

mrbbot reviewed Feb 27, 2023

View reviewed changes

packages/wrangler/scripts/smoke-test.ts Outdated Show resolved Hide resolved

packages/wrangler/scripts/smoke-test.ts Outdated Show resolved Hide resolved

packages/wrangler/scripts/smoke-test.ts Outdated Show resolved Hide resolved

penalosa requested review from petebacondarwin, mrbbot and JacobMGEvans March 1, 2023 13:53

mrbbot reviewed Mar 1, 2023

View reviewed changes

JacobMGEvans suggested changes Mar 1, 2023

View reviewed changes

packages/wrangler/e2e/util.ts Outdated Show resolved Hide resolved

packages/wrangler/e2e/vite.config.ts Outdated Show resolved Hide resolved

JacobMGEvans approved these changes Mar 3, 2023

View reviewed changes

penalosa force-pushed the penalosa/smoke-tests branch from a76d6e3 to 648458b Compare March 3, 2023 18:24

penalosa requested review from JacobMGEvans and mrbbot March 3, 2023 18:24

mrbbot reviewed Mar 6, 2023

View reviewed changes

penalosa added 12 commits March 6, 2023 13:01

Add basic smoke tests

8fbb2d1

Refactor e2e tests to use Vitest, allow for customising the Wrangler …

498502c

…command, and just generally be more resilient

Remove initial test file

aa9f4fe

smoke -> e2e

6536ed3

Cleanup

b287415

More resilient

83432c9

Linting

1398957

Linting

d33e8bd

Linting

d2d2f31

Linting

59c4a7e

regex?

fe74061

Address PR feedback

439efdc

penalosa force-pushed the penalosa/smoke-tests branch from e18c11a to 439efdc Compare March 6, 2023 13:01

JacobMGEvans requested a review from mrbbot March 13, 2023 18:16

JacobMGEvans and others added 4 commits March 13, 2023 13:23

Merge branch 'main' of https://github.com/cloudflare/wrangler2 into p…

5da5412

…enalosa/smoke-tests

Address PR comments

ad11af9

lint

c97d4f6

Add deployments

effaf9e

mrbbot approved these changes Mar 21, 2023

View reviewed changes

penalosa merged commit e062180 into main Mar 21, 2023

penalosa deleted the penalosa/smoke-tests branch March 21, 2023 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic smoke tests #2796

Add basic smoke tests #2796

penalosa commented Feb 24, 2023 •

edited

Loading

changeset-bot bot commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading

petebacondarwin Feb 27, 2023

petebacondarwin Feb 27, 2023

petebacondarwin Feb 27, 2023

mrbbot left a comment

penalosa commented Mar 1, 2023

mrbbot Mar 1, 2023

JacobMGEvans Mar 1, 2023

JacobMGEvans Mar 1, 2023

penalosa Mar 1, 2023

mrbbot Mar 1, 2023 •

edited

Loading

petebacondarwin Mar 1, 2023

penalosa Mar 1, 2023

JacobMGEvans left a comment

codecov bot commented Mar 3, 2023 •

edited

Loading

mrbbot left a comment

mrbbot Mar 21, 2023 •

edited

Loading

penalosa Mar 21, 2023

Add basic smoke tests #2796

Add basic smoke tests #2796

Conversation

penalosa commented Feb 24, 2023 • edited Loading

changeset-bot bot commented Feb 24, 2023 • edited Loading

⚠️ No Changeset found

github-actions bot commented Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrbbot left a comment

Choose a reason for hiding this comment

penalosa commented Mar 1, 2023

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

mrbbot Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacobMGEvans left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 3, 2023 • edited Loading

Codecov Report

mrbbot left a comment

Choose a reason for hiding this comment

mrbbot Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penalosa commented Feb 24, 2023 •

edited

Loading

changeset-bot bot commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading

mrbbot Mar 1, 2023 •

edited

Loading

codecov bot commented Mar 3, 2023 •

edited

Loading

mrbbot Mar 21, 2023 •

edited

Loading