test(activity): cover Activity-ProjectManager wiring through real construction - #8246
Merged
walterbender merged 5 commits intoAug 26, 2026
Conversation
github-actions
Bot
added
tests
Member
|
lmk when its ready for review |
…struction Extends the real-component integration series (noteclamp/pitch/volume/ meter/tone dispatch tests) one layer up: neither the existing Activity toolbar test nor the ProjectManager unit tests exercise a real Activity wired to a real ProjectManager together. This loads both js/activity.js and js/project-manager.js unmodified into one vm sandbox, in the same dependency order as js/loader.js, so setupProjectManager(this) runs for real during construction. Asserts the resulting projectManager is a real ProjectManager instance pointing back at the same activity, and that a real prepareExport() call serializes that activity's real block/turtle state. Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
Address review feedback on the prior commit: - prepareExport() mutation check seeded hasMatrixDataBlock to true before the call, since Activity never initializes that field itself - the previous version could pass even if prepareExport() never touched it. Asserting it flips to false now actually proves the call mutated the real Activity, not a detached copy. - Dropped the _loadAnimationIntervalId assertion; it only restates a ProjectManager implementation detail unrelated to the Activity wiring boundary and would make the test brittle against unrelated refactors. - The activity.js source split now throws with a clear message if the expected initialization line isn't found, instead of silently running more of the module than intended. Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
Address further review feedback:
- Stop slicing activity.js at the literal string
"const activity = new Activity();" to isolate the Activity class before
its auto-instantiation. Instead run the file completely unmodified: the
sandbox's define() stub is a no-op, so the trailing domReady bootstrap
call it's wrapped in is created but never invoked, and the file's own
top-level singleton construction becomes the instance under test.
"this.activity = activity;" pulls that binding out of the vm script's
scope, the same way this test already does for the Activity class name
itself. This removes all coupling to a specific statement's exact text
and layout, replacing it with only the module's own load-bearing
identifiers ("Activity", "activity"). Each test now loads a fresh vm
context so no state can leak between them.
- Reworded the header comment: unrelated Activity dependencies (Turtles,
Blocks, Logo, the setupXController functions, etc.) are stubbed to keep
construction free of real DOM/audio/canvas machinery Jest can't provide -
they sit outside the Activity-ProjectManager seam, not on either side of
it, so 'never mocks either side of that seam' was imprecise.
- Loosened the prepareExport() serialization check from a full deep-equal
of the exported array to the specific facts that show the real block and
turtle graphs were read (the turtle's real id in the start block's
args, the note block's connection resolving through the real block
list) - exact serialization format is already covered by
project-manager.test.js.
- Split the second test into two single-purpose tests: one for reading
Activity-owned state through ProjectManager, one for ProjectManager
writing back to the same real Activity instance (hasMatrixDataBlock).
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
Address feedback that the wiring test's ~280-line bespoke vm sandbox was disproportionate to the 2-3 assertions it backed, and largely duplicated activity_toolbar_integration.test.js's own sandbox: - Factor the shared dependency stubs (Turtles, Blocks, Logo, the various setupXController functions, etc. - everything outside whatever seam a given test is exercising) into js/__tests__/helpers/activity-vm-sandbox.js, a small loader that runs the real, unmodified activity.js in a vm context and returns it. activity_toolbar_integration.test.js now uses it too (behavior unchanged: still gets a real Activity with setupProjectManager mocked, still exercises the same toolbar-delegation methods) instead of keeping its own duplicate copy of the same ~120-line scaffold. - activity-projectmanager-integration.test.js now only adds what its own seam needs on top of the shared helper: the real project-manager.js source (via the helper's prependCode hook) instead of the shared sandbox's default mocked setupProjectManager, plus the handful of extra globals project-manager.js's methods read at call time. - Trimmed the wiring test itself from three tests validating pieces of prepareExport()'s serialization contract (already covered in project-manager.test.js) down to two: real construction wiring (activity.projectManager instanceof ProjectManager, back-reference to the same activity), and one combined check that a real prepareExport() call both reads Activity-owned state (exported block count matches the real blockList) and mutates the same real Activity instance (hasMatrixDataBlock, seeded to the opposite value first so the assertion actually proves the mutation). Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
… loading into it Address further review feedback: - Restored a structural assertion on prepareExport()'s output: the second block's connection (originally block index 0) resolves to the first block's exported index, and its name/position are checked too - proving ProjectManager walked the real activity.blocks.blockList graph to build that entry, not just that the array happened to have the right length. - Trimmed js/__tests__/helpers/activity-vm-sandbox.js's stub set to only what Activity's constructor actually touches directly - verified empirically by removing stubs until construction stopped succeeding. Turtles/Palettes/Blocks/Logo/LanguageBox/ThemeBox/SaveInterface/ StatsWindow/Trashcan/PasteBox/HelpWidget/GIFAnimator/i18next/ AlertController/platformColor/globalActivity/LEADING/MYDEFINES/jQuery/ createjs are never referenced during construction itself: they're wired up later by setupDependencies(), which only runs from the domReady bootstrap callback the sandbox's no-op define() never invokes. - Moved the real-project-manager loading logic (reading project-manager.js, appending the this.setupProjectManager/this.ProjectManager exposers) into the helper as loadActivityWithRealProjectManager(), so the integration test itself only expresses intent instead of repeating that plumbing. Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
vanshika2720
force-pushed
the
test/integration-activity-projectmanager
branch
from
August 26, 2026 14:22
9682277 to
788a4ac
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8246 +/- ##
=======================================
Coverage 65.33% 65.33%
=======================================
Files 190 190
Lines 59318 59318
=======================================
Hits 38758 38758
Misses 20560 20560 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
vanshika2720
marked this pull request as ready for review
August 26, 2026 14:41
Collaborator
Author
|
@Ashutoshx7 ready |
github-project-automation
Bot
moved this from Todo
to Done
in @MusicBlocks Issues and PRs
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
3 participants
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.
Description
Adds integration coverage for the wiring between the real
ActivityandProjectManagercomponents.The test loads both
js/activity.jsandjs/project-manager.jsunmodified into the samevmcontext, following the dependency order used byjs/loader.js. This allowsActivityconstruction to invoke the realsetupProjectManager(this)path instead of replacing the project manager with a mock.The test verifies that a real
Activityreceives a realProjectManager, that the project manager references the same activity instance, and that a realprepareExport()call reads and mutates state belonging to that activity.No production code is changed.
Category
Changes Made
js/__tests__/activity-projectmanager-integration.test.js.ActivityandProjectManagerimplementations into a sharedvmcontext.Activityconstruction creates and wires a realProjectManager.activity.projectManager.activityreferences the sameActivityinstance.ProjectManager.prepareExport()method against a small block/turtle fixture.Activitystate mutation.Visual Changes
Not applicable. This change only adds automated tests.
Testing Performed
git diff --check: clean.musickeyboard.test.js— existing failure reproduced independently.camera.test.js— full-suite worker SIGSEGV; the test passes when run in isolation.Checklist
npm run lintandnpx prettier --check .with no errors.Additional Notes
This test covers a previously untested integration boundary: real
Activityconstruction wiring a realProjectManagerinstance.Existing coverage tests
Activitywith a mockedsetupProjectManagerand testsProjectManageragainst a plain fake activity, but neither exercises both real components together. This test fills that gap without introducing DOM/canvas-heavy setup.prepareExport()was selected because it is synchronous and can exercise the real Activity/ProjectManager relationship without requiring the file-loading or UI infrastructure used bysaveLocallyanddoLoad.The test does not currently provide a full project load/save round trip. Those flows still depend on additional
Turtles/Blocksinfrastructure and can be covered separately if needed.