-
Notifications
You must be signed in to change notification settings - Fork 5
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
chore: integration tests setup #116
Conversation
a3f1a35
to
4e211c4
Compare
4e211c4
to
cd864ab
Compare
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.
test/fixtures/worker-fixture.js
Outdated
* The IP address of the test worker. | ||
* @type {string} | ||
*/ | ||
let ip = 'localhost' |
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.
suggestion Maybe let's put these all in one object (the return value of await runWranglerDev()
), to make it clear that they all get set together. I had to read through the whole file a couple of times to understand what was setting these.
Along those lines, let's separate the requested port (which is a constant) from the actual port, and stop providing default values for ip
and port
. Right now, they're lying until Wrangler boots. If Wrangler's not up yet, we should signal that rather than provide fake values.
One way to set this up (which might be overkill) would be to have mochaGlobalSetup()
store the promise from runWranglerDev()
rather than await
it itself, and have getWorkerInfo()
be async. Then the tests will start booting Wrangler and not wait for it to be set up until the values are actually requested. (We don't really have a performance issue here, though, so the choice should be down to which one communicates what's happening the clearest.)
/** @type {(value: { ip: string port: number }) => void} */ | ||
let resolveReadyPromise | ||
/** @type {(reason: unknown) => void} */ | ||
let rejectReadyPromise |
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.
suggestion: Let's avoid all these let
s and put the stuff below inside the new Promise()
. It'll have access to resolve
and reject
, and we don't need to track settledReadyPromise
at all: resolve
ing or reject
ing after it's settled is a noop.
@@ -18,7 +18,7 @@ | |||
"build": "esbuild --bundle src/index.js --format=esm --sourcemap --minify --outfile=dist/worker.mjs && npm run build:tsc", | |||
"build:debug": "esbuild --bundle src/index.js --format=esm --outfile=dist/worker.mjs", | |||
"build:tsc": "tsc --build", | |||
"test:miniflare": "npm run build:debug && mocha --experimental-vm-modules test/index.spec.js", | |||
"test:miniflare": "npm run build:debug && mocha --experimental-vm-modules --recursive test/miniflare/**/*.spec.js", |
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.
praise: I like this move!
Most of the suggestions were implemented. The open ones won't be addressed for now.
Integration Test Setup for Cloudflare Local Worker
In this PR, I have implemented an integration test setup to verify the functionality of our Cloudflare Worker. This setup involves running the worker locally with all the bindings enabled in
wrangler.toml
(integration
environment), and performing a series of HTTP requests to ensure the worker behaves as expected under various conditions.By running the worker locally, we avoid the complexities and overhead associated with handling authentication for deploying a remote worker via CI. This makes the testing process more straightforward.
Key Components
Worker Fixture:
setupWorker
: Deploys the worker and starts the local Wrangler dev server.teardownWorker
: Stops the local Wrangler dev server.getWorkerState
: Provides the current state of the worker, including IP, port, and output.Integration Test Folder:
Configuration
wrangler.toml
file. This includes settings such as rate limiting, environment variables, and other configurations necessary for the worker to function correctly. Make sure you are editing theintegration
environment config - which is the default environment used by the integration test setup.