[RFC] flow compatible mocking API

[aaronabramov]

when using flow in tests we can't patch objects in runtime anymore, that means we'll have to provide a Jest API that will be capable of mocking things without changing their public interface.

API that i propose is:

// alias
console.warn = jest.fn();
// to
jest.stub(console, 'warn');

// alias
console.warn.mock.calls[0][0];
// to
jest.getMock(console.warn); // throws if it's not a mock

// alias
require.requireActual()
// to
jest.requireActual() // there's an issue for it, also requires change in the haste parser

there's also a set of functions that is defined on the mock, that we can alias somewhere too

like

jest.mock.mockRestore(getMock(console.warn));
jest.mockImplementation(getMock(console.warn, () => 42))

at this point i don't think we should rewrite or redesign the whole API, just aliasing these methods in a flow compatible way should work.

what i'm not sure about is stub as a term, because there's a lot of confusion between stubs, mock, spies, doubles an all these terms, but since jest.mock is already taken, jest.stub might be our best option.

cc @TheSavior @dalongi

[Daniel15]

jest.mockImplementation(getMock(console.warn, () => 42))

What about an API like this instead:

jest.getMock(console.warn).mockImpl(() => 42)

Similarly, console.warn.mock.calls[0][0] would be replaced by jest.getMock(console.warn).calls()[0] (or maybe something like jest.getMock(console.warn).lastCall())

getMock could return a Flow-typed mock function with all these functions. See FBMock_MockObject from FBMock as prior art with a similar API: https://github.com/facebook/FBMock/blob/b90fce5b95c6ea9343084ba7b8403fef935b1688/MockObject.php#L12

I wonder if you could implement that simply by making jest.getMock return the mock function just like we have today, except cast it to a Flow type. So, console.warn.mockImpl and jest.getMock(console.warn).mockImpl would be identical.

[elicwhite]

@Daniel15, I believe that is the approach we've been thinking about but @aaronabramov's description may not have been clear based on his example.

His PR at #4265 implements exactly what you describe. One of the tests includes this:

jest.getMock(obj.a).mockImplementation(() => 'waffles');

[cpojer]

I would like to spend more time discussing this proposal before merging PRs into Jest. I think this is a large task and we are considering a new API surface that patches over existing shortcomings of the mocking system. I'd also like @calebmer to be involved in this conversation.

Here is my concrete feedback for the current proposal:

• jest.requireActual sounds good, as previously discussed, but we need jest.requireMock. I would like to see a codemod run at FB for this.
• jest.getMock – in the PR, it returns MockData but getMock implies that the mock itself is return. What is a mock, on its own? I'm not sure if the API is clear. What about adding this to jest-mock and making it importable? const mock = require('jest-mock'); mock.callsFor(mockFn) etc.? That way we stop polluting the jest object with mocking features and it will work when used separately, outside of Jest.
• jest.stub is dangerous to me. Putting the naming aside, it doesn't do anything to make flow typing of tests better, it's basically just like a // $FlowFixMe embedded in the function call. jest.stub, similar to jest.fn will mock out a method on a class/class instance but it doesn't guarantee anything about the return types of the mock function. jest.stub(foo, 'bar') itself changes the return type of the function to undefined (if it wasn't undefined before), therefore you get not added benefit from flow-typing that test code, except flow swallowing the error. I think this actually needs some support in Flow.

What do you all think about these considerations?

cc @calebmer

[elicwhite]

Here is some additional context from a conversation I had with @aaronabramov offline.

I think there are two distinct pieces to "make mocking work with flow". One is to let flow work with the original code and have no concept of what the implementation of our mocks are. That will cause Flow to fail when the mocks are being used in tests in a way that is inconsistent with the original code. Using assignments to mock things currently confuses Flow.

The other is leverage flow to enforce that mocks follow the same signature as the thing being mocked. This will ensure that original code calling functions that are being mocked will be tested closer to how they are run in prod.

