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

mockRedwoodDirective fails when ValidatorDirective is async #5713

Closed
xmonkee opened this issue Jun 8, 2022 · 5 comments · Fixed by #5822
Closed

mockRedwoodDirective fails when ValidatorDirective is async #5713

xmonkee opened this issue Jun 8, 2022 · 5 comments · Fixed by #5822
Assignees
Labels
bug/confirmed We have confirmed this is a bug topic/testing

Comments

@xmonkee
Copy link

xmonkee commented Jun 8, 2022

Issue

mockRedwoodDirective does not await the inner validator function, thus making it hard to test that an async validator functions throws an error

Repro

  1. Generate a new directive (validator):
    yarn rw generate directive requireSomething
  2. Make the validator function async:
const validate: ValidatorDirectiveFunc = async ({ context, directiveArgs }) => {
  throw new Error('Implementation missing for requireSomething')
}
  1. Run the default tests
describe('requireSomething directive', () => {
  it('has a requireSomething throws an error if validation does not pass', async () => {
    const mockExecution = mockRedwoodDirective(requireSomething, {})
    expect(mockExecution).toThrowError()
  })
})

It will give two errors: Received function did not throw and then the actual throw as an error

This is the same behavior jest shows when trying to expect(...).toThrow() on an async function without doing async expect(..).rejects.toThrow()

Possible fix

Add a mockAsyncRedwoodDirective. This is just mockRedwoodDirective which now returns an async function that awaits the validator and transformer functions

export const mockAsyncRedwoodDirective = (directive, executionMock) => {
  const { directiveArgs = {}, context, ...others } = executionMock

  if (context) {
    ;(0, _graphqlServer.setContext)(context || {})
  }

  return async () => {
    if (directive.type === _graphqlServer.DirectiveType.TRANSFORMER) {
      const { mockedResolvedValue } = others
      return await directive.onResolverCalled({
        resolvedValue: mockedResolvedValue,
        context: _graphqlServer.context,
        ...others,
      })
    } else {
      await directive.onResolverCalled({
        context: _graphqlServer.context,
        directiveArgs,
        ...others,
      })
    }
  }
}

And modify the test

  it('has a requireSomething throws an error if validation does not pass', async () => {
    const mockExecution = mockAsyncRedwoodDirective(requireSomething, {})
    await expect(mockExecution()).rejects.toThrowError(
      'Implementation missing for requireSomething'
    )
  })
@jtoar jtoar moved this to Triage in Main Jun 8, 2022
@jtoar jtoar added this to Main Jun 8, 2022
@jtoar jtoar added bug/confirmed We have confirmed this is a bug topic/testing labels Jun 10, 2022
@jtoar
Copy link
Contributor

jtoar commented Jun 11, 2022

Thanks @xmonkee, I was able to confirm this one. @dthyresson is the mind behind many of the GraphQL integrations in the framework so I'll tag him here to see what he thinks about your fix.

@dthyresson
Copy link
Contributor

@dac09 Thoughts on letting validator directives be async?

@dac09
Copy link
Contributor

dac09 commented Jun 14, 2022

I’m a little spread thin DT - I don’t want to get in the way of you investigating/solving this!

@dthyresson
Copy link
Contributor

@dac09 I still planned to review, I just wanted to know if you had any reservations about a validator making some sort of async call (like a db query inside the graphql execution).

@jtoar jtoar moved this from Triage to Todo in Main Jun 20, 2022
@dthyresson
Copy link
Contributor

Hi @xmonkee let's add

export const mockAsyncRedwoodDirective: DirectiveMocker = async (
  directive,
  executionMock
) => {
  const { directiveArgs = {}, context, ...others } = executionMock

  if (context) {
    setContext(context || {})
  }

  return async () => {
    if (directive.type === DirectiveType.TRANSFORMER) {
      const { mockedResolvedValue } = others as TransformerMock
      return await directive.onResolverCalled({
        resolvedValue: mockedResolvedValue,
        context: globalContext,
        ...others,
      } as DirectiveParams)
    } else {
      await directive.onResolverCalled({
        context: globalContext,
        directiveArgs,
        ...others,
      } as DirectiveParams)
    }
  }
}

Would you be interested in contributing a PR for that and the test? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/testing
Projects
None yet
5 participants