Use vitest#expect from the local context#12385
Conversation
|
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| test("should not retry a step if a NonRetryableError (with a generic error message) is thrown", async ({ | ||
| expect, | ||
| }) => { | ||
| const name = randomUUID(); | ||
| await fetchJson( | ||
| `http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=false&errorMessage=generic_error_message"` |
There was a problem hiding this comment.
🔴 Stray double-quote in fetchJson URL breaks NonRetryableError retry test
The createDemo3 URL used in the NonRetryableError test includes an extra " at the end of the query string.
Actual behavior: the request is made to a different URL than intended (with an extra " character in the errorMessage parameter), so the fixture may not exercise the intended code path and can cause flaky/failed assertions about retry behavior.
Expected behavior: the URL should end after errorMessage=generic_error_message with no trailing quote.
Code reference
await fetchJson(
`http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=false&errorMessage=generic_error_message"`
);(Refers to lines 192-198)
Recommendation: Remove the trailing " from the template literal so the query string is ...&errorMessage=generic_error_message.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
That's a good point... about existing code untouched in this PR.
I'll follow up after the PR gets merged.
There was a problem hiding this comment.
🔴 Using expect(() => promise).rejects prevents rejection assertion from working
The test attempts to assert that a fetch() Promise rejects, but wraps the promise in a function and then uses .rejects.
Actual: .rejects expects a Promise/thenable. Passing a function means Vitest will not treat it as a promise rejection assertion, so this either fails immediately (type mismatch at runtime) or asserts the wrong thing.
Expected: pass the Promise directly: await expect(SELF.fetch(...)).rejects....
Click to expand
Code:
await expect(() =>
SELF.fetch("http://example.com/container/hello")
).rejects.toThrow();Correct pattern:
await expect(SELF.fetch("http://example.com/container/hello")).rejects.toThrow();(Refers to lines 12-15)
Recommendation: Pass the SELF.fetch(...) promise directly to expect(...) when using .rejects.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Again a good finding in an existing and untouched test, I'll follow up after this PR.
| ) { | ||
| let match = await cache.match("http://0.0.0.0/test"); | ||
| expect(match).toBeUndefined(); | ||
| // Note: we cannot use expect here since it's not in the test context |
There was a problem hiding this comment.
Alternatively we could have passed the expect as a param to the testNoOpCache function...
There was a problem hiding this comment.
Good point!
I'll handle as part of the follow up with the 2 Devin findings.
3 participants
Part of #12346
Part of a series (#12347, #12356, #12373) handling simple refactors, one package at a time to keep the review simpler.
This PR updates the
fixturesfolder.Bceause it was not linted before, eslint configs have been added and
@cloudflare/eslint-config-sharedadded to package.json.The code changes are courtesy of OpenCode/Opus
Summary of Changes to fixtures/
Migrated fixture test files to use context-based expect from vitest for concurrency safety, and enabled the workers-sdk/no-vitest-import-expect ESLint rule.
Test File Migrations (~108 files)
Converted test files from:
To:
ESLint Configuration Added (57 projects)
Added eslint.config.mjs and @cloudflare/eslint-config-shared dependency to:
Complex Files with eslint-disable (5 files)
These files use patterns incompatible with context-based expect (e.g., .each, helper functions):
Other Changes
A picture of a cute animal (not mandatory, but encouraged)