While both are extremely valuable, @aaronabramov thought a reasonable course of action would be to tackle the first one in the short term since tackling the second would require a larger overhaul of mocking in Jest, almost guaranteed to be backwards incompatible, and likely require changes in Flow to support it.

[cpojer]

Yeah, personally I don't think that adding more type-unsafe APIs is in any way unblocking us from flow-typing tests. While this API can be added, and then flow types can be used for tests, they do not really provide any more value over the status quo or adding // $FlowFixMe comments and don't add any type safety. I don't think such an API is a good candidate to increase Jest's API surface at this point. If you'd like to add an API like this at FB in the short term, I think it should be a custom FB API in a setup file, rather than an API within Jest core. That API should be explicit, something like fbUnsafeMockFunction. Before committing to such workarounds, I would like to see a more complete plan about real flow-typed mocks and what kind of strategy we have to build this into Jest so that we know that this unsafe mocking is temporary and won't stick around for years to come.

[aaronabramov]

i agree that introducing short-term not type safe API is probably not the best thing to do. I started thinking about stricter flow types for mocks, here's what i got so far:

// @flow

const obj = {
  // original function that returs a 'string'
  a: (): string => 'str'
};

type MOCK<TRETURN> = {
  mockImplementation(() => TRETURN): void,
}

const mockObjectMethod = (obj: Object, fn) => {
  // We need to get the return type of the original function,
  // I don't know how to do it, there should be some kind of
  // $GetFunctionReturnType<fn> utility type or something.
  // But for now i hacked it with just executing function and getting
  // its return type from returned value;

  // flow will think this variable is always true
  let flowThinksThisVarIsAlwaysTrue: true = true;
  // $FlowFixMe but we swap it without flow knowing
  let flowThinksThisVarIsAlwaysTrue = false;

  // flow thinks that we execute the function and get its return type,
  // but in runtime we'll never do it!
  const returnValueTypeContainer = flowThinksThisVarIsAlwaysTrue && fn();

  // const mock: MOCK<typeof returnValueTypeContainer> = {
  //   mockImplementation(mockFn) {/* replace real function with mockFn */}
  // }
  return {
    mockImplementation(mockFn: () => typeof returnValueTypeContainer) {
      /* actually replace the original function with a mock function */
    }
  };
}

const mock = mockObjectMethod(obj, obj.a);

mock.mockImplementation(() => 'str');
// $FlowExpectError number is passed instead of string
mock.mockImplementation(() => 555);
// $FlowExpectError boolean is passed instead of string
mock.mockImplementation(() => true);

this only types the return value. I'm not sure how to type arguments of the mock function. most of the time we create a mock function that takes no argument, because we already have a pre-defined response. So my thought were to make the whole arguments array optional (if you pass at least one argument to a mock function, you should have all of them strictly typed, if none are passed, nothing is flowtyped)

[jamiebuilds]

For module mocking, what if Jest just told you to import your mocks directly?

For example, with the source module methods.js:

export function foo() {
  return 'foo';
}

And the mock module __mocks__/methods.js:

import * as methods from '../methods';
const mockMethods = jest.generateMock(methods);

let fooMock = 'foo';
mockMethods.foo = () => fooMock;
mockMethods.__setFooMock = val => { fooMock = val };

jest.setModuleMock('../methods', mockMethods);
export default mockMethods;

You would consume the mock in __tests__/methods.js by importing the mock directly.

import methods from '../__mock__/methods';
methods.__setFooMock('bar');

By importing the mocks directly, you can create the correct Flow/TS types, you can also use the import to __mock__ to setup the module mock to be used everywhere jest.setModuleMock(filePath, mockedModule)

[elicwhite]

I think flow is currently happy with mock modules that exist in __mocks__ directories simply because it doesn't know about them.

If you have module source.js which contains

export function foo() {
  return 'foo';
}

and you have __mocks__/source.js that contains a module with different function signatures, since flow doesn't have a concept of mocks, when your test requires source.js, flow thinks you have the original file whereas jest gives you the mock. So this forces you to work with the flow types of the original source files, essentially ensuring that the mock function signatures match and that it doesn't export more functions.

