Migrate more jest tests to vitest - #1607
Open
zetter-rpf wants to merge 12 commits into
Open
Conversation
zetter-rpf
temporarily deployed
to
previews/1607/merge
August 21, 2026 15:34 — with
GitHub Actions
Inactive
These 5 files were the first candidates blocked by literal jest.fn/ jest.mock usage rather than the JSX-in-.test.js issue earlier migrations hit. Swapping jest.fn -> vi.fn and jest.mock -> vi.mock was enough for 4 of the 5, but MotionInput.test.jsx's assertions on call counts leaked across tests within the same describe block, because Vitest's mocks aren't reset between tests by default the way Jest's resetMocks: true config resets jest.fn(). Added the equivalent mockReset: true to vite.config.js's test block so Vitest mocks behave the same way. Confirmed via full Jest and Vitest runs: same 1006 tests pass before and after (832 Jest + 174 Vitest).
Converting jest.mock/jest.fn to vi.mock/vi.fn was enough for the file-saver, jszip and scratchIframe mocks, but the jszip-utils factory mock needed an explicit default export. Jest's CJS interop treats a factory mock with no __esModule flag as the default export as a whole, so `getBinaryContent: jest.fn()` was reachable via the default import in the component. Vitest doesn't apply that interop to mock factories, so the component's `import JSZipUtils from "jszip-utils"` resolved to undefined until the factory returned an explicit `default` key. Confirmed via full Jest and Vitest runs: same 1006 tests pass before and after (822 Jest + 184 Vitest).
EditorPanel, ErrorMessage and Output passed unmodified. EditorInput and HtmlRenderer both mock react-responsive with a factory referencing jest.requireActual, ported to an async factory using vi.importActual. Getting EditorInput running under Vitest surfaced two setupTests.vitest.js gaps shared by the whole Editor tree: window.matchMedia isn't implemented by jsdom by default (src/utils/settings.js calls it at import time), and @raspberrypifoundation/python-friendly-error-messages's dist build doesn't resolve under Vitest's module runner the way Jest's CJS require() tolerates. Both are now mocked/polyfilled globally in setupTests.vitest.js, matching their equivalents in the Jest-only setupTests.js. Six more Editor/Runners files were tried and left on the Jest list: Project.test.jsx (module-level jest.useFakeTimers() combined with this setup produces an empty render under Vitest - needs deeper investigation), ScratchContainer.test.jsx and HtmlRunner.test.jsx (both call window.localStorage, which is undefined under this Vitest+jsdom+Node 22 combination - jsdom's own localStorage never gets bridged onto the global object), PyodideRunner.test.jsx and PythonRunner.test.jsx (both import app/store, which constructs an oidc-client UserManager that reads localStorage at import time, hitting the same gap), and PyodideWorker.test.js (dynamically re-imports the worker script per test via vi.resetModules()/import(), but the script's `new TextEncoder()` call fails post-reset in a way its Jest require()-based equivalent doesn't). Confirmed via full Jest and Vitest runs: same 1006 tests pass before and after (785 Jest + 221 Vitest).
Previously ContextMenu, FileMenu, and the ten Sidebar test files (DownloadPanel, FilePanel, InstructionsPanel, ProgressBar, ProjectsPanel, SettingsPanel, ThemeToggle, Sidebar, SidebarBar, SidebarBarOption) only ran under Jest. This change swaps jest.fn/jest.mock for vi.fn/vi.mock across the batch. DownloadPanel needed the jszip-utils factory mock to return an explicit default export, matching the fix already applied to DownloadButton, since Vitest does not apply Jest's CJS default-export interop to mock factories. InstructionsPanel's "adds the demo instructions" test asserted the demo markdown had been replaced by Jest's jest-transform-stub, which returns the filename for any non-JS asset. Vitest's `?raw` import loads the real file content instead, so the assertion now compares against populateMarkdownTemplate(demoInstructions, ...) to match the component's actual behaviour rather than a Jest-only stand-in. Confirmed via full Jest and Vitest runs: 616 Jest + 435 Vitest tests pass, the same total as before the move.
Previously MobileProject, ErrorModal, and GeneralModal only ran under Jest. This change swaps jest.fn for vi.fn across all three; none used jest.mock or other Jest-only APIs, so no other changes were needed. Confirmed via full Jest and Vitest runs: 600 Jest + 451 Vitest tests pass, the same total as before the move.
… tests Previously ProjectBar, ScratchProjectBar, ProjectName, SaveButton, SaveStatus, and UploadButton only ran under Jest. This change swaps jest.fn/jest.mock/jest.useFakeTimers/jest.spyOn for their vi equivalents, and jest.requireActual for the async vi.importActual pattern already used for react-responsive. Rendering SaveStatus (directly, and via ProjectBar/ScratchProjectBar) crashed under Vitest because react-i18nexts real useTranslation returns an uninitialised i18n instance - nothing calls i18n.init() in a component test, unlike the full app - so SaveStatus reading i18n.options.fallbackLng threw. setupTests.vitest.js now mocks react-i18next globally the same way setupTests.js already does for Jest, which fixes this for any test rendering SaveStatus, not just this batch. ProjectNames "Updates project name" tests asserted store.getActions()).toEqual([updateProjectName(project.name)]), which only ever passed under Jest because its useDispatch mock returns a dispatch thats disconnected from the mock store, so getActions() is always [], and Jests toEqual treats [] and [undefined] (the auto-mocked action creators return value) as equal - a quirk Vitest doesnt share. Replaced with the equivalent, more direct expect(updateProjectName).toHaveBeenCalledWith(...) assertion already used elsewhere in the same file. StopButton.test.jsx was tried and left on the Jest list: it imports app/store, hitting the same oidc-client/localStorage gap under Vitest+jsdom+Node 22 documented for the Editor/Runners files. Confirmed via full Jest and Vitest runs: 510 Jest + 541 Vitest tests pass, the same total as before the move.
The react-i18next mock added in the previous commit already has its rationale in that commits message; the inline comment restating it was redundant.
Previously WebComponentProject.test.jsx and runEventCodeSnapshot.test.js only ran under Jest. This change swaps jest.fn/jest.useFakeTimers/jest.advanceTimersByTime for their vi equivalents. Rendering either file crashed under Vitest because src/utils/i18n.js imports initReactI18next from react-i18next, which the react-i18next mock added in the previous commit did not export - it only replaced useTranslation and Trans. setupTests.vitest.js now also mocks ./i18n itself the same way setupTests.js does for Jest, so the real i18n.js (and its initReactI18next usage) never runs in a component test. WebComponentProject.integration.test.jsx and src/containers/WebComponentLoader.test.jsx were tried and left on the Jest list: both hit the same window.localStorage-is-undefined gap under Vitest+jsdom+Node 22 already documented for the Editor/Runners files, this time via ScratchContainer and direct localStorage calls respectively. Confirmed via full Jest and Vitest runs: 474 Jest + 577 Vitest tests pass, the same total as before the move.
Previously useAutoSave, useContainerMinWidth, useIsOnline, useProject, useProjectPersistence, and the two useScratchSave test files only ran under Jest. This change swaps jest.fn/jest.mock/jest.useFakeTimers/ jest.advanceTimersByTime/jest.runAllTimers/jest.clearAllMocks/ jest.clearAllTimers for their vi equivalents, jest.requireActual for the async vi.importActual pattern, and, in useProject.test.jsx, wraps the apiCallHandler factory mocks return value in an explicit default key - the same jszip-utils-style fix already applied elsewhere, since Vitest does not apply Jests CJS default-export interop to mock factories. Rendering useProject and useProjectPersistence crashed under Vitest because window.localStorage is undefined in this environment: Node 22 defines its own global localStorage accessor, which takes precedence over jsdoms and returns undefined unless --localstorage-file is passed. setupTests.vitest.js now replaces it with a minimal in-memory Storage, which fixes this for any test that reads or writes localStorage, not just this batch. Making that work surfaced a second gap: defaultProjects.js and Notifications.jsx import the i18n instance from ./i18n as a default export, but setupTests.vitest.jss existing ./i18n mock only exported a bare t function with no default key, so Vitest could not resolve the import. Fixed by nesting it under default, matching the shape setupTests.js already mocks for Jest. Confirmed via full Jest and Vitest runs: 323 Jest + 728 Vitest tests pass, the same total as before the move.
These three were previously left on the Jest list because they all
hit the window.localStorage gap fixed for the hooks batch in the
previous commit - StopButton and WebComponentProject.integration via
app/store/ScratchContainer constructing an oidc-client UserManager
that reads localStorage at import time, WebComponentLoader via its
own direct localStorage calls. With that gap fixed they now pass, so
this change swaps their jest.fn/jest.mock/jest.useFakeTimers/
jest.useRealTimers/jest.advanceTimersByTime for vi equivalents.
Two WebComponentLoader assertions still failed after that: rendering
the same #wc-carrying markup twice within one test (once in a nested
beforeEach, once in the test body, both left mounted) left two
elements with id="wc" in the document, and Vitests jsdom/nwsapi
resolves a scoped container.querySelector("#wc") via a
document-wide getElementById fast path that returns null when the
first document-wide match with that id isnt inside the given
container - unlike Jests jsdom, which falls back to a proper scoped
search. Switched both assertions to the equivalent attribute selector
container.querySelector("[id='wc']"), which resolves correctly
under both.
Confirmed via full Jest and Vitest runs: 323 Jest + 728 Vitest tests
pass, the same total as before the move.
Previously EditorSlice.test.js and loadProjectReducers.test.js only ran under Jest. This change swaps jest.fn for vi.fn, and wraps the apiCallHandler factory mocks in both files in an explicit default key, the same fix already applied elsewhere for Vitests stricter mock factory interop. "When project has an identifier > The saveProject/fulfilled action sets saving to success" failed under Vitest because Date.now was never restored after the earlier "When project has no identifier" describe reassigned it to a fixed mock - a real leak, previously masked under Jest only because resetMocks happened to wipe that leaked mocks return value back to undefined before this describe ran, and Jests toEqual ignores undefined-valued properties, hiding the resulting lastSavedTime: undefined. Fixed the leak with a file-level afterEach restoring the original Date.now, then updated this describes own beforeEach to mock Date.now deterministically (as its sibling describe already does) and added the resulting lastSavedTime to the expected state, since the reducer sets it on every successful save. Confirmed via full Jest and Vitest runs: 238 Jest + 813 Vitest tests pass, the same total as before the move.
Previously Notifications, ResizableWithHandle, SelectButtons,
ToastCloseButton, apiCallHandler, autoSaveHostApi, autoSaveLifecycle,
and scratchIframe only ran under Jest - the last non-Editor files on
the list.
This change swaps jest.fn/jest.mock/jest.useFakeTimers/
jest.advanceTimersByTime for vi equivalents and jest.requireActual for
the async vi.importActual pattern. Notifications.test.js also mocked
./i18n with a bare { t } object with no default key, the same
default-export gap fixed elsewhere, so it is now wrapped as
{ default: { t } }.
Only the src/components/Editor test files remain on the Jest list.
Confirmed via full Jest and Vitest runs: 193 Jest + 858 Vitest tests
pass, the same total as before the move.
zetter-rpf
force-pushed
the
migrate-more-jest-tests
branch
from
August 21, 2026 16:13
022b7ac to
b7d7bf1
Compare
zetter-rpf
marked this pull request as ready for review
August 21, 2026 16:16
maxelkins
reviewed
Aug 21, 2026
| }); | ||
|
|
||
| expect(store.getState().editor.project.instructions).toBe( | ||
| "demoInstructions.md", |
maxelkins
approved these changes
Aug 21, 2026
maxelkins
left a comment
Contributor
There was a problem hiding this comment.
Checked each, looks great!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This follows on from #1570 and migrates more files to vitest
Most of the tests required renaming from
jesttovi, and a few mocking changes.Most of the changes to
setupTest.vitest.jswere imported from the jestsetupTests.jsfile.I have migrated the remaining files on a branch, I've just kept them separate to reduce the changeset as some of them are slightly more complex to migrate.
changes made with Claude