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

allow for deeply nested test double objects #407

Merged
merged 11 commits into from Feb 24, 2019

Conversation

searls
Copy link
Member

@searls searls commented Feb 7, 2019

pasted from #402

Popular Prisma graphqlgen scaffolds a graphql server with a Context interface, that is used by graphql resolvers to execute commands and queries (on services, repositories, etc).

the default generated example looks like this:

export interface Context {
  data: Data
}

export interface User {
  id: string
  name: string | null
  postIDs: string[]
}

export interface Post {
  id: string
  title: string
  content: string
  published: boolean
  authorId: string
}

export interface Data {
  posts: Post[]
  users: User[]
  idProvider: () => string
}

With this change I can do things like:

const context = td.object<Context>();

context.data.users = [{id: "userId", name: "Hello", postIDs: []}]
td.when(context.data.idProvider()).thenReturn("providerId")

And everything is still properly typed. :-)
Meaning - if I do

context.data.users = [{ids: "userId"}]

I get TS2322: Type '{ ids: string; }' is not assignable to type 'User'.

and for

context.data.users = [{id: "userId", name: "Hello", postIDs: [1]}]

I get TS2322: Type 'number' is not assignable to type 'string'. error, with the 1 marked in the IDE as an error.

for

   td.when(context.data.idProvider("sdf")).thenReturn("providerId")

I get TS2554: Expected 0 arguments, but got 1.

...and so on.

This context type that holds repositories/services is a common graphql pattern. I'm pretty sure there could be other uses as well.

@searls
Copy link
Member Author

searls commented Feb 13, 2019

@lgandecki IMO the change to td.explain in 247b673 increases its complexity too great a degree relative to how minor an edge case it's meant to handle. And because its intention (handling this edge case) isn't clearly encoded anywhere in the new code, future-@searls would be very likely to refactor it right back if I stumble upon it.

If this is the solution we should use, then I think we ought to extract it into a local utility module with a name like proxySafeCloneDeepWith and do our best to otherwise leave td.explain unchanged, so that we aren't mixing levels of abstraction in the scope of this fix (which is really a workaround of _.cloneDeepWith and not td.explain per se)

@lgandecki
Copy link
Contributor

I agree. The complexity increases but not for a good reason. I will try to extract parts of it to a utility module.

@lgandecki
Copy link
Contributor

Hey @searls , thanks for pushing me here, I found a much better solution:
https://github.com/testdouble/testdouble.js/pull/407/files#diff-bc82ad43935d216a46374bd892b82a76R30
I'm not sure where to move this proxySafeCloneDeepWith function though.

I tried to break this with a bunch of tests, but they all behaved the same way with and without this fix.
Not sure if this additional safety net makes sense there, so I didn't include it in a commit.

 'explains a nested td.constructor' () {
    function Dog () {}
    Dog.prototype.bark = function () {}
    Dog.woof = function () {}
    var FakeDog = td.constructor(Dog)

    const baz = {
      FakeDog
    }

    result = td.explain(baz)
    assert._isEqual(result, {
      name: null,
      callCount: 0,
      calls: [],
      description: 'This object contains 1 test double function: ["Dog"]',
      children: {
        FakeDog: {
          name: 'Dog',
          callCount: 0,
          calls: [],
          description: 'This test double `Dog` has 0 stubbings and 0 invocations.',
          children: {
            toString: {
              name: undefined,
              callCount: 0,
              calls: [],
              description: 'This is not a test double function.',
              isTestDouble: false
            },
            woof: {
              name: 'Dog.woof',
              callCount: 0,
              calls: [],
              description: 'This test double `Dog.woof` has 0 stubbings and 0 invocations.',
              children: {
                toString: {
                  name: undefined,
                  callCount: 0,
                  calls: [],
                  description: 'This is not a test double function.',
                  isTestDouble: false
                }
              },
              isTestDouble: true
            }
          },
          isTestDouble: true
        }
      },
      isTestDouble: true
    }
    )
  },
  'passed an object where toString is mocked' () {
    const baz = {
      toString: td.when(td.function('foo')()).thenReturn('biz'),
      bar: () => 'bar'
    }
    result = td.explain(baz)

    assert._isEqual(result, {
      name: null,
      callCount: 0,
      calls: [],
      description: 'This object contains 1 test double function: ["foo"]',
      children: {
        toString: {
          name: 'foo',
          callCount: 0,
          calls: [],
          description: 'This test double `foo` has 1 stubbings and 0 invocations.\n\nStubbings:\n  - when called with `()`, then return `"biz"`.',
          children: {
            toString: {
              name: undefined,
              callCount: 0,
              calls: [],
              description: 'This is not a test double function.',
              isTestDouble: false
            }
          },
          isTestDouble: true
        },
        bar: {
          name: undefined,
          callCount: 0,
          calls: [],
          description: 'This is not a test double function.',
          isTestDouble: false
        }
      },
      isTestDouble: true
    })
  },
  'passed an object that contains a nested class that overwrites toString' () {
    const somethingWithNestedClass = {
      myClass: class Hello {
        constructor () {
          console.log('hello world')
        }
        toString () {
          return 'hello'
        }
      }
    }

    const fakeThing = td.object(somethingWithNestedClass)

    result = td.explain(fakeThing)

    assert._isEqual(result, {
      name: null,
      callCount: 0,
      calls: [],
      description: 'This object contains 1 test double function: [".myClass"]',
      children: {
        myClass: {
          name: '.myClass',
          callCount: 0,
          calls: [],
          description: 'This test double `.myClass` has 0 stubbings and 0 invocations.',
          children: {
            toString: {
              name: undefined,
              callCount: 0,
              calls: [],
              description: 'This is not a test double function.',
              isTestDouble: false
            }
          },
          isTestDouble: true
        },
        toString: {
          name: undefined,
          callCount: 0,
          calls: [],
          description: 'This is not a test double function.',
          isTestDouble: false
        }
      },
      isTestDouble: true
    }
    )
  }

@searls searls merged commit 738f5a5 into master Feb 24, 2019
@searls
Copy link
Member Author

searls commented Feb 24, 2019

landed in 3.11.0

@searls searls deleted the lgandecki-deeplyNestedProxyObjects branch February 24, 2019 18:13
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.

None yet

2 participants