Skip to content

Commit

Permalink
Upgrade @cloudflare/vitest-pool-workers to Vitest v2+ (#6232)
Browse files Browse the repository at this point in the history
  • Loading branch information
penalosa authored Sep 13, 2024
1 parent aa603ab commit f3c0400
Show file tree
Hide file tree
Showing 26 changed files with 266 additions and 412 deletions.
7 changes: 7 additions & 0 deletions .changeset/soft-beans-explode.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@cloudflare/vitest-pool-workers": minor
---

feat: Support Vitest v2.0.x

This drops support for Vitest@1, due to the issues described in [workers-sdk#6071](https://github.com/cloudflare/workers-sdk/issues/6071)
2 changes: 1 addition & 1 deletion fixtures/asset-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@cloudflare/workers-types": "^4.20240821.1",
"run-script-os": "^1.1.6",
"undici": "^5.28.4",
"vitest": "1.5.0",
"vitest": "catalog:default",
"wrangler": "workspace:*"
},
"volta": {
Expand Down
6 changes: 3 additions & 3 deletions fixtures/asset-config/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ export default defineWorkersConfig({
// always "externalised", meaning they're imported directly by Node.
server: {
deps: {
// Vitest automatically adds `/^(?!.*(?:node_modules)).*\.mjs$/` as an
// `inline` RegExp: https://github.com/vitest-dev/vitest/blob/v1.5.0/packages/vitest/src/node/config.ts#L236
// Vitest automatically adds `/^(?!.*node_modules).*\.mjs$/` as an
// `inline` RegExp: https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vitest/src/constants.ts#L9
// We'd like `packages/vitest-pool-workers/dist/pool/index.mjs` to be
// externalised though. Unfortunately, `inline`s are checked before
// `external`s, so there's no nice way we can override this. Instead,
// we prevent the extra `inline` being added in the first place.
inline: new FilteredPushArray((item) => {
const str = `${item}`;
return str !== "/^(?!.*(?:node_modules)).*\\.mjs$/";
return str !== "/^(?!.*node_modules).*\\.mjs$/";
}),
external: [
/packages\/vitest-pool-workers\/dist/,
Expand Down
4 changes: 2 additions & 2 deletions fixtures/local-mode-tests/tests/logging.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import path from "node:path";
import { setTimeout } from "node:timers/promises";
import util from "node:util";
import { afterEach, beforeEach, expect, it, vi } from "vitest";
import { afterEach, beforeEach, expect, it, Mock, vi } from "vitest";
import { unstable_dev } from "wrangler";

let output = "";
function spyOnConsoleMethod(name: keyof typeof console) {
vi.spyOn(console, name).mockImplementation((...args: unknown[]) => {
(vi.spyOn(console, name) as Mock).mockImplementation((...args: unknown[]) => {
output += util.format(...args) + "\n";
});
}
Expand Down
3 changes: 2 additions & 1 deletion fixtures/vitest-pool-workers-examples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"type": "module",
"scripts": {
"check:type": "node tsc-all.mjs",
"list": "vitest --config vitest.workers.config.ts list",
"test": "vitest --config vitest.workers.config.ts --reporter basic",
"test:ci": "vitest run --config vitest.workers.config.ts --reporter basic"
},
Expand All @@ -16,7 +17,7 @@
"miniflare": "workspace:*",
"toucan-js": "^3.3.1",
"typescript": "^5.5.2",
"vitest": "1.5.0",
"vitest": "catalog:default",
"wrangler": "workspace:*"
},
"volta": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ export default defineConfig({
// always "externalised", meaning they're imported directly by Node.
server: {
deps: {
// Vitest automatically adds `/^(?!.*(?:node_modules)).*\.mjs$/` as an
// `inline` RegExp: https://github.com/vitest-dev/vitest/blob/v1.5.0/packages/vitest/src/node/config.ts#L236
// Vitest automatically adds `/^(?!.*node_modules).*\.mjs$/` as an
// `inline` RegExp: https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vitest/src/constants.ts#L9
// We'd like `packages/vitest-pool-workers/dist/pool/index.mjs` to be
// externalised though. Unfortunately, `inline`s are checked before
// `external`s, so there's no nice way we can override this. Instead,
// we prevent the extra `inline` being added in the first place.
inline: new FilteredPushArray((item) => {
const str = item.toString();
return str !== "/^(?!.*(?:node_modules)).*\\.mjs$/";
return str !== "/^(?!.*node_modules).*\\.mjs$/";
}),
external: [
/packages\/vitest-pool-workers\/dist/,
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"turbo": "^1.10.14",
"typescript": "^5.5.2",
"vite": "^5.0.12",
"vitest": "^1.2.2"
"vitest": "catalog:default"
},
"packageManager": "pnpm@9.1.3",
"engines": {
Expand All @@ -75,7 +75,7 @@
"@types/react-transition-group>@types/react": "^18",
"@cloudflare/elements>@types/react": "^18",
"capnpc-ts>typescript": "4.2.4",
"vitest@1.5.0>@types/node": "$@types/node"
"vitest>@types/node": "$@types/node"
},
"patchedDependencies": {
"ink@3.2.0": "patches/ink@3.2.0.patch",
Expand Down
2 changes: 2 additions & 0 deletions packages/create-cloudflare/e2e-tests/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import assert from "assert";
import {
createWriteStream,
mkdirSync,
Expand Down Expand Up @@ -284,6 +285,7 @@ export const waitForExit = async (
export const createTestLogStream = (ctx: TaskContext) => {
// The .ansi extension allows for editor extensions that format ansi terminal codes
const fileName = `${normalizeTestName(ctx)}.ansi`;
assert(ctx.task.suite, "Suite must be defined");
return createWriteStream(path.join(getLogPath(ctx.task.suite), fileName), {
flags: "a",
});
Expand Down
14 changes: 12 additions & 2 deletions packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import zlib from "zlib";
import exitHook from "exit-hook";
import { $ as colors$ } from "kleur/colors";
import stoppable from "stoppable";
import { Dispatcher, getGlobalDispatcher, Pool } from "undici";
import {
Dispatcher,
getGlobalDispatcher,
Pool,
Response as UndiciResponse,
} from "undici";
import SCRIPT_MINIFLARE_SHARED from "worker:shared/index";
import SCRIPT_MINIFLARE_ZOD from "worker:shared/zod";
import { WebSocketServer } from "ws";
Expand Down Expand Up @@ -830,7 +835,12 @@ export class Miniflare {
// Should only define custom service bindings if `service` is a function
assert(typeof service === "function");
try {
const response = await service(request, this);
let response: UndiciResponse | Response = await service(request, this);

if (!(response instanceof Response)) {
response = new Response(response.body, response);
}

// Validate return type as `service` is a user defined function
// TODO: should we validate outside this try/catch?
return z.instanceof(Response).parse(response);
Expand Down
12 changes: 6 additions & 6 deletions packages/vitest-pool-workers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,19 @@
"@cloudflare/workers-types": "^4.20240909.0",
"@types/node": "20.8.3",
"@types/semver": "^7.5.1",
"@vitest/runner": "1.3.x - 1.5.x",
"@vitest/snapshot": "1.3.x - 1.5.x",
"@vitest/runner": "catalog:default",
"@vitest/snapshot": "catalog:default",
"capnp-ts": "^0.7.0",
"capnpc-ts": "^0.7.0",
"ts-dedent": "^2.2.0",
"typescript": "^5.5.2",
"undici": "^5.28.4",
"vitest": "1.5.0"
"vitest": "catalog:default"
},
"peerDependencies": {
"@vitest/runner": "1.3.x - 1.5.x",
"@vitest/snapshot": "1.3.x - 1.5.x",
"vitest": "1.3.x - 1.5.x"
"@vitest/runner": "2.0.x",
"@vitest/snapshot": "2.0.x",
"vitest": "2.0.x"
},
"volta": {
"extends": "../../package.json"
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest-pool-workers/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function createConfigPlugin(): Plugin<WorkersConfigPluginAPI> {
},
},
// Run after `vitest:project` plugin:
// https://github.com/vitest-dev/vitest/blob/8014614475afa880f4e583b166bb91dea5415cc6/packages/vitest/src/node/plugins/workspace.ts#L26
// https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vitest/src/node/plugins/workspace.ts#L34
config(config) {
config.resolve ??= {};
config.resolve.conditions ??= [];
Expand All @@ -140,7 +140,7 @@ function createConfigPlugin(): Plugin<WorkersConfigPluginAPI> {
ensureArrayIncludes(config.resolve.conditions, requiredConditions);

// Vitest sets this to an empty array if unset, so restore Vite defaults:
// https://github.com/vitest-dev/vitest/blob/v1.5.0/packages/vitest/src/node/plugins/index.ts#L77
// https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vitest/src/node/plugins/index.ts#L93
ensureArrayIncludes(config.resolve.mainFields, requiredMainFields);

// Apply `package.json` `browser` field remapping in SSR mode:
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest-pool-workers/src/pool/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export async function parseProjectOptions(
let workersPoolOptions = poolOptions?.workers ?? {};
try {
if (typeof workersPoolOptions === "function") {
// https://github.com/vitest-dev/vitest/blob/v1.5.0/packages/vitest/src/integrations/inject.ts
// https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vitest/src/integrations/inject.ts
const inject = <K extends keyof ProvidedContext>(
key: K
): ProvidedContext[K] => {
Expand Down
43 changes: 38 additions & 5 deletions packages/vitest-pool-workers/src/pool/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,45 @@ import type {
import type { Readable } from "node:stream";
import type { MessagePort } from "node:worker_threads";
import type {
ResolvedConfig,
RunnerRPC,
RuntimeRPC,
SerializedConfig,
WorkerContext,
} from "vitest";
import type { ProcessPool, Vitest, WorkspaceProject } from "vitest/node";

// https://github.com/vitest-dev/vitest/blob/v1.5.0/packages/vite-node/src/client.ts#L386
interface SerializedOptions {
main?: string;
durableObjectBindingDesignators?: Map<
string /* bound name */,
DurableObjectDesignator
>;
isolatedStorage?: boolean;
}

// https://github.com/vitest-dev/vitest/blob/v2.0.5/packages/vite-node/src/client.ts#L468
declare const __vite_ssr_import__: unknown;
assert(
typeof __vite_ssr_import__ === "undefined",
"Expected `@cloudflare/vitest-pool-workers` not to be transformed by Vite"
);

function structuredSerializableStringify(value: unknown): string {
// Vitest v2+ sends a sourcemap to it's runner, which we can't serialise currently
// Deleting it doesn't seem to cause any problems, and error stack traces etc...
// still seem to work
// TODO: Figure out how to serialise SourceMap instances
if (
value &&
typeof value === "object" &&
"r" in value &&
value.r &&
typeof value.r === "object" &&
"map" in value.r &&
value.r.map
) {
delete value.r.map;
}
return devalue.stringify(value, structuredSerializableReducers);
}
function structuredSerializableParse(value: string): unknown {
Expand Down Expand Up @@ -663,7 +687,7 @@ async function runTests(
mf: Miniflare,
workerName: string,
project: Project,
config: ResolvedConfig,
config: SerializedConfig,
files: string[],
invalidates: string[] = []
) {
Expand Down Expand Up @@ -740,7 +764,9 @@ async function runTests(
const rules = project.options.miniflare?.modulesRules;
const compiledRules = compileModuleRules(rules ?? []);

const localRpcFunctions = createMethodsRPC(project.project);
const localRpcFunctions = createMethodsRPC(project.project, {
cacheFs: false,
});
const patchedLocalRpcFunctions: RuntimeRPC = {
...localRpcFunctions,
async fetch(...args) {
Expand Down Expand Up @@ -953,6 +979,7 @@ export default function (ctx: Vitest): ProcessPool {
// serialisable. `getSerializableConfig()` may also return references to
// the same objects, so override it with a new object.
config.poolOptions = {
// @ts-expect-error Vitest provides no way to extend this type
threads: {
// Allow workers to be re-used by removing the isolation requirement
isolate: false,
Expand All @@ -974,7 +1001,7 @@ export default function (ctx: Vitest): ProcessPool {
// project, so we know whether to call out to the loopback service
// to push/pop the storage stack between tests.
isolatedStorage: project.options.isolatedStorage,
},
} satisfies SerializedOptions,
};

const mf = await getProjectMiniflare(ctx, project);
Expand Down Expand Up @@ -1046,6 +1073,12 @@ export default function (ctx: Vitest): ProcessPool {
// assert(testModule && thingModule);
// thingModule.importers.add(testModule);
},
async collectTests(_specs, _invalidates) {
// TODO: This is a new API introduced in Vitest v2+ which we should consider supporting at some point
throw new Error(
"The Cloudflare Workers Vitest integration does not support the `.collect()` or `vitest list` APIs"
);
},
async close() {
// `close()` will be called when shutting down Vitest or updating config
log.debug("Shutting down runtimes...");
Expand Down
14 changes: 13 additions & 1 deletion packages/vitest-pool-workers/src/pool/module-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,23 @@ async function viteResolve(
if (workerdBuiltinModules.has(id)) {
return `/${id}`;
}
if (id.startsWith("node:")) {
throw new Error("Not found");
}

id = `node:${id}`;
if (workerdBuiltinModules.has(id)) {
return `/${id}`;
}
throw new Error("Not found");

// If we get this far, we have something that:
// - looks like a built-in node module but wasn't imported with a `node:` prefix
// - and isn't provided by workerd natively
// In that case, _try_ and load the identifier with a `node:` prefix.
// This will potentially load one of the Node.js polyfills provided by `vitest-pool-workers`
// Note: User imports should never get here! This is only meant to cater for Vitest internals
// (Specifically, the "tinyrainbow" module imports `node:tty` as `tty`)
return id;
}
return resolved.id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import { VitestTestRunner } from "vitest/runners";
import workerdUnsafe from "workerd:unsafe";
import type { CancelReason, Suite, Test } from "@vitest/runner";
import type { ResolvedConfig, WorkerGlobalState, WorkerRPC } from "vitest";
import type { SerializedConfig, WorkerGlobalState, WorkerRPC } from "vitest";

// When `DEBUG` is `true`, runner operations will be logged and slowed down
// TODO(soon): remove this
Expand Down Expand Up @@ -114,7 +114,7 @@ export default class WorkersTestRunner extends VitestTestRunner {
readonly state: WorkerGlobalState;
readonly isolatedStorage: boolean;

constructor(config: ResolvedConfig) {
constructor(config: SerializedConfig) {
super(config);

// @ts-expect-error `this.workerState` has "private" access, how quaint :D
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest-pool-workers/src/worker/lib/node/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ export function statSync(_path: string) {

export const promises = {};

export default {};
export default { existsSync };
7 changes: 3 additions & 4 deletions packages/vitest-pool-workers/test/console.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ test.skipIf(process.platform === "win32")(
});
const result = vitestDev();
await waitFor(() => {
expect(result.stdout).toMatch("stdout | index.test.ts:2:9\nglobal");
expect(result.stdout).toMatch("stdout | index.test.ts:4:10\ndescribe");
expect(result.stdout).toMatch("stdout | index.test.ts\nglobal\ndescribe");
expect(result.stdout).toMatch(
"stdout | index.test.ts > thing > does something\ntest"
);
Expand All @@ -42,10 +41,10 @@ test.skipIf(process.platform === "win32")(
`,
});
await waitFor(() => {
expect(result.stdout).toMatch("stdout | index.test.ts:2:9\nnew global");
expect(result.stdout).toMatch(
"stdout | index.test.ts:4:10\nnew describe"
"stdout | index.test.ts\nnew global\nnew describe"
);

expect(result.stdout).toMatch(
"stdout | index.test.ts > new thing > does something else\nnew test"
);
Expand Down
5 changes: 4 additions & 1 deletion packages/vitest-pool-workers/test/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export default async function ({ provide }: GlobalSetupContext) {
},
};
await fs.writeFile(packageJsonPath, JSON.stringify(packageJson));
childProcess.execSync("pnpm install", { cwd: tmpPath, stdio: "inherit" });
childProcess.execSync("pnpm install", {
cwd: tmpPath,
stdio: "inherit",
});

provide("tmpPoolInstallationPath", tmpPath);

Expand Down
2 changes: 1 addition & 1 deletion packages/workers-shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"rimraf": "^6.0.1",
"toucan-js": "^3.3.1",
"typescript": "^5.5.4",
"vitest": "1.5.0"
"vitest": "catalog:default"
},
"engines": {
"node": ">=16.7.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/workers.new/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@types/node": "20.8.3",
"miniflare": "workspace:*",
"typescript": "^5.5.2",
"vitest": "1.5.0",
"vitest": "catalog:default",
"wrangler": "workspace:*"
},
"volta": {
Expand Down
Loading

0 comments on commit f3c0400

Please sign in to comment.