workers-sdk icon indicating copy to clipboard operation
workers-sdk copied to clipboard

đźš§ wip: `worker.scheduled()` for `unstable_dev`

Open cameron-robey opened this issue 3 years ago • 6 comments

Initial implementation of worker.scheduled() for unstable_dev. (https://github.com/cloudflare/wrangler2/issues/1804)

const worker = await unstable_dev("index.js");

const resp1 = await worker.fetch("/path");

// We now export a way of testing scheduled events
const resp2 = await worker.scheduled();
// or with a custom cron
const resp3 = await worker.scheduled("* * * * *");

There are a few things to think about here:

  1. This relies on the middleware for testing scheduled events (https://github.com/cloudflare/wrangler2/pull/1815) - and hence actually requires us to start unstable_dev with testScheduled: true We have a couple of options here as it isn't ideal to make people pass through testScheduled:
  • Option 1: Only enable worker.scheduled() if testScheduled is set to true.
  • Option 2: Enable testScheduled by default. This will always wrap the worker with middleware unless disabled. This would override the route /__scheduled which is very unlikely to have been used by people as default behaviour, but we would still expose the option for people to disable it, (and when disabled we would not allow worker.scheduled() to work.
  • Option 3: We bundle the worker twice, once with the scheduled middleware disabled, and once with it enabled and then start two dev servers and direct traffic to each of the two depending on if people call worker.fetch() or `worker.scheduled().
  • Option 4: Similar to option 3, just we only start up the second server on the first worker.scheduled() request. We could additionally only start up the non-scheduled event middleware (regular) server on the first worker.fetch(). Meaning if only one of .scheduled() or .fetch() is called we only start up one server and shouldn't(?) lead to any increased loading times overall. This would only be true in testing environments and outside of that would behave as it does now.

For Option 4:

// Initial `unstable_dev` call doesn't start up any servers
const worker = await unstable_dev("index.js");

// First call to `worker.fetch()` bundles and starts up a server
const resp1 = await worker.fetch("/path");

// Subsequent calls to `worker.fetch()` use the existing server 
const resp2 = await worker.fetch("/path");

// First call to `worker.scheduled()` starts up a second server with the scheduled testing middleware bundled
const resp3 = await worker.scheduled();

// Subsequent calls to `worker.scheduled()` use the existing server with middleware bundled
const resp4 = await worker.scheduled("* * * * *");

// shuts down all servers
await worker.stop();
  1. It is very hard to capture any response out of the scheduled events as scheduled events can't return a response. It is probably worth having a dig into how people are using scheduled events to figure out how might be best to capture something testable out of them.

cameron-robey avatar Sep 15 '22 14:09 cameron-robey

⚠️ No Changeset found

Latest commit: a0d12b6c3263e530c31ebcb6f2d60739b1c3aaff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 15 '22 14:09 changeset-bot[bot]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/3061467426/npm-package-wrangler-1862

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1862/npm-package-wrangler-1862

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/3061467426/npm-package-wrangler-1862 dev path/to/script.js

github-actions[bot] avatar Sep 15 '22 14:09 github-actions[bot]

Codecov Report

Merging #1862 (a0d12b6) into main (d8fe95d) will decrease coverage by 0.00%. The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1862      +/-   ##
==========================================
- Coverage   76.89%   76.88%   -0.01%     
==========================================
  Files         104      104              
  Lines        7341     7347       +6     
  Branches     1928     1930       +2     
==========================================
+ Hits         5645     5649       +4     
- Misses       1696     1698       +2     
Impacted Files Coverage Δ
packages/wrangler/src/api/dev.ts 67.24% <50.00%> (-1.99%) :arrow_down:
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts 100.00% <0.00%> (+5.26%) :arrow_up:

codecov[bot] avatar Sep 15 '22 15:09 codecov[bot]

I didn't realise scheduled events don't return anything - is there a value in calling them from a test runner then? What should we assert/expect on?

rozenmd avatar Sep 16 '22 12:09 rozenmd

I think it would be nice to have some way of capturing logging from the started dev process, that would give something to assert / expect on. Currently we can assert/expect on errors

cameron-robey avatar Sep 16 '22 13:09 cameron-robey

Here is an example test that you could do:

it("should be false then true", async () => {
  const scriptContent = `
  let test = false;
  
  export default {
    fetch(request, env, ctx) {
      return new Response(test);
    },
    scheduled(event, env, ctx) {
      test = true;
    },
  };
`;
  fs.writeFileSync("index.js", scriptContent);

  worker = await unstable_dev(
    "index.js",
    { testScheduled: true },
    { disableExperimentalWarning: true }
  );
  const resp1 = await worker.fetch("/test");
  const resp2 = await worker.scheduled();
  const resp3 = await worker.fetch("/test");

  if (resp1) expect(await resp1.json()).toBe(false);
  if (resp2) expect(await resp2.text()).toBe("Ran scheduled event");
  if (resp3) expect(await resp3.json()).toBe(true);

  await worker.stop();
});

If people are updating KV or something like that we could check for that pre and post operation.

Potentially we could even add worker.getKV() or other helpers - I guess this all depends on what the testing story ends up looking like... These could be implemented quite easily using middleware

cameron-robey avatar Sep 16 '22 14:09 cameron-robey

I started using Cloudflare workers as infrastructure for a personal project, and I wanted to know exactly how to test a locally scheduled cron job (with the scheduled module syntax). I'm again on the bleeding edge (matter of attraction, lol).

How is the community testing this exactly? I couldn't find many examples or references in the documentation to understand what unstable_dev's experimental.testScheduled option does. I know this is just a draft, but could you point me in the right direction?

fmquaglia avatar Jan 24 '23 21:01 fmquaglia

Hey @fmquaglia, check out the docs for --test-scheduled under https://developers.cloudflare.com/workers/wrangler/commands/#dev

rozenmd avatar Jan 25 '23 10:01 rozenmd

Closing this for now. The current approach is to provide { experimental: { testScheduled: true } } in the unstable_dev() options, and then to hit the /__scheduled URL. Also going forward the native vitest unit test running support that is being worked on will provide a way to run the scheduled() handler directly in tests if needed.

petebacondarwin avatar Jan 02 '24 11:01 petebacondarwin