Bugfix for resolving & mocking, when resolve is wrapped

[DxCx]

I've noted a bug with @sebastienbarre 's new logic to merge resolvers & mocks (#115)
Basiclly, if there is a connector or a logger (or any other wrapper, hidden as a resolve function),
the original resolve function value will be undefined (as it's not provided)
and then an undefined will be returned, instead of the proper mocked value.

TODO:

• Update CHANGELOG.md with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[helfer]

Are you sure that's a bug with mocking and not with the logger? It seems strange to me that it would change anything whether or not the resolve function is wrapped with another one.

[DxCx]

Yes, if the logger is not present, then the field resolve will be undefined (defaults to default resolver) which mocking will override by mockResolver.
however, if you do use a logger and then there will be an error on default resolver you want it to be printed.
Therefore IMO once the resolve is wrapped we need to keep it wrapped..
The other solution is to just make sure the mockReaolver will be written instead any wrapper that is there.
you can look at the tests i've committed, without the fixes, they return NULL instead of mocked data,
while if there won't be a logger/connector the return value will be mocked data.

[sebastienbarre]

@DxCx thanks for all the research. Unfortunately I don't have much expertise with the loggers or connectors (yet).
But your fix:

        return (undefined !== resolvedValue) ? resolvedValue : mockedValue;

Doesn't fix this test for me:

  it('can mock object types (again)', () => {
    const jsSchema = buildSchemaFromTypeDefinitions(shorthand);
    const resolvers = {
      RootQuery: {
        returnInt: () => 123,
      },
    };
    addResolveFunctionsToSchema(jsSchema, resolvers);
    const testQuery = `{
      returnObject { returnInt, returnString }
    }`;
    const expected = {
      returnObject: { returnInt: 123, returnString: 'Hello World' },
    };
    return graphql(jsSchema, testQuery).then((res) => {
      expect(res.data).to.deep.equal(expected);
    });
  });

as I'm getting:

      AssertionError: expected { returnObject: null } to deeply equal { Object (returnObject) }
      + expected - actual

       {
      -  "returnObject": [null]
      +  "returnObject": {
      +    "returnInt": 123
      +    "returnString": "Hello World"
      +  }
       }

And this slightly altered test, where the mockMap is empty:

  it('can mock object types (again)', () => {
    const jsSchema = buildSchemaFromTypeDefinitions(shorthand);
    const mockMap = {};
    const resolvers = {
      RootQuery: {
        returnInt: () => 123,
      },
    };
    addResolveFunctionsToSchema(jsSchema, resolvers);
    addMockFunctionsToSchema({ schema: jsSchema, mocks: mockMap, preserveResolvers: true });
    const testQuery = `{
      returnObject { returnInt, returnString }
    }`;
    const expected = {
      returnObject: { returnInt: 123, returnString: 'Hello World' },
    };
    return graphql(jsSchema, testQuery).then((res) => {
      expect(res.data).to.deep.equal(expected);
    });
  });

Also fails for me. I'm getting:

      AssertionError: expected { Object (returnObject) } to deeply equal { Object (returnObject) }
      + expected - actual

       {
         "returnObject": {
      -    "returnInt": -62
      +    "returnInt": 123
           "returnString": "Hello World"
         }
       }

Granted, both tests were also failing without your fix.

[DxCx]

hi @sebastienbarre,
Thanks for the review.

Please note that your query is to the query => returnObject,
also, there is no mock asked for the first example,
so after applying those changes:

@@ -406,10 +406,10 @@ describe('Mock', () => {
     };
     addResolveFunctionsToSchema(jsSchema, resolvers);
     const testQuery = `{
-      returnObject { returnInt, returnString }
+      returnInt, returnString
     }`;
     const expected = {
-      returnObject: { returnInt: 123, returnString: 'Hello World' },
+      returnInt: 123, returnString: null,
     };
     return graphql(jsSchema, testQuery).then((res) => {
       expect(res.data).to.deep.equal(expected);

so we can see that without mocking, string returns null as expected

and for the second:

@@ -426,10 +426,10 @@ describe('Mock', () => {
     addResolveFunctionsToSchema(jsSchema, resolvers);
     addMockFunctionsToSchema({ schema: jsSchema, mocks: mockMap, preserveResolvers: true });
     const testQuery = `{
-      returnObject { returnInt, returnString }
+      returnInt, returnString
     }`;
     const expected = {
-      returnObject: { returnInt: 123, returnString: 'Hello World' },
+      returnInt: 123, returnString: 'Hello World',
     };
     return graphql(jsSchema, testQuery).then((res) => {
       expect(res.data).to.deep.equal(expected);

and with mocking, it is returning the mocked string with the resolved int as expected.

[helfer]

I think this could potentially lead to confusion if someone defines a resolver which returns undefined by accident. It will be much harder to find the error in that case, because the return value will be replaced with mock data. I don't have a simple solution for that though, so let's just go with this for now.

[DxCx]

I Agree it should be solved in a more proper way, and its not an easy fix, That's the reason i used explicitly undefined ( !== ) so the user can return null for specifying undefined behavior.

[sebastienbarre]

@DxCx sorry, I'm not sure why you are saying my query is incorrect.

     const testQuery = `{
       returnObject { returnInt, returnString }
     }`;

If you look at the schema:

    interface Flying {
      returnInt: Int
    }
    type Bird implements Flying {
      returnInt: Int
      returnString: String
      returnStringArgument(s: String): String
    }
    type RootQuery {
      returnObject: Bird
    }

Unless I'm mistaken, the above implies that returnObject does have both the returnInt and returnString fields. So even if there are no mocks for returnObject, the schema is all is needed to infer that it has both returnInt and returnString, which can be mocked automatically (returnInt has a resolver).

[DxCx]

Hi @sebastienbarre,
the resolver was for returnInt of rootQuery, not Bird.
doing the following modification also works:

-    const mockMap = {};
+    const mockMap = {
+               Int: () => 123,
+       };
     const resolvers = {
       RootQuery: {
-        returnInt: () => 123,
+        returnInt: () => 124,
       },
     };
     addResolveFunctionsToSchema(jsSchema, resolvers);
     addMockFunctionsToSchema({ schema: jsSchema, mocks: mockMap, preserveResolvers: true });
     const testQuery = `{
-      returnObject { returnInt, returnString }
+      returnObject { returnInt, returnString }, returnInt
     }`;
     const expected = {
       returnObject: { returnInt: 123, returnString: 'Hello World' },
+      returnInt: 124
     };
     return graphql(jsSchema, testQuery).then((res) => {
       expect(res.data).to.deep.equal(expected);
     });
   });

which shows that returnObj int is mocked
and returnInt of rootQuery is resolved.