fix: normalize workbench tests#292
Conversation
🦋 Changeset detectedLatest commit: b6d2d7c The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔧 Build Fix:
The StructuredErrorSchema import is declared but never used in the file, causing a TypeScript compilation error TS6133.
View Details
📝 Patch Details
diff --git a/packages/world-vercel/src/steps.ts b/packages/world-vercel/src/steps.ts
index 0cb4959..8323344 100644
--- a/packages/world-vercel/src/steps.ts
+++ b/packages/world-vercel/src/steps.ts
@@ -6,7 +6,6 @@ import {
PaginatedResponseSchema,
type Step,
StepSchema,
- StructuredErrorSchema,
type UpdateStepRequest,
} from '@workflow/world';
import { z } from 'zod';
Analysis
Unused import causes TypeScript compilation failure
What fails: TypeScript compiler fails on packages/world-vercel/src/steps.ts due to unused import StructuredErrorSchema
How to reproduce:
pnpm turbo run build --filter=@workflow/world-vercelResult:
src/steps.ts(9,3): error TS6133: 'StructuredErrorSchema' is declared but its value is never read.
ELIFECYCLE Command failed with exit code 2.
There was a problem hiding this comment.
Additional Suggestion:
The test uses a .toBeOneOf() matcher that doesn't exist in vitest, causing test failures with "expect(...).toBeOneOf is not a function".
View Details
📝 Patch Details
diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts
index 5076b61..7e5103d 100644
--- a/packages/core/e2e/e2e.test.ts
+++ b/packages/core/e2e/e2e.test.ts
@@ -91,11 +91,11 @@ describe('e2e', () => {
});
// In local vs. vercel backends, the workflow name is different, so we check for either,
// since this test runs against both. Also different workbenches have different directory structures.
- expect(json.workflowName).toBeOneOf([
+ expect([
`workflow//example/${workflow.workflowFile}//${workflow.workflowFn}`,
`workflow//${workflow.workflowFile}//${workflow.workflowFn}`,
`workflow//src/${workflow.workflowFile}//${workflow.workflowFn}`,
- ]);
+ ]).toContain(json.workflowName);
});
test('promiseAllWorkflow', { timeout: 60_000 }, async () => {
@@ -158,7 +158,7 @@ describe('e2e', () => {
// NOTE: For Nitro apps (Vite, Hono, etc.) in dev mode, status 404 does some
// unexpected stuff and could return a Vite SPA fallback or can cause a Hono route to hang.
// This is because Nitro passes the 404 requests to the dev server to handle.
- expect(res.status).toBeOneOf([404, 422]);
+ expect([404, 422]).toContain(res.status);
body = await res.json();
expect(body).toBeNull();
Analysis
Undefined .toBeOneOf() matcher in e2e tests
What fails: Tests in packages/core/e2e/e2e.test.ts use .toBeOneOf() matcher on lines 94 and 161, which is not a standard vitest matcher and is not defined anywhere in the codebase, causing test execution failures with "expect(...).toBeOneOf is not a function"
How to reproduce:
# Run the e2e tests
pnpm test:e2eThe tests will fail when encountering the .toBeOneOf() calls because vitest does not recognize this matcher.
Result: Runtime error: TypeError: expect(...).toBeOneOf is not a function
Expected: Tests should use standard vitest matchers. The .toBeOneOf() usage checks if a value belongs to an array of possible values. The standard vitest approach is to use .toContain() on an array, reversing the assertion operands.
Fix applied: Replaced both instances of .toBeOneOf() with .toContain():
- Line 94: Changed
expect(json.workflowName).toBeOneOf([...])toexpect([...]).toContain(json.workflowName) - Line 161: Changed
expect(res.status).toBeOneOf([404, 422])toexpect([404, 422]).toContain(res.status)
This is the standard vitest pattern for checking membership in an array, as documented in the vitest expect API.
| } | ||
| server.ws.send({ | ||
| type: 'full-reload', | ||
| path: '*', |
There was a problem hiding this comment.
probably fine for now and we can revisit
| @@ -163,8 +163,8 @@ app.post('/api/hook', async ({ req }) => { | |||
| } catch (error) { | |||
| console.log('error during getHookByToken', error); | |||
| // TODO: `WorkflowAPIError` is not exported, so for now | |||
There was a problem hiding this comment.
cc @TooTallNate we should have a HookNotFound error thrown from the world?
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
left some comments but let's merge it into the upstream PR and feel free to address here or there :)
3 participants
No description provided.