Allow use with mocks created by spyOn

[RylanBueckert-Broadsign]

In jest-when it is possible to use when mocks on spied functions. It would be very nice if we could do the same here.

Simple Example to demonstrate what should be possible

test('Example', () => {
  class FooClass {
    bar(arg1: string) {
      return 'called with ' + arg1;
    }
  }

  const foo = new FooClass();

  const barSpy = vi.spyOn(foo, 'bar');

  when(barSpy).calledWith('baz').thenReturn('mocked return value');

  expect(foo.bar('baz')).toBe('mocked return value');
  expect(foo.bar('buzz')).toBe('called with buzz');
});

There is an type error when using when in this example:
Argument of type MockInstance<(arg1: string) => string> is not assignable to parameter of type AnyFunction Type MockInstance<(arg1: string) => string> provides no match for the signature (...args: never[]): unknown

If I simply ignore the error and run the test, the when mock does actually work when calling with the specified parameters, but it returns undefined instead of the original behavior when the params do not match.

[mcous]

Hey @RylanBueckert-Broadsign, thanks for this request!

There is an type error when using when in this example

Nice catch! This should be a fairly easy update to the typings to support this. I should be able to address it shortly.

the when mock... returns undefined instead of the original behavior when the params do not match.

I did a little digging, and Jest and Vitest have some important differences that make this feature difficult to implement in vitest-when:

const spy = vi.spyOn(foo, 'bar');
const impl = spy.getMockImplementation();

In jest, impl will be the original, non-mocked implementation. In vitest, it will be undefined. This means vitest-when has no easy access to the original function, so we've got nothing to fall back to from our own mock implementation that adds the argument matcher. Unless Vitest changes this behavior, I don't think this feature can be implemented in vitest-when.

To be honest, though, even if there wasn't a technical blocker, I'm not super jazzed about supporting vi.spyOn, especially with a fallback to the original function. vi.spyOn creates partial mocks, which I consider a big mocking anti-pattern.

In my opinion, mocks should be used to help you design APIs that are easy to use so you can write code that's easy to test. Using spyOn is the opposite: it's using mocks to pretend that your code isn't easy to test. I'd recommend you avoid using it, and I am very hesitant to add features to vitest-when that make it easier to use partial mocks.

[RylanBueckert-Broadsign]

Understand that calling the original non-mocked version may not be simple, but what about falling back to the existing mock implementation? For example:

let barSpy: Mocked<typeof foo.bar>;

beforeEach(() => {
  barSpy = vi.spyOn(foo, 'bar');
  
  barSpy.mockReturnValue('default mocked return value');
});


test('Example', () => {
  when(barSpy).calledWith('baz').thenReturn('mocked return value');
  
  expect(foo.bar('buzz')).toBe('default mocked return value');
});

This also currently returns undefined if I remember correctly.

Sometimes vi.spyOn is much easier or maybe the only way (afaik) to mock in some situations. For example what if I want to use when to mock a function on a global object like document.getElementById? There is no module that can be mocked via vi.mock and even if there was, I may not want to mock all of document. I am not familiar with a way to achieve this without using a spy like vi.spyOn(document, 'getElementById').

[mcous]

Understand that calling the original non-mocked version may not be simple, but what about falling back to the existing mock implementation? For example:

I'm amendable to this change! Feels pretty consistent with Vitest in general and provides an interesting place to provide a default fallback without increasing the API surface area of the vitest-when. I will give this a shot.

Sometimes vi.spyOn is much easier or maybe the only way (afaik) to mock in some situations. For example what if I want to use when to mock a function on a global object like document.getElementById?

My take here is a little spicy depending on one's experience with using mocks, but can be summed up as "you shouldn't mock that out (directly)." I assume that sounds super unhelpful! So, if you're curious, a little essay/rant:

Most mock usage in the wild is to fake out something that's hard or inconvenient to test - like knocking out document.getElementById. I think this is a "poor" use of mocks. Mocks really shine if you use them to fake out pieces of your own code that you haven't yet written. Mocks allow you to iterate on the API portion of the design in tests without actually implementing your lower level logic, so API change is cheap. Once you've written your mock-based tests and settled on internal APIs, you can then implement whatever logic you need in those internals. It's sort of a top-down test-driven approach to building your code.

Outside of that specific use-case in a TDD flow, I don't think one should really use mocks, because they cost so much in terms of coupling your test cases to your code under test. They also don't really test that your code works in the real world - because you've got a bunch of take things in play. If you're not getting useful structural design feedback of your internals, I haven't found the cost of mocks to be worth it.

This idea can roughly be encapsulated in the advice "Don't mock what you don't own". If you're designing some internal logic that needs getElementId to function, I'd recommend one or both of:

• Structure your code so your important logic doesn't call getElementById directly, and you can avoid testing with mocks
• Wrap up the document.getElementById in your own module, mock out your module

For more on this style of mocking, I think this talk is a fantastic introduction, and the concepts it touches on really leveled me up in terms of how I approach testing my software and teach my teams how to test!

[RylanBueckert-Broadsign]

I do get your point. In this case document.getElementById was just an example of something that could be difficult to mock without using spyOn. It was mostly just to point out that such cases can exist, and in those cases, vitest-when would not be able to be used.

The fact that when currently cannot be used with spyOn is what caused me to realize that my team may not be using mocks correctly in our tests. The issue is that we have overused spyOn in the past, so when we are migrating from jest and jest-when to vitest and vitest-when, we have to make additional changes to migrate those tests.

I do appreciate your explanation, and I will watch that talk. Thanks for maintaining this library.