Skip to content
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

Class mocks break when using mockReset / resetAllMocks #6948

Open
6 tasks done
simon-abbott opened this issue Nov 22, 2024 · 0 comments
Open
6 tasks done

Class mocks break when using mockReset / resetAllMocks #6948

simon-abbott opened this issue Nov 22, 2024 · 0 comments

Comments

@simon-abbott
Copy link
Contributor

Describe the bug

#4564 made class methods' mock instances independent, but it overlooked some important edge cases:

  1. Resetting an automocked constructor causes calling that constructor to no longer isolate its called members
  2. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant