-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
IrrelevantUsed 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.
Reactions are currently unavailable
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