-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[RFC] flow compatible mocking API #4257
Comments
What about an API like this instead:
Similarly,
I wonder if you could implement that simply by making |
@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:
|
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:
What do you all think about these considerations? cc @calebmer |
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. |
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 |
i agree that introducing short-term not type safe API is probably not the best thing to do. // @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. |
For module mocking, what if Jest just told you to import your mocks directly? For example, with the source module export function foo() {
return 'foo';
} And the mock module 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 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 |
I think flow is currently happy with mock modules that exist in If you have module
and you have 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. |
I think we need to get in the habit of doing stuff like: let warn = jest.stub(console, 'warn');
warn.mock.calls... Where <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 & () => ((() => void) & ({ mock: {...} }))
& (F) => (F & ({ mock: {...} })) |
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:
And in a test we do something like this:
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. |
Oh, upon rereading your comment, perhaps that flow type will work. We should give it a try! :-) |
I tried this out in the REPL and couldn't get the
|
Oh right, |
The OP mostly talks about API. We now have both /cc @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 :) |
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. |
We have |
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. |
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:
there's also a set of functions that is defined on the mock, that we can alias somewhere too
like
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 sincejest.mock
is already taken,jest.stub
might be our best option.cc @TheSavior @dalongi
The text was updated successfully, but these errors were encountered: