-
-
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
chore: add type tests for Mock Function #12459
Conversation
mockImplementation(fn: (...args: Y) => T): this; | ||
/** @internal */ | ||
mockImplementation(fn: () => Promise<T>): this; | ||
mockImplementationOnce(fn: (...args: Y) => T): this; | ||
/** @internal */ | ||
mockImplementationOnce(fn: () => Promise<T>): this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These overloads are useful only internally. Removing them from public types simply cleans up error message. Without @internal
marks, if return type is string
, the error message tells that string
or Promise<string>
is expected type. Even more, if return type is Promise<string>
, TS is expecting to see Promise<string>
or Promise<Promise<string>>
. I added type test with promises to make sure that these overloads can be safely erased.
* Undefined when type === 'incomplete'. | ||
* Result of a single call to a mock function that returned. | ||
*/ | ||
value: T; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be I misunderstood something, but if these TSDoc comments are placed above type
, they don’t show up in IDE. That’s how they are placed in @types/jest
. So I was trying to find some place where they could be useful.
isMockFunction<T, Y extends Array<unknown> = Array<unknown>>( | ||
fn: SpyInstance<T, Y>, | ||
): fn is SpyInstance<T, Y>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spy went missing (;
@@ -686,6 +703,7 @@ export class ModuleMocker { | |||
// NOTE: Intentionally NOT pushing/indexing into the array of mock | |||
// results here to avoid corrupting results data if mockClear() | |||
// is called during the execution of the mock. | |||
// @ts-expect-error reassigning 'incomplete' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous type was working good internally, but it did not allow narrowing if (returnValue.type === 'return')
. Unfortunately after refactor TS is convinced that type
here is 'incomplete'
and does not allow reassigning. I was wrestling with this problem, but could not find a solution.
Co-authored-by: Simen Bekkhus <[email protected]>
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
As agreed #12442 (comment), I moved
jest.fn
andjest.spyOn
type tests tojest-mock
. Also added tests for all Mock Function methods and props. Took some time to understand some details. To make it all work I had to refactorMockFunctionResult
type.Test plan
Current tests and new tests should pass.