If that is sufficient for full module mocking defined in other files, I think the bigger question at hand in this task is how to write mocks for functions / modules inside of the test file.

[jamiebuilds]

I think we need to get in the habit of doing stuff like:

let warn = jest.stub(console, 'warn');
warn.mock.calls...

Where jest.stub has the type of:

<Obj, Prop>(obj: Obj, prop: Prop) => $PropertyType<Obj, Prop> & { mock: {...} };

And for non-objects:

let stub = jest.fn();
eventEmitter.on('event', stub);
stub.mock.calls...

Or:

let stub = jest.fn((a: number, b: number) => a + b);

Where the type of jest.fn is:

& () => ((() => void) & ({ mock: {...} }))
& (F) => (F & ({ mock: {...} }))

[elicwhite]

Yeah, exactly. I believe that is Aaron's proposal. It's also very consistent with how sinon stubbing works but not how jests current API works.

However, what we really want to guarantee is that if you have a source file:

module.exports = {
   foo(arg1: string) : number {
      return 4;
   }
}

And in a test we do something like this:

jest.stub(sourceModule, 'foo', (arg1: string, arg2: string) : boolean => { return true; });

We believe we want flow to complain that this mock doesn't have the same signature as the function being mocked. I'm not sure how we can accomplish that.

[elicwhite]

Oh, upon rereading your comment, perhaps that flow type will work. We should give it a try! :-)

[elicwhite]

I tried this out in the REPL and couldn't get the $PropertyType call to work. Any ideas?

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjOA7AzgFzAFsBPAWTgBMBXGAUzAF4wBvVMMKOOACgEMATgHMAjAC4wBAQEtsQgJQTs1IgCM6A1uzABIAXXzUB2MACYA3NoC+qG5hwEwAKzqPmbDgWqqAPAHlVJwkAlwx8ABowAAUBOAAHCSlZIQA+Hm0dOEDgwMiMogMACyoJGPi8jh1pIjj6Aux8PnxpHAkAEjK4jXwSABUSLv9c6Ni4lO15LQ4OLKcAbQL8YsoAXSYwatq6esbmnEsOGytLVGBgMF7CjQZcYtpKMHUwPhMNWM0cMCWGLhh4BGSYBgsgY6gwfGouAY3zAp3OmzqdAaTRaJkocFcYGwcEIRCaGEKXyukmkQmwTSMDDgUGI5CotDoADouHBUC4CIyvKoeKQKDR6JEAOQswWRHiTRgpVhWeTmMBAA

const myModule = {
  foo(arg1: string): number {
    return 2;
  }
}

const jest = {
  stub<Obj: Object, Prop: string>(
    obj: Obj, 
    method: Prop, 
    implementation: $PropertyType<Obj, Prop>
  ) {
    obj[method] = implementation;
  }
};

13:     implementation: $PropertyType<Obj, Prop>
                       ^ expected object type and string literal as arguments to $PropertyType

[jamiebuilds]

Oh right, $PropertyType wants you to put the string literal inline: $PropertyType<Obj, 'prop'>. I wonder if the Flow team could help out with this to improve Jest. @mroch @samwgoldman ?

[SimenB]

The OP mostly talks about API. We now have both jest.requireActual and jest.spyOn. Does that mean this can be closed? Or do we want it to track some sort of type support for mocks as well?

/cc @aaronabramov

[aaronabramov]

i think there are still some unresolved issues ,but generally this little function

export const getMock = (fn: Function): JestMockFn<any, any> => {
  if (!fn._isMockFunction) {
    throw new Error('Passed function is not a mock');
  }
  return fn;
};

got me 80% there so i think this might be closed since i don't think we're planning any work on that any time soon :)

[github-actions[bot]]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[SimenB]

We have jest.mocked which solves the use case for TypeScript I think. Approach can probably be copied for Flow as well. jest.fn<typeof console.log>() also works

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.