-
-
Notifications
You must be signed in to change notification settings - Fork 12
tests, scripts: cleanup tests and dev environment things #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
cabb03d
to
7f020e2
Compare
@flakey5 -- do you need a review now or is this still WIP? |
Still a wip |
74763d5
to
2d28a66
Compare
a213355
to
47f78f7
Compare
Signed-off-by: flakey5 <[email protected]>
47f78f7
to
91d7f42
Compare
Gonna go ahead and mark this for review now. This unfortunately doesn't resolve the annoyances with running the worker locally though, there's still some things that I need to play around with to see if I can get it working in an ideal way. |
CACHING: boolean; | ||
|
||
LOG_ERRORS?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget what the actual number is but there is a limit on the about of bindings & vars a worker can have, so might be beneficial to have these in a FLAGS
object or something
@@ -0,0 +1,137 @@ | |||
import { env, fetchMock, createExecutionContext } from 'cloudflare:test'; | |||
import { test, beforeAll, afterEach, expect } from 'vitest'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use node:test
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd be good yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes (this is how the tests currently work) but vitest is the preferred way to write tests for workers. With the vitest integration the tests run inside of workerd
instead of node + better typescript support (although gap is closing with Node's type stripping I think?). Iirc with vitest we also get better stack traces when things error since we're not testing a built & minified version of the worker.
This is an attempt to make the tests and local dev environment a lot nicer to work with.
Some of the main problems with them right now are:
This pr:
dist-prod
bucket (calleddev-bucket
)tests/e2e/test-data/R2_BUCKET
for e2e testsnpm run dev
- starts a locally running version of the worker with a populated R2 bucketPopulated from thedev-bucket
directoryWhat's TODO:
Some current annoyances:
worker.js
file. I do have some ideas, but not entirely sure if they're gonna work.Ideally we would be able to use Workers' vitest integration, but it's not really possible since we're doing things like mocking an S3 api in our e2e tests. Vitest patches a lot of modules and globals making things likeWorkaround for this was found, so we can use the Vitest integration nownode:http
unavailable.