blitz-js / blitz

⚡️ The Missing Fullstack Toolkit for Next.js
https://Blitzjs.com
MIT License
13.65k stars 798 forks source link

Mutation / Query Mocking utilities #280

Closed Skn0tt closed 11 months ago

Skn0tt commented 4 years ago

What do you want and why?

Commonly, API calls will be mocked out in tests in order to decouple and speed them up. Since one of Blitz core concepts is to hide network calls and replace them by (opaque) RPC, the natural counterpart would be to just mock out the generated network fetching logic instead.

The interface could look like this:

import ProjectsList from "./ProjectsList";
import getProjects from "./queries/getProjects";
import { mockQuery } from "blitz/test";

const getProjectsMock = mockQuery(
  getProjects,
  // mock implementation
  async () => {
    return [{ id: 1, name: "Blitz" }];
  } 
);

describe("ProjectsList", () => {
  it("calls `getProjects`", () => {
    const comp = render(() => <ProjectsList />);
    expect(getProjects).toHaveBeenCalled();
  })
})

Possible implementation(s)

Since Blitz already replaces queries with generated code, we could just generate a "sleeping mock" during testing. By "Sleeping", i mean: mocking would be disabled by default, but could be enabled using mockQuery.

This would circumvent all Test-Runner mocking mechanisms and thus doesn't run into the problem of mocks needing to be hoisted (see Jest hoisting jest.mock).

aem commented 4 years ago

Do we need this? Couldn't we just mock the getProjects fn in that example?

Skn0tt commented 4 years ago

You've got a point there - I should've explained why simply running jest.mock isn't sufficient 😅

So the problem is that the useQuery hook requires a cache key, which is hidden away in Blitz' code generation.

When mocking getProjects, you'd need to assign that cache key:

import getProjects from "./queries/getProjects";

async function mockImplementation() {
   return [{ id: 1, name: "Blitz" }];
}

mockImplementation.cacheKey = "mockImplementation";

jest.mock("./queries/getProjects", () => mockImplementation);

// ...

Since the cacheKey is a code gen implementation detail that should be hidden away from the user, a helper function could be provided from Blitz:

// blitz/test
export function createQueryMock<T extends Function>(mockImplementation: T) {
  mockImplementation.cacheKey = mockImplementation.name; // alternatively: some random, but stable name
  return mockImplementation;
}
import getProjects from "./queries/getProjects";
import { createQueryMock } from "blitz/test";

jest.mock(
  "./queries/getProjects",
  () => createQueryMock(async () => {
    return [{ id: 1, name: "Blitz" }];
  })
);

// ...

There's just one Problem with this: Since jest.mock calls are hoisted (see above), making createQueryMock available would require some kind of Babel/Webpack trickery.

There's one possible fix that came to my mind: Using queryFn.cacheKey ?? queryFn.name as the cache key. But I don't really know about the uniqueness guarantees required for cacheKey / given by Function#name, so maybe that's a good idea, maybe it isn't.

ryardley commented 4 years ago

What level of testing are you suggesting this is for?

Skn0tt commented 4 years ago

Mostly integration testing. It would allow you to test behaviour of your components and assert on the neccessary RPC calls being made.

aem commented 4 years ago

Jest mocking shouldn't require a mock cache key, the call should never hit the underlying implementation which means the cache key should be irrelevant 🤔 unless I'm reading this wrong

Skn0tt commented 4 years ago

The cache key is not a required part of the implementation, but solely to make it work with react-query. Testing components would hit react-query implementation, so that's why it's relevant.

ryardley commented 4 years ago

I get why you might want that but I am not sure that testing at that level makes a whole lot of sense honestly. Testing your backend talks to your frontend should probably be done with cypress. The cache key stuff will likely go through a lot of churn soon so anything we do here would need to be changed too. You still have standard jest based react-testing-library for pages and can simply use jest to mock out return values for queries and mutations using the require hooks jest provides.

Skn0tt commented 4 years ago

Well that's exactly the problem here: Mocking out return values for queries isn't easily possible, as the recommended way (useQuery(myFunnyQuery)) requires that cache key to be present in mocks - or am I missing something here? Actually, failing to test using react-testing-library and jest.mock is what inspired this issue in the first place.

ryardley commented 4 years ago

Ahh ok I see. Sounds to me like we should just have useQuery simply call the resolver function it is passed when it is being tested in jest as the backend won't be available.

Skn0tt commented 4 years ago

So like this ...

function useQuery(resolver) {
  if (isTestMode) {
    return [ resolver(), reactQuerySecondReturnValueMock ];
  }
  ...
}

(has the problem of not resembling react-query characteristics, which makes tests less useful)

... or like this ...

import { useQuery as useReactQuery } from "react-query";

function useQuery(resolver) {
  if (isTestMode) {
    return useReactQuery([ resolver.name ], resolver);
  }
  ...
}

(has the problem of the cache key, resolver.name in this case, being potentially duplicated, which could clash with its query cache)

... or some totally different way? Is there an option I'm missing here?

jan-wilhelm commented 4 years ago

Is there any update on this? I still find integration testing quite hard with Blitz. Any RFC / issues / PR somebody could point me to to leave my thoughts on?

flybayer commented 4 years ago

@jan-wilhelm the only thing we have so far is this default test file we include in new apps by default that shows how to mock a shared hook.

JoseRFelix commented 4 years ago

I recommend using Mock Service Worker to mock the requests at the network level using service workers. It is really easy to learn and use, and it lives separate from your code which makes it more readable. Check it out!

By the way, testing if a query or mutation is called is considered an implementation detail. I'm generally against it. Here is a really good article from Kent C. Dodds that explains why.

MrLeebo commented 3 years ago

