-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Closed
Labels
feat: module mockingIssues related to code with vi.mockIssues related to code with vi.mockp2-to-be-discussedEnhancement under consideration (priority)Enhancement under consideration (priority)pending triage
Description
Describe the bug
#4564 made class methods' mock instances independent, but it overlooked some important edge cases:
- Resetting an automocked constructor causes calling that constructor to no longer isolate its called members
- Resetting/overriding the implementation of an isolated function detaches it entirely from the parent, causing the prototype method to no longer register mock calls
This is causing a lot of headaches while I'm trying to update our version of Vitest past 1.0.0 as we used prototypes and resetAllMocks
all over the place.
The second point I can forego a fix for if necessary, since I can rewrite most of our tests using instance accessing instead of prototypes (i.e. foo.bar
instead of Foo.prototype.bar
), but the first point is really painful, and both are very counterintuitive.
Reproduction
// file: src/class.ts
export class TestClass {
public doAThing() {
return 12;
}
}
// file: src/__tests__/class.test.ts
import { expect, test, vitest } from "vitest";
import { TestClass } from "../class.js";
vitest.mock("../class.js");
test("isolation", () => {
const inst1 = new TestClass();
// Normally, using the prototype counts the same calls as the instance.
void inst1.doAThing();
expect(inst1.doAThing).toHaveBeenCalledTimes(1);
expect(TestClass.prototype.doAThing).toHaveBeenCalledTimes(1);
// However, mocking the implementation of the instance detaches calls from the
// prototype.
vitest.mocked(inst1.doAThing).mockReturnValue(21);
void inst1.doAThing();
expect(inst1.doAThing).toHaveBeenCalledTimes(2);
expect.soft(TestClass.prototype.doAThing).toHaveBeenCalledTimes(2);
// Resetting mocks should restore back to the original behavior, but it doesn't:
vitest.resetAllMocks();
void inst1.doAThing();
expect(inst1.doAThing).toHaveBeenCalledTimes(1);
expect.soft(TestClass.prototype.doAThing).toHaveBeenCalledTimes(1); // This is once again decoupled.
// And now the constructor shim is also broken, so these two instances are no longer isolated.
const inst2 = new TestClass();
const inst3 = new TestClass();
expect.soft(inst2.doAThing).not.toBe(inst3.doAThing);
expect.soft(vitest.mocked(inst2.doAThing).mock).not.toBe(vitest.mocked(inst3.doAThing).mock);
void inst2.doAThing();
expect(inst2.doAThing).toHaveBeenCalledTimes(1);
expect.soft(inst3.doAThing).toHaveBeenCalledTimes(0);
});
System Info
Irrelevant
Used Package Manager
npm
Validations
- Follow our Code of Conduct
- Read the Contributing Guidelines.
- Read the docs.
- Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
- The provided reproduction is a minimal reproducible example of the bug.
Metadata
Metadata
Assignees
Labels
feat: module mockingIssues related to code with vi.mockIssues related to code with vi.mockp2-to-be-discussedEnhancement under consideration (priority)Enhancement under consideration (priority)pending triage
Type
Projects
Status
Has plan