Add review comment suggestions

Author: pombosilva

fixtures/vitest-pool-workers-examples/workflows/src/index.ts

@@ -54,30 +54,34 @@ export class TestLongWorkflow extends WorkflowEntrypoint<Env, Params> {
 
 export default {
 	async fetch(request: Request, env: Env) {
-		const maybeId = new URL(request.url).searchParams.get("id");
+		const url = new URL(request.url);
+		const maybeId = url.searchParams.get("id");
 		if (maybeId !== null) {
 			const instance = await env.TEST_WORKFLOW.get(maybeId);
 
 			return Response.json(await instance.status());
 		}
 
-		if (request.url.endsWith("/long-workflow")) {
+		if (url.pathname === "/long-workflow") {
 			const workflow = await env.TEST_LONG_WORKFLOW.create();
-			return new Response(JSON.stringify({ id: workflow.id }));
+			return Response.json({
+				id: workflow.id,
+				details: await workflow.status(),
+			});
 		}
 
-		if (request.url.endsWith("/long-workflow-batch")) {
+		if (url.pathname === "/long-workflow-batch") {
 			const workflows = await env.TEST_LONG_WORKFLOW.createBatch([
 				{},
 				{ id: "321" },
 				{},
 			]);
 			const ids = workflows.map((workflow) => workflow.id);
-			return new Response(JSON.stringify({ ids: ids }));
+			return Response.json({ ids: ids });
 		}
 
 		const workflow = await env.TEST_WORKFLOW.create();
 
-		return new Response(JSON.stringify({ id: workflow.id }));
+		return Response.json({ id: workflow.id });
 	},
 };

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

@@ -1,18 +1,7 @@
-import {
-	env,
-	introspectWorkflow,
-	introspectWorkflowInstance,
-	SELF,
-} from "cloudflare:test";
-import {
-	afterEach,
-	assert,
-	beforeEach,
-	describe,
-	expect,
-	it,
-	vi,
-} from "vitest";
+import { env, introspectWorkflow, SELF } from "cloudflare:test";
+import { describe, expect, it, vi } from "vitest";
+
+const STATUS_COMPLETE = "complete";
 
 describe("Test Workflow", () => {
 	it("should be able to trigger a workflow", async () => {
@@ -30,78 +19,78 @@ describe("Test Workflow", () => {
 			const res = await SELF.fetch(`https://mock-worker.local?id=${json.id}`);
 
 			const statusJson = await res.json<{ status: string }>();
-			console.log(statusJson);
-			return statusJson.status === "complete";
+			expect(statusJson.status).toBe(STATUS_COMPLETE);
+			return true;
 		}, 1000);
 	});
 
 	it("workflow should reach the end and be successful with introspector", async () => {
-		const introspector = await introspectWorkflow(env.TEST_WORKFLOW);
+		// CONFIG with `using` to ensure cleanup:
+		await using introspector = await introspectWorkflow(env.TEST_WORKFLOW);
 
 		await SELF.fetch("https://mock-worker.local");
 
 		const instances = introspector.get();
-		assert(instances.length === 1);
+		expect(instances.length).toBe(1);
 
+		// ASSERTIONS:
 		const instance = instances[0];
-		await instance.waitForStatus("complete");
-		await instance.cleanUp();
+		await instance.waitForStatus(STATUS_COMPLETE);
+
+		// CLEANUP: assured by Symbol.asyncDispose
 	});
 });
 
 describe("Test long Workflow", () => {
 	const STEP_NAME = "my step";
 	const mockResult = "mocked result";
 
-	let introspector: Awaited<ReturnType<typeof introspectWorkflow>>;
-	let instances: Awaited<ReturnType<typeof introspectWorkflowInstance>>[];
-
-	beforeEach(async () => {
-		// CONFIG:
-		introspector = await introspectWorkflow(env.TEST_LONG_WORKFLOW);
+	it("workflow should be able to introspect and reach the end and be successful", async () => {
+		// CONFIG with `using` to ensure cleanup:
+		await using introspector = await introspectWorkflow(env.TEST_LONG_WORKFLOW);
 		introspector.modifyAll(async (m) => {
 			await m.disableSleeps();
 			await m.mockStepResult({ name: STEP_NAME }, mockResult);
 		});
-	});
-
-	afterEach(async () => {
-		// CLEANUP:
-		// Instance introspectors should be clean to prevent persisted state across tests
-		for (const instance of instances) {
-			instance.cleanUp();
-		}
-
-		// Workflow introspector should be cleaned at the end of/after each test
-		introspector.cleanUp();
-	});
 
-	it("workflow should be able to introspect and reach the end and be successful", async () => {
 		await SELF.fetch("https://mock-worker.local/long-workflow");
 
-		instances = introspector.get();
-		assert(instances.length === 1);
+		const instances = introspector.get();
+		expect(instances.length).toBe(1);
 
 		// ASSERTIONS:
 		const instance = instances[0];
 		expect(await instance.waitForStepResult({ name: STEP_NAME })).toEqual(
 			mockResult
 		);
-		await instance.waitForStatus("complete");
+		await instance.waitForStatus(STATUS_COMPLETE);
+
+		// CLEANUP: done by Symbol.asyncDispose
 	});
 
 	it("workflow batch should be able to introspect and reach the end and be successful", async () => {
+		let introspector = await introspectWorkflow(env.TEST_LONG_WORKFLOW);
+		introspector.modifyAll(async (m) => {
+			await m.disableSleeps();
+			await m.mockStepResult({ name: STEP_NAME }, mockResult);
+		});
+
 		await SELF.fetch("https://mock-worker.local/long-workflow-batch");
 
-		instances = introspector.get();
-		assert(instances.length === 3);
+		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("complete");
+			await instance.waitForStatus(STATUS_COMPLETE);
 		}
+
+		// 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
+		introspector.cleanUp();
 	});
 });