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

fix: make automocking aware of symbol indexed methods #1119

Merged
merged 2 commits into from Apr 7, 2022

Conversation

eleith
Copy link
Contributor

@eleith eleith commented Apr 7, 2022

this PR intends to resolve #1082 by updating automocking to fully support symbol indexed methods (specifically, fixing the root issue which occurs when trying to restore and later use such methods)

caveat

in a fresh project, i can get the below test snippet to fail for vitest@0.9.0. i can then get them to pass when linking vitest locally to this branch. however, when running the tests in examples/mocks, the below tests fail as the mock is not properly restored.

local testing

in a fresh project, the following will fail for v0.9.0 and pass on this PR

in log.ts

const sym = Symbol('x')

const logger = {
  warn() {
    return this[sym]()
  },
  [sym]() {
    return 'hello'
  }
}

export default logger

and in log.test.ts

import { vi, test, expect } from 'vitest'
import logger from './log'

vi.mock('./log')

test('auto mocked', () => {
  expect(logger.warn()).toBeUndefined()
})

test('restoring auto mock', () => {
  vi.restoreAllMocks()
  expect(logger.warn()).toBe('hello')
})

@netlify
Copy link

netlify bot commented Apr 7, 2022

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit b2b2fdd
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/624efadc7b1cfc0009fd1e2f
😎 Deploy Preview https://deploy-preview-1119--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@eleith
Copy link
Contributor Author

eleith commented Apr 7, 2022

@antfu could you push a new updated build to tinyspy?

i can confirm when installing locally that v0.3.1 does not include your latest changes for symbol support.

@sheremet-va
Copy link
Member

@antfu could you push a new updated build to tinyspy?

i can confirm when installing locally that v0.3.1 does not include your latest changes for symbol support.

released in 0.3.2

@eleith eleith changed the title making automocking aware of symbol indexed methods fix: making automocking aware of symbol indexed methods Apr 7, 2022
@eleith eleith changed the title fix: making automocking aware of symbol indexed methods fix: make automocking aware of symbol indexed methods Apr 7, 2022
@eleith
Copy link
Contributor Author

eleith commented Apr 7, 2022

on both windows and in a clean project directory (even linux) all tests in this PR pass with only a single call to vi.restoreAllMocks()

however when running tests in examples/mocks on linux (14.x and 16.x) the tests in this PR will fail unless the auto mocks are manually restored by calling .mockRestore()

when inspecting the value of spies in the internals of vi.restoreAllMocks, in a failed test run, it is incorrectly an empty Set().

in a successful test run, spies is a full Set() of all automocks.

@eleith eleith marked this pull request as ready for review April 7, 2022 08:53
@eleith eleith requested a review from sheremet-va April 7, 2022 15:01
@antfu antfu merged commit 06faf8a into vitest-dev:main Apr 7, 2022
@eleith eleith deleted the fix-pino-mocking branch April 7, 2022 18:12
chaii3 pushed a commit to chaii3/vitest that referenced this pull request May 13, 2022
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

automocking fails in some cases when implementation involves symbols
3 participants