Spied-on functions that return Promises change their mock return values when the Promise resolves

[beckjake]

Describe the bug

If a spied-upon function returns a promise, the .mock.results[...].value item starts off as a promise, but when the promise resolves it becomes the resolved item. This behavior doesn't seem to be documented anywhere.

I found this while migrating an existing test suite from jest, and I know this might not be the most desirable test pattern (I will simply change it to not expect this), but I found this behavior pretty surprising.

It looks like it might have been introduced in #2835.

Reproduction

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

Note that you can add any amount of indirection you want here, and it'll behave the same way. So the issue isn't that I'm calling bar and it's spied upon - I originally discovered this issue in a test that spied on bar, called a function that called bar, and examined the mock's result to make sure it was a promise (and then awaited it to examine the result, etc.).

System Info

System:
    OS: macOS 13.4
    CPU: (8) arm64 Apple M1 Pro
    Memory: 317.55 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.5.0 - ~/.nvm/versions/node/v18.5.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.5.0/bin/yarn
    npm: 8.12.1 - ~/.nvm/versions/node/v18.5.0/bin/npm
  Browsers:
    Chrome: 114.0.5735.133
    Safari: 16.5
  npmPackages:
    vitest: ^0.32.2 => 0.32.2

### Used Package Manager

yarn

### Validations

- [X] Follow our [Code of Conduct](https://github.com/vitest-dev/vitest/blob/main/CODE_OF_CONDUCT.md)
- [X] Read the [Contributing Guidelines](https://github.com/vitest-dev/vitest/blob/main/CONTRIBUTING.md).
- [X] Read the [docs](https://vitest.dev/guide/).
- [X] Check that there isn't [already an issue](https://github.com/vitest-dev/vitest/issues) 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](https://github.com/vitest-dev/vitest/discussions) or join our [Discord Chat Server](https://chat.vitest.dev).
- [X] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.

[stackblitz]

Start a new pull request in StackBlitz Codeflow.

• vitest-dev-vitest-mk9jek (Open reproduction)

[sheremet-va]

This behavior is different from jest's, but it is expected. Not sure exactly what you want to document, but feel free to open a PR.

[ITenthusiasm]

@sheremet-va Is there a way for a developer to inspect the original promise? I previously found it helpful to be able to inspect the original Promise.

[ITenthusiasm]

Eh. The use case is because I needed to check whether or not a Promise had resolved. I'm guessing in vitest land the way to do that is to verify that the value is a Promise before resolution and verify that it's a resolved value after resolution?

If that's the case, maybe it isn't necessary to preserve the original. That said, an update to the Migration guide could also be helpful. (Can you tell I'm in the middle of migrating from Jest yet? 😅)

[sheremet-va]

I don't know. Maybe semantically it's better to not resolve a promise because this is what the actual function return value. So I would consider it a bug. To fix this, it's better to open a separate issue in tinyspy.

[ITenthusiasm]

I feel like you could argue either way. Originally I was a little upset with the implementation. But in the long run, this implementation could be considered advantageous -- as long as it's clearly documented.

The disadvantage of leaving returned Promises alone is that whenever developers want to inspect the details of the Promise, they have to create their own extension of the Promise and overwrite the global Promise object. Then, when the test is finished, they have to restore the original implementation of the global Promise so that unexpected accidents don't happen. This is... a more convoluted development experience. So the current way Vitest works seems like a better DX.

The current way Vitest works simplified my codebase for the aforementioned reason. (I don't have to inspect whether or not the Promise is already resolved. If I have the raw value, I know it resolved. If I still have the Promise, I know it didn't resolve.) If you decide you want to change things anyway to be more like Jest though, I would understand.

[sheremet-va]

We now document this difference in Vitest documentation so I think it's safe to close this issue.

[ITenthusiasm]

Thanks for keeping the current implementation. 🙏🏾 I like what Vitest does so much better.