Drop jest test environment in favor of vitest #3884
Conversation
nishitmistry
force-pushed
the
drop-Jest-test-environment-in-favor-of-Vitest
branch
from
7f9a9ce to
49b971e
Compare
eemeli
left a comment
eemeli
left a comment
There was a problem hiding this comment.
A first-pass review, see inline for fixes.
@nishitmistry Please note that I'll be on vacation from the end of this week until the end of January, so probably won't be available for reviews until early February.
Also, we have a Matrix channel #pontoon:mozilla.org you could consider joining?
translate/src/test/utils.jsx
Outdated
| return Object.defineProperty(window, 'matchMedia', { | ||
| writable: true, | ||
| value: (query) => ({ | ||
| value: vi.fn().mockImplementation((query) => ({ |
There was a problem hiding this comment.
Couldn't we do something like this instead? Also, could we confirm that mocking this is even necessary anymore?
vi.spyOn(window, 'matchMedia').mockImplementation(() => ({ ... }));There was a problem hiding this comment.
unfortunately this does not work i guess its because jsdom still haven't implemented matchMedia mock jsdom issue and spyOn cries when it receives undefined instead of a function.
i have changed this to mock using vi.stubGlobal which does same thing but cleaner.
the mock is indeed necessary, but thing is matchMedia is not being called in any test so removing this makes no difference. i have removed the mockMatchMedia calls but have kept the util function.
nishitmistry
force-pushed
the
drop-Jest-test-environment-in-favor-of-Vitest
branch
from
dd504ab to
12eee31
Compare
nishitmistry
force-pushed
the
drop-Jest-test-environment-in-favor-of-Vitest
branch
from
12eee31 to
97986ef
Compare
|
Hey @eemeli thanks for the heads up regarding your vacation i will try to create the sinon migration PR asap, i have joined Matrix channel and i guess all the future queries should be addressed directly in the channel or Dms, right :) ? |
eemeli
left a comment
eemeli
left a comment
There was a problem hiding this comment.
Other than the one inline suggestion, this looks good to merge.
i have joined Matrix channel and i guess all the future queries should be addressed directly in the channel or Dms, right :) ?
Feel free to continue with whatever seems most appropriate for any given query. Probably better to use the channel or comments/issues on GitHub as a default; while I'm out (and also otherwise!) @mathjazz is actually our main Pontoon developer, and there are also others who might be able to answer any questions you have.
translate/src/test/utils.jsx
Outdated
| * | ||
| * Source: https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom | ||
| */ | ||
| export function mockMatchMedia() { |
There was a problem hiding this comment.
If/as this isn't used by anything anymore, we should remove it.
2 participants
In continuation of the previous PR #3880 for task #3768
@eemeli this PR marks the completion of jest to vitest migration but alot of sinon mock are still present so should i start the migration of sinon mock to vitest under #3768 task ?
this PR will be ready to review once #3882 is merged