Add vitest and fix memory leak#145
Open
sankhesh wants to merge 9 commits intoKitware:masterfrom
Open
Conversation
Set up the dependencies needed for component-level unit testing: @testing-library/react and @testing-library/jest-dom for rendering and matchers, jsdom as the simulated DOM environment, and vitest as the runner. Also drops @babel/core which is no longer needed at the root since rollup pulls it in transitively.
Wire vitest into the project: a jsdom environment with globals enabled, a setup file that pulls in @testing-library/jest-dom matchers and stubs out ResizeObserver (which jsdom does not provide), and a new test:unit script slotted into the existing test pipeline.
The project's tsconfig sets "jsx": "react-jsx" so source files do not import React for JSX. Without this esbuild option, vitest falls back to the classic transform and tests with JSX fail at evaluation with "React is not defined".
Wire the full vtk.js pipeline (vtkConeSource -> mapper -> actor -> renderer) through the React components and assert on data availability, mapper input geometry, actor registration, visibility, and cleanup. Mocks only the two GPU-touching modules: vtkOpenGLRenderWindow and vtkRenderWindowInteractor.
vtkRenderWindow.addView() calls view.setRenderable(publicAPI) to back-link the view to its owning render window. The mock was missing this method, so every test crashed inside the React passive-effect flush with "view.setRenderable is not a function" before any assertion ran.
Mounts and unmounts the component 17 times - one past Chrome's per-process WebGL context limit of 16 - and asserts that the cleanup callback (a) calls WEBGL_lose_context.loseContext() once per cycle and (b) does so strictly before view.delete(), so the GPU slot is returned to the browser before vtk.js tears down the extension handle. Fails on this commit; the fix lands next.
vtkOpenGLRenderWindow.delete() tears down the vtk.js side of the view but does not relinquish the underlying WebGL context. Chrome enforces a hard per-process limit of ~16 WebGL contexts; mounting and unmounting viewports across case transitions exhausted that pool, leaving any still-active viewport blank. Grab the canvas before delete(), look up the WEBGL_lose_context extension, and call loseContext() so the GPU slot is returned to the browser. Ordering matters: loseContext() must run before view.delete(), otherwise the extension handle is already gone.
Adds a CI workflow that triggers on pull_request to master and beta and runs the same npm run test pipeline (lint -> unit -> build) that the publish workflow runs on push, so the PR gate matches the release gate. Concurrency is keyed by PR number so a force-push cancels the in-flight run instead of queueing behind it.
Bumps both ci.yml and publish.yaml to node-version: 24 so the runner's npm matches the npm 11 that produced package-lock.json locally; regenerates the lock file in that same shape so npm ci no longer complains about missing transitive entries (cosmiconfig, ts-node). Also lands the lint cleanups required to keep the new test files passing the test:lint stage of npm run test: an eslint override for src/__tests__/** that disables a small set of rules that conflict with vitest patterns (vi.mock between imports, non-null assertions after expect-truthy, no-op stub methods), plus prettier formatting fixes in ConeRendering.test.tsx.
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
1 participant
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.
No description provided.