I agree with @JoseRFelix about MSW being a good approach for this. It would be cool if we had a blitz wrapper around MSW for handling RPC calls without having to calculate their URLs:

import { rpc } from '@blitz/test';
import { setupServer } from 'msw/node';
import getUsers from 'app/users/queries/getUsers';

const server = setupServer(rpc(getUsers, (req, res, ctx) => { ... }));
reggie3 commented 3 years ago

Has anyone had any success getting Mock Service Worker working so far?

I'm trying to set up basic MSW integration with blitz for testing and am receiving this error when running my test.

TypeError: Cannot read property '$authorize' of undefined
        at _innerAuthorize (C:\Users\rej_i\OneDrive\Development\blitz-quiz-app\node_modules\@blitzjs\core\server\dist\blitzjs-core-server.cjs.dev.js:377:13)
        at C:\Users\rej_i\OneDrive\Development\blitz-quiz-app\node_modules\@blitzjs\core\server\dist\blitzjs-core-server.cjs.dev.js:358:26
        at processTicksAndRejections (internal/process/task_queues.js:97:5)

which looks related to the authorize function in blitzjs-core-server.cjs.dev.js here (console.logs added by me for debugging):

image

So my test is throwing that error because it doesn't pass a ctx argument with a valid session. What is the correct way around this?

flybayer commented 3 years ago

@reggie3 I haven't used MSW, but my understanding is that when MSW makes it easier to test your frontend by removing any calls to backend code, right?

If that's true, then the issue you are showing is that backend code is still being called. Probably because you are importing the query or mutation and passing to useQuery. But in our build, we replace that import with a tiny http client. So by default with code as it's written, there is no http calls. You first have to replace the server function import with an http client.

It might be possible to add some MSW support directly to our useQuery hook that would automatically do that swap in a MSW environment 🤔

reggie3 commented 3 years ago

@flybayer Thanks for the quick response.

With MSW, an API call is still made by the client, but the call is intercepted by MSW which responds with whatever is specified in the configuration give to MSW. So the backend call isn't removed, just intercepted before it gets to the actual server.

Edit: This statement was incorrect, the test was actually calling the server and returning data from it: =========> I found that if I comment out the call to resolver.authorize function and hardcoded the the value for a reference to ctx.session.userId I was able to successfully run a test that retrieved the expected response from MSW. <=========

image

I have a few question:

Here is my test:

import { render, screen } from "test/utils"
import userEvent from "@testing-library/user-event"
import NewAnswerPage from "./new"

const { queryByTestId } = screen

describe("new answer test", () => {
  it("should let the user create a new answer", () => {
    render(<NewAnswerPage />)
    userEvent.type(queryByTestId("labeled-text-field-answer")!, "HELLO NURSE")
    userEvent.click(queryByTestId("button-create-answer")!)
    // console.log(screen.debug())
  })
})

Where NewAnswerPage is a BlitzPage component created by the generator. It contains the "usual" form component that calls the mutation function on form submission, which the test does using userEvent.click(queryByTestId("button-create-answer")!)

flybayer commented 3 years ago

Ok, so in your case MSW is not doing anything, because there's no network request. The resolver is being called directly. If MSW was intercepting a network request, then that resolver function would not be called.

Here's the rpc client that we replace the function import with.

Maybe let's take a step back, what's the end goal you are trying to accomplish here?

reggie3 commented 3 years ago

Thanks for the info on the rpc client.

  1. I'm trying to run a React Testing Library test that types text into an input and clicks a submit button.
  2. The submit button makes an async call to a mutation.
  3. When the program is run in dev, the mutation returns an object based on the response from the server. Since this is a test, I want to specify the response that is returned.
  4. A successful object return should result in a new component with some of that objects information being added to the list which I can make an assertion on in the test.

I guess the bottom line is that I need a way to specify what the mutation returns.

This is in line with the prior conversations in this thread, and MSW was one of the techniques mentioned as a potential solution.

flybayer commented 3 years ago

@reggie3 ok understood, Then I think by far the easiest way is to mock your createAnswer mutation with jest, like mocking any old function. No network involved. No need for MSW.

reggie3 commented 3 years ago

Seems like I'll run into the same problems @Skn0tt ran into previously if I try it that way, but I'll give it a shot.

reggie3 commented 3 years ago

@flybayer Once again thanks for all the help so far. I do have another question. When you said:

"Ok, so in your case MSW is not doing anything, because there's no network request. The resolver is being called directly. If MSW was intercepting a network request, then that resolver function would not be called."

How is the resolver is being called directly? The documentation for Mutation Resolvers states "Blitz mutations are plain, asynchronous JavaScript functions that always run on the server."

I verified my MSW setup works when calling the Star Wars API so that's not an issue. Is there something about testing that prevents the generated code from attempting to call APIs?

Edit: From the "Blitz Internal Codebase Walkthrough" at https://youtu.be/RBAkhDrVRiA?t=879 there was an isomorphicRpc function in the old rpc.ts file that returned a resolver if window was undefined. A similar setup in whatever replaced rpc.ts would cause a Jest test to immediately call the resolver instead of making an API call.

I also understand better when you said "But in our build, we replace that import with a tiny http client. So by default with code as it's written, there is no http calls. You first have to replace the server function import with an http client." Is that what is causing the resolver to be called directly?

flybayer commented 3 years ago

Here's the babel plugin that replaces the resolver file contents in the client build: https://github.com/blitz-js/blitz/blob/canary/nextjs/packages/next/build/babel/plugins/blitz-rpc-client.ts

And here's the babel plugin that modifies the resolver file contents in the server build: https://github.com/blitz-js/blitz/blob/canary/nextjs/packages/next/build/babel/plugins/blitz-rpc-server-transform.ts