test(specs): add vitest mocks for tauri, resizeObserver, and canvas APIs#807
test(specs): add vitest mocks for tauri, resizeObserver, and canvas APIs#807senutpal wants to merge 5 commits intoCircuitVerse:mainfrom
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughStandardizes Vitest-based test infrastructure across simulator spec files in both src/simulator/spec and v1/src/simulator/spec. Adds vi-based mocks and global shims for Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/simulator/spec/vitestSetup.ts`:
- Around line 34-40: The afterAll cleanup currently calls vi.useRealTimers()
before vi.clearAllTimers(), so clearAllTimers() never runs under fake timers;
update the afterAll block (the cleanup that uses vi.useRealTimers,
vi.clearAllTimers, vi.clearAllMocks, vi.restoreAllMocks) to call
vi.clearAllTimers() before vi.useRealTimers() while keeping vi.clearAllMocks()
and vi.restoreAllMocks() as part of the cleanup.
- Around line 61-75: The process-level handlers for 'unhandledRejection' and
'uncaughtException' are being registered directly in the Vitest setup file (via
process.on(...)) which runs per spec file; instead, register the handlers into
variables (e.g., unhandledRejectionHandler, uncaughtExceptionHandler) and add
them with process.on(...) inside the setup, then remove them in an afterAll()
teardown using process.removeListener or process.off; ensure the handlers call
shouldSuppressError(...) and rethrow as currently implemented, but cleanly
unregister both handlers in afterAll() to prevent accumulation and memory
warnings.
In `@v1/src/simulator/spec/data.spec.js`:
- Around line 1-25: Remove the duplicate Vitest import that re-declares
beforeAll, describe, expect, test, vi (the import on line with "from 'vitest'"
near the bottom of the file); keep the original import at the top (import {
describe, test, expect, vi, beforeAll } from 'vitest') and delete the redundant
import to resolve the noRedeclare lint error.
In `@v1/src/simulator/spec/vitestSetup.ts`:
- Around line 3-28: Tests set a nonstandard global window.Jquery which
mismatches production usage (window.jQuery) and also don't set global.$; replace
any assignment to window.Jquery with window.jQuery and ensure both global.jQuery
and global.$ are assigned the jQuery require value and that window.$ and
window.jQuery are both set, removing the camel‑cased window.Jquery to keep
global and window namespaces consistent with production (refer to symbols
window.Jquery, window.jQuery, global.jQuery, global.$, window.$).
- Around line 34-75: The current global handlers for unhandledRejection and
uncaughtException (registered via process.on('unhandledRejection') and
process.on('uncaughtException')) suppress errors for the entire test run; change
this to scope suppression to teardown by adding an isTearingDown flag checked
inside shouldSuppressError (or checked in the handlers) and only suppress when
that flag is true, and register/unregister the handlers inside the module so
they are removed in the afterAll() block (which already calls
vi.useRealTimers/clearAllTimers/clearAllMocks/restoreAllMocks), ensuring
suppressedPatterns and shouldSuppressError remain but the handlers are removed
in afterAll to avoid masking real test failures.
♻️ Duplicate comments (6)
v1/src/simulator/spec/complexCircuit.spec.js (1)
17-51: Same mock duplication as noted in other spec files.Refer to the centralization suggestion in
sequential.spec.jsreview.src/simulator/spec/bitConvertor.spec.js (1)
15-50: Mock boilerplate duplicated across src and v1 directories.Both
src/simulator/spec/andv1/src/simulator/spec/contain identical mock definitions. Consider a shared utility that both directories can import.src/simulator/spec/combinationalAnalysis.spec.js (1)
17-51: Same mock duplication pattern.Refer to the centralization suggestion in
sequential.spec.jsreview.v1/src/simulator/spec/decoders-plexers.spec.js (1)
15-49: Shared mock duplication.Consider consolidating these mocks into a shared setup/helper to prevent drift across spec files.
v1/src/simulator/spec/combinationalAnalysis.spec.js (1)
17-52: Shared mock duplication.Consider consolidating these mocks into a shared setup/helper to prevent drift across spec files.
v1/src/simulator/spec/gates.spec.js (1)
15-49: Shared mock duplication.Consider consolidating these mocks into a shared setup/helper to prevent drift across spec files.
🧹 Nitpick comments (9)
src/simulator/spec/gates.spec.js (1)
15-49: Consider centralizing repeated mocks into shared setup.
The tauri/ResizeObserver/canvas/codemirror mocks are repeated across many specs. Moving them intosrc/simulator/spec/vitestSetup.tsor a shared helper would reduce duplication and simplify updates.src/simulator/spec/sequential.spec.js (2)
15-50: Consider centralizing mock boilerplate in a shared setup file.This mock configuration (Tauri event, ResizeObserver, Canvas context, CodeMirror) is duplicated verbatim across all spec files. Extracting these into
vitestSetup.tsor a shared test utility would improve maintainability and reduce the risk of drift between files.♻️ Suggested approach
Create a shared mock utility file:
// src/simulator/spec/testUtils.ts import { vi } from 'vitest' export function setupGlobalMocks() { global.ResizeObserver = vi.fn().mockImplementation(() => ({ observe: vi.fn(), unobserve: vi.fn(), disconnect: vi.fn(), })) HTMLCanvasElement.prototype.getContext = vi.fn(() => ({ clearRect: vi.fn(), fillRect: vi.fn(), fillText: vi.fn(), strokeRect: vi.fn(), beginPath: vi.fn(), moveTo: vi.fn(), lineTo: vi.fn(), stroke: vi.fn(), closePath: vi.fn(), arc: vi.fn(), fill: vi.fn(), })) global.document.createRange = vi.fn(() => ({ setEnd: vi.fn(), setStart: vi.fn(), getBoundingClientRect: vi.fn(() => ({ x: 0, y: 0, width: 0, height: 0, top: 0, right: 0, bottom: 0, left: 0 })), getClientRects: vi.fn(() => ({ item: vi.fn(() => null), length: 0, [Symbol.iterator]: vi.fn(() => []) })), })) global.globalScope = global.globalScope || {} }Then use
vi.mockcalls invitestSetup.tsfor module mocks, and callsetupGlobalMocks()in each spec'sbeforeAll.
92-100: Missing cleanup in afterAll.The component is mounted and attached to
document.body, but there's no corresponding cleanup. This could cause test pollution if multiple spec files run in the same jsdom instance.♻️ Suggested fix
+ let wrapper + beforeAll(async () => { // ... existing setup ... - mount(simulator, { + wrapper = mount(simulator, { global: { plugins: [pinia, router, i18n, vuetify], }, attachTo: elem, }) setup() }) + + afterAll(() => { + wrapper?.unmount() + })v1/src/simulator/spec/bitConvertor.spec.js (2)
112-116: Removeconsole.logfrom test.Logging to console in tests creates noise in test output. If you need to debug, use Vitest's reporter or remove the log for committed code.
♻️ Suggested fix
test('function setBaseValues working', () => { const randomBaseValue = Math.floor(Math.random() * 100) - console.log('Testing for Base Value --> ', randomBaseValue) expect(() => setBaseValues(randomBaseValue)).not.toThrow() })
104-116: Tests only verify no exceptions are thrown.These tests confirm functions don't throw but don't validate behavior. Consider adding assertions for expected side effects (e.g., verifying the store's dialog state changes for
bitConverterDialog, or that input values are set correctly forsetBaseValues).src/simulator/spec/bitConvertor.spec.js (1)
112-116: Sameconsole.logissue as inv1/src/simulator/spec/bitConvertor.spec.js.Remove the console.log statement for cleaner test output.
♻️ Suggested fix
test('function setBaseValues working', () => { const randomBaseValue = Math.floor(Math.random() * 100) - console.log('Testing for Base Value --> ', randomBaseValue) expect(() => setBaseValues(randomBaseValue)).not.toThrow() })src/simulator/spec/combinationalAnalysis.spec.js (1)
114-121: Mutating shared test data may cause flaky tests.Modifying
testData.AndGate.groups[0].inputs[0].labeletc. mutates the imported JSON object. If test order changes or tests run in parallel, this could cause unexpected failures. Consider using a deep copy.♻️ Suggested fix
test('testing Combinational circuit', () => { - testData.AndGate.groups[0].inputs[0].label = 'A' - testData.AndGate.groups[0].inputs[1].label = 'B' - testData.AndGate.groups[0].outputs[0].label = 'AB' + const andGateData = JSON.parse(JSON.stringify(testData.AndGate)) + andGateData.groups[0].inputs[0].label = 'A' + andGateData.groups[0].inputs[1].label = 'B' + andGateData.groups[0].outputs[0].label = 'AB' - const result = runAll(testData.AndGate) + const result = runAll(andGateData) expect(result.summary.passed).toBe(3) })src/simulator/spec/complexCircuit.spec.js (1)
17-51: Consider centralizing shared mocks in a single Vitest setup file.These mocks are repeated across many spec files; moving them into a shared setup (via Vitest
setupFiles) reduces duplication and keeps behavior consistent across the suite.v1/src/simulator/spec/sequential.spec.js (1)
15-49: Centralize shared mocks to avoid drift.These mocks are duplicated across multiple spec files; consider extracting into a shared test setup helper (or vitest setup file) to keep the suite consistent and easier to maintain.
♻️ Example refactor (extract into a shared helper)
-import { beforeAll, describe, expect, test, vi } from 'vitest' +import { beforeAll, describe, expect, test, vi } from 'vitest' +import { setupSimulatorTestMocks } from './testEnvMocks' + +setupSimulatorTestMocks() - -vi.mock('@tauri-apps/api/event', () => ({ - listen: vi.fn(() => Promise.resolve(() => {})), -})) -// ... other mocks ...
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/simulator/spec/bitConvertor.spec.jssrc/simulator/spec/combinationalAnalysis.spec.jssrc/simulator/spec/complexCircuit.spec.jssrc/simulator/spec/data.spec.jssrc/simulator/spec/decoders-plexers.spec.jssrc/simulator/spec/gates.spec.jssrc/simulator/spec/misc.spec.jssrc/simulator/spec/sequential.spec.jssrc/simulator/spec/subCircuit.spec.jssrc/simulator/spec/vitestSetup.tsv1/src/simulator/spec/bitConvertor.spec.jsv1/src/simulator/spec/combinationalAnalysis.spec.jsv1/src/simulator/spec/complexCircuit.spec.jsv1/src/simulator/spec/data.spec.jsv1/src/simulator/spec/decoders-plexers.spec.jsv1/src/simulator/spec/gates.spec.jsv1/src/simulator/spec/misc.spec.jsv1/src/simulator/spec/sequential.spec.jsv1/src/simulator/spec/subCircuit.spec.jsv1/src/simulator/spec/vitestSetup.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-13T21:31:34.486Z
Learnt from: naman79820
Repo: CircuitVerse/cv-frontend-vue PR: 791
File: src/simulator/src/setup.js:175-187
Timestamp: 2026-01-13T21:31:34.486Z
Learning: In src/simulator/src/setup.js, startMainListeners() is intentionally NOT called directly in setup(). Instead, it's initialized via Vue lifecycle through setupPanelsWhenReady() (called from Extra.vue onMounted), which waits for all panels to be mounted before attaching listeners. This lifecycle-driven approach prevents race conditions on first Tauri launch.
Applied to files:
v1/src/simulator/spec/misc.spec.jssrc/simulator/spec/subCircuit.spec.jsv1/src/simulator/spec/gates.spec.jssrc/simulator/spec/data.spec.jsv1/src/simulator/spec/data.spec.jssrc/simulator/spec/complexCircuit.spec.jssrc/simulator/spec/decoders-plexers.spec.jssrc/simulator/spec/gates.spec.jssrc/simulator/spec/sequential.spec.jsv1/src/simulator/spec/subCircuit.spec.jsv1/src/simulator/spec/bitConvertor.spec.jssrc/simulator/spec/misc.spec.jssrc/simulator/spec/combinationalAnalysis.spec.jsv1/src/simulator/spec/complexCircuit.spec.jsv1/src/simulator/spec/combinationalAnalysis.spec.jsv1/src/simulator/spec/decoders-plexers.spec.jsv1/src/simulator/spec/sequential.spec.jssrc/simulator/spec/bitConvertor.spec.js
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.
Applied to files:
src/simulator/spec/subCircuit.spec.jssrc/simulator/spec/data.spec.jsv1/src/simulator/spec/data.spec.jsv1/src/simulator/spec/vitestSetup.tsv1/src/simulator/spec/subCircuit.spec.jssrc/simulator/spec/vitestSetup.ts
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope is a global variable that should be typed by extending the Window interface in a TypeScript declaration file (global.d.ts), as it's initialized on the window object.
Applied to files:
v1/src/simulator/spec/vitestSetup.ts
🧬 Code graph analysis (13)
src/simulator/spec/subCircuit.spec.js (1)
src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/gates.spec.js (2)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/data.spec.js (3)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/data/backupCircuit.js (2)
scheduleBackup(72-85)checkIfBackup(16-21)src/simulator/src/data/project.ts (3)
clearProject(144-152)recoverProject(18-29)openOffline(35-79)
src/simulator/spec/decoders-plexers.spec.js (2)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/testbench.ts (1)
runAll(422-474)
src/simulator/spec/gates.spec.js (2)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/subCircuit.spec.js (2)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/bitConvertor.spec.js (2)
src/simulator/spec/bitConvertor.spec.js (3)
actual(41-41)pinia(53-53)router(54-54)src/simulator/src/utils.ts (3)
bitConverterDialog(229-232)setupBitConvertor(271-314)setBaseValues(252-259)
src/simulator/spec/misc.spec.js (1)
v1/src/simulator/spec/misc.spec.js (2)
pinia(52-52)router(53-53)
src/simulator/spec/combinationalAnalysis.spec.js (2)
src/simulator/src/combinationalAnalysis.js (4)
performCombinationalAnalysis(18-57)performCombinationalAnalysis(18-57)GenerateCircuit(59-91)GenerateCircuit(59-91)src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/combinationalAnalysis.spec.js (1)
src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/decoders-plexers.spec.js (1)
src/simulator/src/testbench.ts (1)
runAll(422-474)
v1/src/simulator/spec/sequential.spec.js (2)
src/simulator/src/data/load.js (1)
load(207-293)src/simulator/src/testbench.ts (1)
runAll(422-474)
src/simulator/spec/bitConvertor.spec.js (1)
src/simulator/src/utils.ts (3)
bitConverterDialog(229-232)setupBitConvertor(271-314)setBaseValues(252-259)
🪛 Biome (2.1.2)
v1/src/simulator/spec/data.spec.js
[error] 25-25: Shouldn't redeclare 'beforeAll'. Consider to delete it or rename it.
'beforeAll' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'describe'. Consider to delete it or rename it.
'describe' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'expect'. Consider to delete it or rename it.
'expect' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'test'. Consider to delete it or rename it.
'test' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'vi'. Consider to delete it or rename it.
'vi' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (14)
src/simulator/spec/misc.spec.js (1)
15-100: Mocks and simulator bootstrap look solid.
The tauri/ResizeObserver/canvas/codemirror mocks plus Pinia/Router/Vuetify setup align well with jsdom constraints.src/simulator/spec/subCircuit.spec.js (1)
1-14: No changes needed. Vitest'sglobals: trueis already configured in vite.config.ts (line 60), which automatically injectsdescribe,beforeAll,test, andexpectglobally. Explicit imports are not required. While some other spec files choose to import these explicitly as a style preference, it is not mandatory and the current file works correctly without them.Likely an incorrect or invalid review comment.
src/simulator/spec/decoders-plexers.spec.js (1)
1-14: The Vitest configuration hasglobals: trueenabled in vite.config.ts (line 60), which meansdescribe,test,expect, andbeforeAllare available globally without explicit imports. The decoders-plexers.spec.js file follows the established pattern used across the majority of the codebase's spec files (bitConvertor, combinationalAnalysis, complexCircuit all use the same approach). No changes are needed.src/simulator/spec/sequential.spec.js (1)
102-129: Test cases look correct.The sequential element tests (D Flip Flop, D Latch, JK/SR/T Flip Flops) properly load circuit data and verify expected pass counts via
runAll. The test structure is sound.v1/src/simulator/spec/complexCircuit.spec.js (1)
104-120: Test cases are well-structured.The ALU and ripple carry adder tests properly load circuit data and assert specific expected pass counts (10 for ripple carry, 5 for ALU). These provide meaningful validation of circuit behavior.
src/simulator/spec/complexCircuit.spec.js (1)
53-121: LGTM on the test bootstrap and assertions.Setup + mount sequence looks consistent and the assertions remain stable.
v1/src/simulator/spec/data.spec.js (1)
26-195: LGTM on the expanded test scaffolding.Mocks + bootstrap setup align with the intended jsdom environment.
v1/src/simulator/spec/misc.spec.js (1)
1-155: LGTM.src/simulator/spec/data.spec.js (1)
1-194: LGTM.v1/src/simulator/spec/subCircuit.spec.js (1)
1-110: LGTM.v1/src/simulator/spec/sequential.spec.js (1)
102-129: Tests remain intact and readable.v1/src/simulator/spec/decoders-plexers.spec.js (1)
102-139: Test expectations remain consistent.v1/src/simulator/spec/combinationalAnalysis.spec.js (1)
104-121: Test cases stay intact.v1/src/simulator/spec/gates.spec.js (1)
102-139: Gate tests still validate behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
- Correct timer cleanup sequence (clearAllTimers before useRealTimers) - Align jQuery namespace with production (window.jQuery, global.$, window.$) - Restrict error suppression to teardown phase via isTearingDown flag - Eliminate duplicate vitest import in data.spec.js
- Import expect, describe, test, beforeall alongside vi in test files - Addresses 'expect is not defined' ESLint errors
- Add missing rect() method to resolve unhandled 'simulationArea.context.rect is not a function' errors during save() tests - Add getSerializedSvg() for SVG export support - Add save(), restore(), translate(), scale() for common canvas operations - Add fillStyle, strokeStyle, font properties Resolves 3 unhandled errors in vitest run across src, v0, and v1 workspace spec files.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/simulator/spec/data.spec.js`:
- Around line 176-187: The tests call async functions newProject, clearProject,
and recoverProject but currently wrap them in synchronous expect(() =>
fn()).not.toThrow(), which won't catch promise rejections; change each test to
be async and await the promise-based call (or use Jest's .resolves matcher) —
e.g. make the "project working", "clear Project working", and "recover Project
working" tests async and assert await
expect(newProject(true)).resolves.toBeUndefined() (and similarly for
clearProject() and recoverProject()) or simply await newProject(true) / await
clearProject() / await recoverProject() inside the async test so any rejection
fails the test.
- Around line 36-57: The canvas 2D context mock returned by
HTMLCanvasElement.prototype.getContext is missing methods used by the simulator;
update that mock to include measureText (returning an object with width),
setLineDash, getImageData, rotate, putImageData, drawImage, and bezierCurveTo so
tests won't fail; locate the getContext mock in src/simulator/spec/data.spec.js
and add vi.fn() stubs for setLineDash, rotate, putImageData, drawImage,
bezierCurveTo, and getImageData (returning a minimal ImageData-like object), and
add measureText as a vi.fn(() => ({ width: 0 })) so calls to measureText and
other canvas methods are handled.
In `@v1/src/simulator/spec/data.spec.js`:
- Around line 176-187: The tests call async functions (newProject, clearProject,
recoverProject) synchronously; update each test to be async and await the async
call, asserting the promise resolves (e.g., mark the test callback async and use
await expect(newProject(true)).resolves.toBeUndefined() or simply await
newProject(true) to ensure no rejection), do the same for clearProject() and
recoverProject() (after setting localStorage), replacing
expect(...).not.toThrow() with await/expect(...).resolves assertions or direct
awaits so failures are caught properly.
🧹 Nitpick comments (1)
v1/src/simulator/spec/data.spec.js (1)
26-69: Consider extracting shared mocks to a common test utility.The Tauri, ResizeObserver, Canvas, and CodeMirror mocks are duplicated verbatim between
src/simulator/spec/data.spec.jsandv1/src/simulator/spec/data.spec.js. Given thatvitestSetup.tsalready exists for shared setup, these mocks could be extracted to a shared module (e.g.,testMocks.ts) and imported in both spec files.This would reduce maintenance burden when extending the canvas mock or updating mock implementations.
Add missing canvas context methods used by simulator code:
- measureText: returns { width: 0 } for Text, Tunnel, Flag modules
- setLineDash: for Rectangle, ImageAnnotation dashed selections
- getImageData: for pixel data operations
- rotate: for canvas rotation
- putImageData: for image data restoration
- drawImage: for image import/save
- bezierCurveTo: for bezier curve drawing
Note: v0/src/simulator/spec/data.spec.js already has these methods.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/simulator/spec/data.spec.js`:
- Around line 200-205: The test silently no-ops because
document.querySelector('#Open_offline_btn')?.click() targets a non-existent
jQuery button; update the spec to actually trigger the OpenOffline behavior by
either rendering the Vue dialog component or mocking the dialog rather than
clicking a missing element. Specifically, replace the optional-chained click
with one of: mount/render OpenOffline.vue (or the parent component that calls
openOffline()) and query the real button rendered by that component, then click
it; or mock the dialog creation path in openOffline() (in project.ts) to
simulate the button callback and assert globalScope.id changes. Also remove the
optional chaining so a failing selector surfaces (so update references to
openOffline(), Open_offline_btn, and globalScope in the test).
♻️ Duplicate comments (2)
src/simulator/spec/data.spec.js (1)
183-194: Async functions wrapped in synchronousexpect().not.toThrow()won't catch rejections.
newProject,clearProject, andrecoverProjectare async functions. Wrapping them inexpect(() => fn()).not.toThrow()only checks that calling them doesn't throw synchronously—it won't catch promise rejections.Proposed fix using async/await assertions
- test('project working', () => { - expect(() => newProject(true)).not.toThrow() - }) + test('project working', async () => { + await expect(newProject(true)).resolves.not.toThrow() + }) - test('clear Project working', () => { - expect(() => clearProject()).not.toThrow() - }) + test('clear Project working', async () => { + await expect(clearProject()).resolves.not.toThrow() + }) - test('recover Project working', () => { - localStorage.setItem('recover', JSON.stringify(gatesCircuitData)) - expect(() => recoverProject()).not.toThrow() - }) + test('recover Project working', async () => { + localStorage.setItem('recover', JSON.stringify(gatesCircuitData)) + await expect(recoverProject()).resolves.not.toThrow() + })v1/src/simulator/spec/data.spec.js (1)
183-194: Same async testing issue exists here.As noted for
src/simulator/spec/data.spec.js,newProject,clearProject, andrecoverProjectare async functions that should be tested with async/await patterns rather than synchronousexpect().not.toThrow().Proposed fix
- test('project working', () => { - expect(() => newProject(true)).not.toThrow() - }) + test('project working', async () => { + await expect(newProject(true)).resolves.not.toThrow() + }) - test('clear Project working', () => { - expect(() => clearProject()).not.toThrow() - }) + test('clear Project working', async () => { + await expect(clearProject()).resolves.not.toThrow() + }) - test('recover Project working', () => { - localStorage.setItem('recover', JSON.stringify(gatesCircuitData)) - expect(() => recoverProject()).not.toThrow() - }) + test('recover Project working', async () => { + localStorage.setItem('recover', JSON.stringify(gatesCircuitData)) + await expect(recoverProject()).resolves.not.toThrow() + })
🧹 Nitpick comments (1)
v1/src/simulator/spec/data.spec.js (1)
1-210: Consider extracting shared test setup to reduce duplication.Both
src/simulator/spec/data.spec.jsandv1/src/simulator/spec/data.spec.jscontain nearly identical code (~200 lines each). The mocks (Tauri, ResizeObserver, Canvas, CodeMirror) and thebeforeAllsetup could be extracted to a shared test utility file (e.g.,spec/testUtils.jsor leverage the existingvitestSetup.ts), reducing maintenance burden and ensuring consistency.
1 participant
Fixes #772
Describe the changes you have made in this PR -
Summary
Added standardized test mocks and imports to all spec files to enable consistent test execution in jsdom environment without browser dependencies.
Changes Made
1. vitestSetup.ts Updates
Added comprehensive global setup for all tests:
$andjQueryvi.useFakeTimers()inbeforeAll, cleanup inafterAll2. Spec File Updates
Added identical imports and mocks to each spec file in
src/simulator/spec/:Added to each file:
@tauri-apps/api/eventResizeObserverHTMLCanvasElement.getContextCode Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The mocks were added to each spec file to ensure isolated test environments. Using
vi.mock()andglobal.assignments ensures each test file has its own isolated mock state. The same block was applied to all spec files to maintain consistency and simplify maintenance - future changes can be made in one pattern and applied uniformly.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.