Change cleanup to dispose. Apply suggestions.

Author: pombosilva

.changeset/nice-carrots-sink.md

@@ -0,0 +1,5 @@
+---
+"miniflare": patch
+---
+
+Include workflow bining name in workflow plugin.

fixtures/vitest-pool-workers-examples/workflows/test/integration.test.ts

@@ -5,12 +5,11 @@ const STATUS_COMPLETE = "complete";
 const STEP_NAME = "AI content scan";
 const mockResult = { violationScore: 0 };
 
+// This example implicitly disposes the Workflow instance
 it("workflow should be able to reach the end and be successful", async () => {
-	// This example shows how to implicitly cleanUp Workflow instances
-
-	// CONFIG with `using` to ensure Workflow instances cleanup:
+	// CONFIG with `await using` to ensure Workflow instances cleanup:
 	await using introspector = await introspectWorkflow(env.MODERATOR);
-	introspector.modifyAll(async (m) => {
+	await introspector.modifyAll(async (m) => {
 		await m.disableSleeps();
 		await m.mockStepResult({ name: STEP_NAME }, mockResult);
 	});
@@ -25,36 +24,39 @@ it("workflow should be able to reach the end and be successful", async () => {
 	expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
 		mockResult
 	);
-	await instance.waitForStatus(STATUS_COMPLETE);
+	await expect(instance.waitForStatus(STATUS_COMPLETE)).resolves.not.toThrow();
 
-	// CLEANUP: ensured by `using`
+	// DISPOSE: ensured by `await using`
 });
 
+// This example explicitly disposes the Workflow instances
 it("workflow batch should be able to reach the end and be successful", async () => {
-	// This example shows how to explicitly cleanUp Workflow instances
-
 	// CONFIG:
 	let introspector = await introspectWorkflow(env.MODERATOR);
-	introspector.modifyAll(async (m) => {
-		await m.disableSleeps();
-		await m.mockStepResult({ name: STEP_NAME }, mockResult);
-	});
-
-	await SELF.fetch(`https://mock-worker.local/moderate-batch`);
-
-	const instances = introspector.get();
-	expect(instances.length).toBe(3);
-
-	// ASSERTIONS:
-	for (const instance of instances) {
-		expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
-			mockResult
-		);
-		await instance.waitForStatus(STATUS_COMPLETE);
+	try {
+		await introspector.modifyAll(async (m) => {
+			await m.disableSleeps();
+			await m.mockStepResult({ name: STEP_NAME }, mockResult);
+		});
+
+		await SELF.fetch(`https://mock-worker.local/moderate-batch`);
+
+		const instances = introspector.get();
+		expect(instances.length).toBe(3);
+
+		// ASSERTIONS:
+		for (const instance of instances) {
+			expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
+				mockResult
+			);
+			await expect(
+				instance.waitForStatus(STATUS_COMPLETE)
+			).resolves.not.toThrow();
+		}
+	} finally {
+		// DISPOSE:
+		// Workflow introspector should be disposed the end of each test, if no `await using` dyntax is used
+		// Also disposes all intercepted instances
+		await introspector.dispose();
 	}
-
-	// CLEANUP:
-	// Workflow introspector should be cleaned at the end of/after each test, if no `using` keyword is used for the introspector
-	// Cleans up all intercepted instances
-	await introspector.cleanUp();
 });

fixtures/vitest-pool-workers-examples/workflows/test/unit.test.ts

@@ -6,9 +6,11 @@ const STATUS_COMPLETE = "complete";
 const STATUS_ERROR = "errored";
 const STEP_NAME = "AI content scan";
 
+// This example implicitly disposes the Workflow instance
 it("should mock a non-violation score and complete", async () => {
 	const mockResult = { violationScore: 0 };
 
+	// CONFIG with `await using` to ensure Workflow instances cleanup:
 	await using instance = await introspectWorkflowInstance(
 		env.MODERATOR,
 		INSTANCE_ID
@@ -22,45 +24,58 @@ it("should mock a non-violation score and complete", async () => {
 		id: INSTANCE_ID,
 	});
 
+	// ASSERTIONS:
 	expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
 		mockResult
 	);
 	expect(
 		await instance.waitForStepResult({ name: "auto approve content" })
 	).toEqual({ status: "auto_approved" });
 
-	await instance.waitForStatus(STATUS_COMPLETE);
+	await expect(instance.waitForStatus(STATUS_COMPLETE)).resolves.not.toThrow();
+
+	// DISPOSE: ensured by `await using`
 });
 
+// This example explicitly disposes the Workflow instance
 it("should mock the violation score calculation to fail 2 times and then complete", async () => {
 	const mockResult = { violationScore: 0 };
 
-	await using instance = await introspectWorkflowInstance(
-		env.MODERATOR,
-		INSTANCE_ID
-	);
-	await instance.modify(async (m) => {
-		await m.disableSleeps();
-		await m.mockStepError(
-			{ name: STEP_NAME },
-			new Error("Something went wrong!"),
-			2
-		);
-		await m.mockStepResult({ name: STEP_NAME }, mockResult);
-	});
-
-	await env.MODERATOR.create({
-		id: INSTANCE_ID,
-	});
+	// CONFIG:
+	const instance = await introspectWorkflowInstance(env.MODERATOR, INSTANCE_ID);
+
+	try {
+		await instance.modify(async (m) => {
+			await m.disableSleeps();
+			await m.mockStepError(
+				{ name: STEP_NAME },
+				new Error("Something went wrong!"),
+				2
+			);
+			await m.mockStepResult({ name: STEP_NAME }, mockResult);
+		});
 
-	expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
-		mockResult
-	);
-	expect(
-		await instance.waitForStepResult({ name: "auto approve content" })
-	).toEqual({ status: "auto_approved" });
+		await env.MODERATOR.create({
+			id: INSTANCE_ID,
+		});
 
-	await instance.waitForStatus(STATUS_COMPLETE);
+		// ASSERTIONS:
+		expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
+			mockResult
+		);
+		expect(
+			await instance.waitForStepResult({ name: "auto approve content" })
+		).toEqual({ status: "auto_approved" });
+
+		await expect(
+			instance.waitForStatus(STATUS_COMPLETE)
+		).resolves.not.toThrow();
+	} finally {
+		// DISPOSE:
+		// Workflow introspector should be disposed the end of each test, if no `await using` dyntax is used
+		// Also disposes all intercepted instances
+		await instance.dispose();
+	}
 });
 
 it("should mock a violation score and complete", async () => {
@@ -86,7 +101,7 @@ it("should mock a violation score and complete", async () => {
 		await instance.waitForStepResult({ name: "auto reject content" })
 	).toEqual({ status: "auto_rejected" });
 
-	await instance.waitForStatus(STATUS_COMPLETE);
+	await expect(instance.waitForStatus(STATUS_COMPLETE)).resolves.not.toThrow();
 });
 
 it("should be reviewed, accepted and complete", async () => {
@@ -116,7 +131,7 @@ it("should be reviewed, accepted and complete", async () => {
 		await instance.waitForStepResult({ name: "apply moderator decision" })
 	).toEqual({ status: "moderated", decision: "approve" });
 
-	await instance.waitForStatus(STATUS_COMPLETE);
+	await expect(instance.waitForStatus(STATUS_COMPLETE)).resolves.not.toThrow();
 });
 
 it("should force human review to timeout and error", async () => {
@@ -140,5 +155,5 @@ it("should force human review to timeout and error", async () => {
 		mockResult
 	);
 
-	await instance.waitForStatus(STATUS_ERROR);
+	await expect(instance.waitForStatus(STATUS_ERROR)).resolves.not.toThrow();
 });

packages/vitest-pool-workers/src/pool/loopback.ts

@@ -360,8 +360,7 @@ function checkAllStorageOperationsResolved(
 					"",
 					separator,
 					`Workflows are being created in ${source}.`,
-					"Isolated storage is enabled, requiring all created Workflow instances to be cleaned/disposed at the end of each test.",
-					"Alternatively, if isolated storage is not required and Workflow instance state needs to be preserved across tests, `isolatedStorage` must be set to false in vitest.config.ts.",
+					"Even with isolated storage, Workflows are required to be manually disposed at the end of each test.",
 					"See https://developers.cloudflare.com/workers/testing/vitest-integration/test-apis/ for more details.",
 					"",
 				].join("\n")

packages/vitest-pool-workers/src/worker/workflows.ts

@@ -20,9 +20,14 @@ export interface WorkflowInstanceIntrospector {
 
 	waitForStatus(status: string): Promise<void>;
 
-	cleanUp(): Promise<void>;
+	dispose(): Promise<void>;
 }
 
+// Note(osilva): `introspectWorkflowInstance()` doesn’t need to be async, but we keep it that way
+// to avoid potential breaking changes later and to stay consistent with `introspectWorkflow`.
+
+// In the "cloudflare:test" module, the exposed type is `Workflow`. Here we use `WorkflowBinding`
+// (which implements `Workflow`) to access unsafe functions.
 export async function introspectWorkflowInstance(
 	workflow: WorkflowBinding,
 	instanceId: string
@@ -85,24 +90,29 @@ class WorkflowInstanceIntrospectorHandle
 		await this.#workflow.unsafeWaitForStatus(this.#instanceId, status);
 	}
 
-	async cleanUp(): Promise<void> {
-		await this.#workflow.unsafeAbort(this.#instanceId, "Instance clean up");
+	async dispose(): Promise<void> {
+		await this.#workflow.unsafeAbort(this.#instanceId, "Instance dispose");
 	}
 
 	async [Symbol.asyncDispose](): Promise<void> {
-		await this.cleanUp();
+		await this.dispose();
 	}
 }
 
 // See public facing `cloudflare:test` types for docs
 export interface WorkflowIntrospector {
-	modifyAll(fn: ModifierCallback): void;
+	modifyAll(fn: ModifierCallback): Promise<void>;
 
 	get(): WorkflowInstanceIntrospector[];
 
-	cleanUp(): Promise<void>;
+	dispose(): Promise<void>;
 }
 
+// Note(osilva): `introspectWorkflow` could be sync with some changes, but we keep it async
+// to avoid potential breaking changes later.
+
+// In the "cloudflare:test" module, the exposed type is `Workflow`. Here we use `WorkflowBinding`
+// (which implements `Workflow`) to access unsafe functions.
 export async function introspectWorkflow(
 	workflow: WorkflowBinding
 ): Promise<WorkflowIntrospectorHandle> {
@@ -189,7 +199,7 @@ export async function introspectWorkflow(
 		};
 	};
 
-	const cleanup = () => {
+	const dispose = () => {
 		internalEnv[bindingName] = internalOriginalWorkflow;
 		env[bindingName] = externalOriginalWorkflow;
 	};
@@ -209,47 +219,47 @@ export async function introspectWorkflow(
 		workflow,
 		modifierCallbacks,
 		instanceIntrospectors,
-		cleanup
+		dispose
 	);
 }
 
 class WorkflowIntrospectorHandle implements WorkflowIntrospector {
 	workflow: WorkflowBinding;
 	#modifierCallbacks: ModifierCallback[];
 	#instanceIntrospectors: WorkflowInstanceIntrospector[];
-	#cleanupCallback: () => void;
+	#disposeCallback: () => void;
 
 	constructor(
 		workflow: WorkflowBinding,
 		modifierCallbacks: ModifierCallback[],
 		instanceIntrospectors: WorkflowInstanceIntrospector[],
-		cleanupCallback: () => void
+		disposeCallback: () => void
 	) {
 		this.workflow = workflow;
 		this.#modifierCallbacks = modifierCallbacks;
 		this.#instanceIntrospectors = instanceIntrospectors;
-		this.#cleanupCallback = cleanupCallback;
+		this.#disposeCallback = disposeCallback;
 	}
 
-	modifyAll(fn: ModifierCallback): void {
+	async modifyAll(fn: ModifierCallback): Promise<void> {
 		this.#modifierCallbacks.push(fn);
 	}
 
 	get(): WorkflowInstanceIntrospector[] {
 		return this.#instanceIntrospectors;
 	}
 
-	async cleanUp(): Promise<void> {
-		// also cleans all instance introspectors
+	async dispose(): Promise<void> {
+		// also disposes all instance introspectors
 		await Promise.all(
-			this.#instanceIntrospectors.map((introspector) => introspector.cleanUp())
+			this.#instanceIntrospectors.map((introspector) => introspector.dispose())
 		);
 		this.#modifierCallbacks = [];
 		this.#instanceIntrospectors = [];
-		this.#cleanupCallback();
+		this.#disposeCallback();
 	}
 
 	async [Symbol.asyncDispose](): Promise<void> {
-		await this.cleanUp();
+		await this.dispose();
 	}
 }