feat(mocking): make response error and result not required when delay is infinite

[mspiess]

Without the changes in this PR mocks always require a result or an error for a response. However, when an infinite delay is specified, that response is never returned, so it might as well be omitted.
With the changes in this PR omission of result and error no longer causes an error, when a delay of Infinity is specified.

const mocks = [
  {
    delay: Infinity
    request: {
      query: GET_DOG_QUERY,
      variables: {
        name: "Buck"
      }
    },
    result: {                                             // all of this
      data: {                                             // makes no
        dog: { id: "1", name: "Buck", breed: "bulldog" }  // sense with
      }                                                   // an infinite
    }                                                     // delay
  }
];


it("renders without error", async () => {
  render(
    <MockedProvider mocks={mocks} addTypename={false}>
      <Dog name="Buck" />
    </MockedProvider>
  );
  expect(await screen.findByText("Loading...")).toBeInTheDocument();
});

[apollo-cla]

@mspiess: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[netlify]

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	7bebece

[changeset-bot]

🦋 Changeset detected

Latest commit: 7bebece

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

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

[jerelmiller]

Hey @mspiess 👋

Appreciate the contribution! Would you be willing to provide some more information on the use case you're solving for and provide some more information about this change? Thanks!

[mspiess]

Hey @jerelmiller 👋

I updated the PR description.

[jerelmiller]

Hey @mspiess 👋

I think this change makes sense and provides a nice improvement in your tests. Appreciate the additional context on why this change makes sense!

I've got a few minor things for you to look at. I'd love to get this in the next patch or two. Thanks again!

[jerelmiller]

One more thing! Looks like we've got a couple snapshot tests that check the output of that error in MockedProvider.test.tsx. After we get the error output settled, can you update those snapshot tests to the new error message? That should get your tests passing in this PR. Thanks!

[alessbell]

Thanks for working on this and pushing those updates! Just took one more look - this is almost ready to merge 👍

[alessbell]

As it turns out, both Number.MAX_VALUE and Number.MAX_SAFE_INTEGER will cause the timeout to fire immediately because neither fit into a 32-bit signed integer. (We can tell that it's not doing anything here since the test finishes in less than the default Jest 5000ms timeout without timer mocks.)

The timing itself isn't material to the behavior of this test, but it would be nice to make it accurate to the behavior we'd see in a real test.

With that in mind, would you mind adding a comment about this here and using a random very high number? We can use Jest's timer mock feature and e.g. jest.advanceTimersByTime(2_000_000_000) here to get the test passing again.

[jerelmiller]

Ah shoot, I forgot about that. Thanks for the reminder @alessbell!

[mspiess]

I used a constant named MAXIMUM_DELAY. Does that provide enough context, or would you prefer a comment?

[alessbell]

A short comment above MAXIMUM_DELAY would be helpful, something like "We've chosen this value as the MAXIMUM_DELAY since values that don't fit into a 32-bit signed int cause setTimeout to fire immediately"

[alessbell]

We're just missing a useRealTimers:

Suggested change

	jest.useFakeTimers();
	beforeAll(() => jest.useFakeTimers());
	afterAll(() => jest.useRealTimers());

[alessbell]

Left two more minor comments, after those changes are pushed up this is good to go 🚀 Appreciate the contribution @mspiess!

[jerelmiller]

Thanks a bunch @mspiess! We'll get this in the next patch which should release sometime next week.

[mspiess]

Great! 🎉 Thanks for the reviews and detailed comments @jerelmiller and @alessbell!