Mocking changes returned promises

miguel-leon commented 7 months ago

Describe the bug

A promise returned by a mocked function or methods, changes a returned promise. i.e., saving the promise in a variable before returning it and then comparing it to what's returned outside the function with === evaluates to false.

I can't find documentation for this and it doesn't happen with jest, so I suppose it's unintended.

Code from the StackBlitz link just in case:

class Foo {
    x?: Promise<void>;

    bar() {
        return this.x = Promise.resolve();
    }
}

Foo.prototype.bar = vi.fn(Foo.prototype.bar);

test('should work', async () => {
    const foo = new Foo();
    const y = foo.bar();
    expect(foo.x === y).toBe(true); // errors
});

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-zkjejn?file=src%2Fbasic.ts

System Info

irrelevant

Used Package Manager

npm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

sheremet-va commented 7 months ago

This is a documented difference from Jest: https://github.com/vitest-dev/vitest/issues/3634

I am open to discuss the change to how it works.

miguel-leon commented 7 months ago

Oh I couldn't find it in the mocking docs. I wasn't doing a migration and didn't look there, I just mentioned Jest to constrast. Although it took me some time to find out it was the mock changing the expected behavior of the function. Perhaps consider adding a warning in the mocking guide?

Heeding your openness to discussion, My use case is that If I have (due to user interaction) subsequent async calls (fetch), besides doing stuff with all awaited values, I also have to, say, update the UI only in response of the user's last interaction. I accomplish this by saving the last promise, and after awaiting, compare the awaited promise with the last saved. This ensures doing the work on the last interaction even if earlier promises resolve later.

I was testing this UI update and it wasn't happening due to the promises being changed by a mock.

In order for these kind of tests to work with vitest, the developer would have to carefully consider what to mock/spy, or to extract parts of the implementation returning promises into unmocked functions.

miguel-leon commented 7 months ago

I just read the migration guide Accessing the Return Values of a Mocked Promise, but I think that even after reading it, I would have not been able to extrapolate that it means Vitest would change the returned promise of a mocked function.

I understand it is perhaps a rare case to check up and compare promises directly (instead of await/then), but it feels like if the framework is changing the behavior of the code, it should be very explicit about it.

Thanks for understanding.

vitest-dev / vitest