test: improve Windows reliability of e2e tests

[petebacondarwin]

What this PR solves / how to test

• child_process.kill() doesn't really work on Windows. Using kill-tree library instead
• killAllWranglerDev() helper could mess up parallel tests since it kills all wrangler/workerd processes regardless of whether they are part of the current test
• shellac library has issues on Windows when running background tasks. Using simple execSync and spawn functions instead
• tightened up the timeouts a bit to avoid long running false negatives
• reworked the helpers to use a class WranglerE2ETestHelper rather than the e2eTest extension.

Author has addressed the following

• Tests
  • [ ] TODO (before merge)
  • [x] Included
  • [ ] Not necessary because:
• E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
  • [ ] I don't know
  • [x] Required / Maybe required
  • [ ] Not required because:
• Changeset (Changeset guidelines)
  • [ ] TODO (before merge)
  • [ ] Included
  • [x] Not necessary because: test refactor
• Public documentation
  • [ ] TODO (before merge)
  • [ ] Cloudflare docs PR(s):
  • [x] Not necessary because: test refactor

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: d38faec3ed6c3d6b2e04e2ee5f6fb40d07fcdf1e

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[bot]]

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/9680655364/npm-package-wrangler-6077

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

```sh npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9680655364/npm-package-create-cloudflare-6077 --no-auto-update ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9680655364/npm-package-cloudflare-kv-asset-handler-6077 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9680655364/npm-package-miniflare-6077 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9680655364/npm-package-cloudflare-pages-shared-6077 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9680655364/npm-package-cloudflare-vitest-pool-workers-6077 ``` Note that these links will no longer work once [the GitHub Actions artifact expires](https://docs.github.com/en/organizations/managing-organization-settings/configuring-the-retention-period-for-github-actions-artifacts-and-logs-in-your-organization).

---

wrangler@3.62.0 includes the following runtime dependencies:

Package	Constraint	Resolved
miniflare	workspace:*	3.20240620.0
workerd	1.20240620.1	1.20240620.1
workerd --version	1.20240620.1	2024-06-20

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

[petebacondarwin]

I'll review this in detail next week, but it looks like a big change here is migrating from vitest fixtures to a new helper class that needs to be instantiated in each test. To me, that seems harder to use, and adds boilerplate to each test run. What was the reasoning for moving away from vitest fixtures here?

It also looks like there's a change from a single run() function to a runWranglerLongLived() and a run() function.

The boilerplate is actually quite small. The main difference is that you instantiate a class to get access to the methods rather than changing the it calls to e2eTest calls.

The primary motivation was that it was not possible to use e2eTest to create a context that persisted across it calls, which is an approach used in a bunch of the tests.

But also, for me, the e2eTest approach used a bunch of Vitest magic that was not apparent on first looking at the code and required me to spend time digging around and reading up on the Vitest docs to understand what was going on and how it works. Simple class instantiation can be understood by anyone with a basic understanding of JS.

The move to split the run command in two is somewhat subjective, but I feel that it is more discoverable since they have quite different flows. It was only when I was updating the docs that I realised that you could just await the original run function to get a promise that effectively had very different usage than not awaiting it. While quite clever this is hard to discover and not intuitive. Finally the implementation of the new run command is much less complex and avoids having to worry about killing the async child process since it just uses standard Node.js methods to run the process to completion.

[petebacondarwin]

Ironically e2e tests are now more stable but the fixture tests are flaking 😢