Fix the stale unit tests revealed by the jsdom/Vitest migration#153
Open
CarsonDavis wants to merge 2 commits into
Open
Fix the stale unit tests revealed by the jsdom/Vitest migration#153CarsonDavis wants to merge 2 commits into
CarsonDavis wants to merge 2 commits into
Conversation
Observe panel layout-changed notifications over the mitt event bus instead of the removed window.dispatchEvent (8 panelManager specs), and drop the over-asserted required-icon check in the dashboard config validator spec. A tool icon is optional by design: getValidIconClass falls back to a default when one is missing, so the validator correctly does not require it. Test-only; no production source changes.
mockLayoutChangedEvents subscribed to the module-singleton mmgisAPI mitt bus and relied on the caller's restore() to unsubscribe. Register the teardown via vitest's onTestFinished so a spec that forgets restore() cannot leak the handler across tests. restore() is kept for explicit use.
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.
What this does
Fixes the 9 stale unit tests that #150 (the Vitest + jsdom migration) left red by design. With the unit suite finally running, these previously-dead specs execute and fail — not because of the migration, but because they were written against older behavior and never ran to catch the drift. This brings them in line with the code's current behavior so the suite is honestly green.
Implements #149. Stacked on #150 (
tests-jsdom).Changes
Test content only — no production source changed.
window.dispatchEvent. The app now broadcasts panel-layout changes over the in-app mitt event bus (mmgisAPI.emit('mmgis-panel-layout-changed', { panels })), so the old listener observed nothing. They now subscribe to the bus and assert the notification fires (and, where applicable, that the{ panels }payload is present and sorted by priority). The shared__mocks__/mmgisAPI.jsstub (added by Run unit tests in a jsdom environment via Vitest #150) is upgraded from a no-op to a realmitt()instance so specs can subscribe to it.icon. Resolved deliberately: a tool icon is optional.getValidIconClass(ToolMetadataUtils.js) falls back to a default icon and only warns when one is missing, and the modern UI renders every tool through it — so the runtime tolerates a missing icon by design. The over-assertion is dropped andDashboardConfigValidator.jsis left unchanged (keeping this a test-only PR).The producer (
PanelManager_.ts) is intentionally not changed — the mitt bus is the current, correct mechanism; reintroducing a window event would be the wrong fix.Result
npx vitest run→ 669 passed (38 files), 0 failures. The unit suite is fully green; combined with #150, CI's unit leg goes green once both land.Notes for reviewers