Add initial test coverage for LegoWidget core logic#6347
Open
DhyaniKavya wants to merge 1 commit intosugarlabs:masterfrom
Open
Add initial test coverage for LegoWidget core logic#6347DhyaniKavya wants to merge 1 commit intosugarlabs:masterfrom
DhyaniKavya wants to merge 1 commit intosugarlabs:masterfrom
Conversation
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Contributor
Author
|
@walterbender @omsuneri Also, Prettier failures in CI are pre-existing and unrelated to this change. |
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.
issue: #6344
Problem:-
The LegoWidget implementation in js/widgets/legobricks.js has no existing unit test coverage, despite being one of the largest and most complex files in the codebase (~3000+ lines).
Critical core logic such as frequency calculation, block management, and internal state handling is not verified through automated tests. This creates a high risk of regressions, where future changes can silently break functionality without being detected.
Solution:-
-This PR introduces initial unit test coverage for key deterministic logic in the LegoWidget.
-focuses only on core, non-UI methods to keep scope minimal and safe
-adds meaningful test cases for frequency calculation, block insertion, and state reset
-includes edge case handling (negative and high octave values)
-preserves all existing functionality without modifying production logic
Changes Made:-
-Files modified:
=> js/widgets/tests/legobricks.test.js
-> added unit tests for _calculateFallbackFrequency() with multiple pitch values, reference case (A4 ≈ 440 Hz), and edge octaves
-> added tests for addRowBlock() including duplicate handling and offset logic
-> added tests for clearBlocks() ensuring full reset of internal state
-> improved test structure using helper functions and beforeEach to reduce redundancy
-> added negative and extreme input tests for robustness
=> js/widgets/legobricks.js
-> added minimal module export guard (4 lines) to enable unit testing, consistent with existing widget patterns
-ESLint: ✅ no lint errors
-Tests: ✅ all tests passed
PR Category
Impact:-
-Introduces test coverage for a previously untested critical module
-Reduces risk of regressions in core logic
-Improves code reliability and maintainability
-Provides a foundation for future test expansion
-No functional changes or runtime impact
Notes:-
This change is intentionally minimal and focused on core logic testing. It avoids UI testing and large-scale coverage expansion, aligning with incremental improvement practices.
Happy to expand coverage further based on maintainer feedback.