Skip to content
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

Feat: allow access to the Durable Object's instance from inside a test #385

Merged
merged 3 commits into from
Dec 22, 2022

Conversation

cdrx
Copy link
Contributor

@cdrx cdrx commented Sep 20, 2022

cdaa62b added the ability to directly construct a Durable Object, which is great for testing a DO in isolation but doesn't allow for easily testing Durable Objects that communicate with other Durable Objects.

Without access to the internal instance that stub.fetch(...) uses, it isn't easy to test request that span across multiple Durable Objects.

For example, given this example of an object that makes a sub request to another object:

class TestObject extends DurableObject {
  fetch(request) {
    // make a subrequest to OtherObject
    const { OTHER_OBJECT } = getMiniflareBindings();
    const id = OTHER_OBJECT.idFromName("test");
    const stub = OTHER_OBJECT.get(id);
    return stub.fetch("https://object/");
  }
}

class OtherObject extends DurableObject {
  testFunction() {
    return "Hello world"
  }
  
  fetch(request) {
    return new Response(this.testFunction());
  }
}

The change in this PR allows writing a test that can introspect the behaviour of OtherObject, even though the initial request goes to TestObject, like this:

test("", async () => {
  const { TEST_OBJECT, OTHER_OBJECT } = getMiniflareBindings();
  
  const otherId = OTHER_OBJECT.idFromName("test");
  const otherInstance = getMiniflareDurableObjectInstance(otherId);
  const otherTestFunction = vi.spyOn(otherInstance, "testFunction");
  
  const testId = TEST_OBJECT.idFromName("test");
  await TEST_OBJECT.get(testId).fetch("https://object/");
  
  expect(otherTestFunction).toHaveBeenCalledOnce();
});

This allows access to the Durable Object instance for direct manipulation and testing
@cdrx
Copy link
Contributor Author

cdrx commented Sep 25, 2022

@mrbbot I'm curious about how the architecture will look, when using Miniflare with the OSS runtime.

When running tests, will it be the case that the Durable Object are run inside the OSS runtime (V8), but Miniflare still runs Jest inside Node's JS context?

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! 👋 Thanks for raising this use case, and opening the PR! 🙂 I've added a comment about input/output gating. Interested to hear your thoughts, but otherwise happy to merge this. ✅

const instance = await getMiniflareDurableObjectInstance(id);
const fetch = jest.spyOn(instance, "fetch");

await stub.fetch(new Request("https://object/"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here, is that fetch won't run under the input/output gates, so concurrent calls to fetch may produce invalid results. I'm not too sure what the best solution here is:

  1. We could ask users to also do const state = getMiniflareDurableObjectState(), and use runWithMiniflareDurableObjectGates() as above, but reflecting on that API now, it feels a little too verbose.
  2. We could wrap use a Proxy or something on getMiniflareDurableObjectInstance() to intercept fetch() calls and apply input/output gates to them, but this might break mocks/spies.

Maybe option 1. is fine for now. What do you think?

@mrbbot
Copy link
Contributor

mrbbot commented Sep 30, 2022

I'm curious about how the architecture will look

Me too! 😉 We're discussing this internally. It should be possible to use a custom Jest runner which runs tests out-of-band in workerd. This would tie us to Jest, which I'd rather not do. Alternatively, we could try compile workerd to a Node native module including Workers runtime APIs, and do things like we're doing at the moment injecting them into the global scope.

@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2022

⚠️ No Changeset found

Latest commit: 75cffa3

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

@mrbbot mrbbot merged commit c2bcc74 into cloudflare:master Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants