Promise-ify all tests, and pass a Promise reject function to mocking utilities. #5474

benjamn · 2019-10-17T17:14:15Z

Because Jest has no ability to trace uncaught exceptions back to the test responsible for throwing them, the Apollo Client test suite has been plagued for a long time by "No more mocked responses for the query" errors, often causing unrelated tests to fail.

I have finally tracked the source of these errors down to the implementation of MockLink, which was using throw to raise exceptions, potentially outside the bounds of the test for which the MockLink was originally created.

This gigantic commit does two things:

Make all done/done.fail-style tests use Promise resolve and reject functions instead (the bulk of the changes). This is important because we don't want stray done.fail calls causing tests that have already passed to fail, and resolve/reject are conveniently idempotent.
Make the mocking utilities (MockLink, mockSingleLink, etc.) require a Promise reject function, so that they can safely report errors back to the true original test that created the Promise.

A better testing framework would maintain an execution context for each test that allowed exceptions to be attributed to the correct test without explicitly passing around reject functions, but doing that right probably requires some ECMAScript language support (or at least a transpiler to simulate it). This is an exciting future possibility, but not something we can tackle here.

benjamn · 2019-10-17T17:14:59Z

src/__mocks__/mockLinks.ts

@@ -67,18 +74,20 @@ export class MockLink extends ApolloLink {
    const key = requestToKey(operation);
    const responses = this.mockedResponsesByKey[key];
    if (!responses || responses.length === 0) {
-      throw new Error(
+      this.reject(new Error(


This is the key to silencing "No more mocked responses for the query" errors after the original test has already passed (or failed).

benjamn · 2019-10-17T17:16:57Z

src/__mocks__/mockLinks.ts

+    if (typeof this.reject !== "function") {
+      throw new Error("Must pass a failure callback when creating MockLink");


Making this an early error was incredibly helpful for tracking down failures to pass in the new reject function.

benjamn · 2019-10-17T17:18:01Z

src/react/testing/mocks/mockLink.ts

@@ -30,10 +30,14 @@ export class MockLink extends ApolloLink {
  private mockedResponsesByKey: { [key: string]: MockedResponse[] } = {};

  constructor(
+    private reject: (reason: any) => any,


@hwillson As we discussed over Slack, I think we should unify the React Apollo versions of MockLink etc. with the Apollo Client implementations, now that everything resides in the same codebase.

benjamn · 2019-10-17T17:19:04Z

src/react/hooks/__tests__/useMutation.test.tsx

@@ -268,7 +268,7 @@ describe('useMutation Hook', () => {
  });

  describe('Optimistic response', () => {
-    it('should support optimistic response handling', async () => {
+    it('should support optimistic response handling', async () => new Promise((resolve, reject) => {


I would love to shorten this to

Suggested change

it('should support optimistic response handling', async () => new Promise((resolve, reject) => {

it.async('should support optimistic response handling', async (resolve, reject) => {

I will push a follow-up commit to this PR if I can figure out a good way to add that it.async API.

Because Jest has no ability to trace uncaught exceptions back to the test responsible for throwing them, the Apollo Client test suite has been plagued for a long time by "No more mocked responses for the query" errors, often causing unrelated tests to fail. I have finally tracked the source of these errors down to the implementation of MockLink, which was using `throw` to raise exceptions, potentially outside the bounds of the test for which the MockLink was originally created. This gigantic commit does two things: * Make all done/done.fail-style tests use Promise resolve and reject functions instead (the bulk of the changes). This is important because we don't want stray done.fail calls causing tests that have already passed to fail, and resolve/reject are conveniently idempotent. * Make the mocking utilities (MockLink, mockSingleLink, etc.) require a Promise reject function, so that they can safely report errors back to the true original test that created the Promise. A better testing framework would maintain an execution context for each test that allowed exceptions to be attributed to the correct test without explicitly passing around reject functions, but doing that right probably requires some ECMAScript language support (or at least a transpiler to simulate it). This is an exciting future possibility, but not something we can tackle here.

hwillson

Thanks very much for working on this @benjamn! It's amazingly annoying that we have to go these lengths, but such is life. As discussed, we should call these changes out somehow/somewhere so contributors are aware, but this pattern should organically catch on as people mirror existing test cases. An update in the changelog should be good enough for now.

src/__tests__/optimistic.ts

benjamn · 2019-10-17T19:33:34Z

src/core/__tests__/QueryManager/index.ts

    }, 10);
  });

  // XXX this looks like a bug in zen-observable but we should figure
  // out a solution for it
-  xit('handles an unsubscribe action that happens before data returns', done => {
+  itAsync.skip('handles an unsubscribe action that happens before data returns', (resolve, reject) => {


Pretty cool that itAsync.skip works just like it.skip!

benjamn added this to the Release 3.0 milestone Oct 17, 2019

benjamn self-assigned this Oct 17, 2019

benjamn commented Oct 17, 2019

View reviewed changes

benjamn force-pushed the promisify-all-tests-and-fix-no-more-mocked-responses-failures branch from 5cd380d to 2982e68 Compare October 17, 2019 17:22

benjamn requested a review from hwillson October 17, 2019 17:22

benjamn added ⛑ TypeScript 👩‍🏭 refactor 🛠 tooling 🧞‍♂️ enhancement labels Oct 17, 2019

hwillson approved these changes Oct 17, 2019

View reviewed changes

src/__tests__/optimistic.ts Outdated Show resolved Hide resolved

Use itAsync helper to reduce Promise-based test boilerplate.

81e6872

benjamn commented Oct 17, 2019

View reviewed changes

benjamn merged commit 4bee850 into release-3.0 Oct 17, 2019

benjamn deleted the promisify-all-tests-and-fix-no-more-mocked-responses-failures branch October 17, 2019 19:36

niekert mentioned this pull request Jul 8, 2020

[V3] - Missing mocks in MockedProvider throws uncaught error #6559

Closed

github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promise-ify all tests, and pass a Promise reject function to mocking utilities. #5474

Promise-ify all tests, and pass a Promise reject function to mocking utilities. #5474

benjamn commented Oct 17, 2019

benjamn Oct 17, 2019

benjamn Oct 17, 2019

benjamn Oct 17, 2019

benjamn Oct 17, 2019

hwillson left a comment

benjamn Oct 17, 2019

hwillson Oct 17, 2019

		if (typeof this.reject !== "function") {
		throw new Error("Must pass a failure callback when creating MockLink");

	it('should support optimistic response handling', async () => new Promise((resolve, reject) => {
	it.async('should support optimistic response handling', async (resolve, reject) => {

Promise-ify all tests, and pass a Promise reject function to mocking utilities. #5474

Promise-ify all tests, and pass a Promise reject function to mocking utilities. #5474

Conversation

benjamn commented Oct 17, 2019

benjamn Oct 17, 2019

Choose a reason for hiding this comment

benjamn Oct 17, 2019

Choose a reason for hiding this comment

benjamn Oct 17, 2019

Choose a reason for hiding this comment

benjamn Oct 17, 2019

Choose a reason for hiding this comment

hwillson left a comment

Choose a reason for hiding this comment

benjamn Oct 17, 2019

Choose a reason for hiding this comment

hwillson Oct 17, 2019

Choose a reason for hiding this comment