test: improve unit test coverage for legobricks.js

[NAME-ASHWANIYADAV]

Description

legobricks.js is one of the largest widget files (119KB / 3150 lines) but had limited test coverage (46 tests). This PR adds 62 new tests (total: 108) covering previously untested methods and edge cases.

PR Category

• Tests - Adds or updates test coverage

Changes Made

New test suites for previously untested methods:

• _generateRowsFromPitchBlocks (5 tests) - pitch row generation, frequency-based sorting, solfege display name conversion, drum block filtering, default row fallback
• • _collectNotesToPlay (5 tests) - null/empty colorData handling, background vs active note detection, note value assignment by duration, multi-row pitch collection
• • _mergeConsecutiveColorSegments (6 tests) - same-color merging, gray variant merging, non-gray rejection, zero/one segment handling, multi-row independence

Extended edge-case coverage for existing methods:

• _getColorFamily (10 tests) - all 10 hue ranges: orange, yellow, cyan, purple, magenta, pink, red wrap-around, gray mid-lightness, boundary values
• • _rgbToHsl (6 tests) - green, blue, mid-gray, yellow, cyan, low-lightness
• • _convertRowToPitch (6 tests) - flat notes (Db, Eb, Bb), sharps (F#), all 7 natural notes, multi-digit octaves
• • _getColorHex (1 test) - all 12 known color codes in a single assertion
• • _getContrastColor (2 tests) - all 5 light colors, all 7 dark colors
• • _shouldMergeColors (3 tests) - all gray pairs, non-gray distinct pairs, gray vs non-gray
• • _colorsAreSimilar (6 tests) - both null, low-saturation, close HSL values, large sat/light differences, hue wrap-around
• • _filterSmallSegments (4 tests) - empty input, single element, all-large segments, consecutive small absorption
• • _analyzeColumnBoundaries (3 tests) - no colorSegments, deduplication, 500ms merge threshold
• • addRowBlock (2 tests) - triple duplicates, correct rowMap indexing
• • _getColorFamilyByName (2 tests) - all 12 known colors, hue property verification

Testing Performed

• Ran npx jest js/widgets/__tests__/legobricks.test.js --verbose locally
• • All 108 tests pass successfully
• • Prettier formatting verified (unchanged)
• • ESLint clean (no warnings)

Checklist

• I have tested these changes locally and they work as expected.
• I have added/updated tests that prove the effectiveness of these changes.
• I have followed the project's coding style guidelines.
• I have run npm run lint and npx prettier --check . with no errors.
• I have enabled "Allow edits from maintainers"

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.85%. Comparing base (338362b) to head (2760671).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7733      +/-   ##
==========================================
+ Coverage   56.80%   56.85%   +0.05%     
==========================================
  Files         172      172              
  Lines       57683    57683              
==========================================
+ Hits        32767    32798      +31     
+ Misses      24916    24885      -31     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[NAME-ASHWANIYADAV]

@walterbender PTAL !!

[vanshika2720]

@NAME-ASHWANIYADAV _generateRowsFromPitchBlocks, _collectNotesToPlay, and _mergeConsecutiveColorSegments are still untested despite being key parts of the workflow. I'd also recommend adding a test that exercises _addColorSegment's initialization path, which currently exposes a production bug (this.this). Once those are covered, this should be in good shape.

[Chaitu7032]

The new tests are a great step forward. You've covered the main functions and helpers really well.
I agree with vanshika there is a need of test for how "addColorSegment" starts up. That's the exact place where the "this.this" bug would show up if a row needs to be created from scratch ..

[NAME-ASHWANIYADAV]

@NAME-ASHWANIYADAV _generateRowsFromPitchBlocks, _collectNotesToPlay, and _mergeConsecutiveColorSegments are still untested despite being key parts of the workflow. I'd also recommend adding a test that exercises _addColorSegment's initialization path, which currently exposes a production bug (this.this). Once those are covered, this should be in good shape.

Thanks for the review @vanshika2720!

Just to clarify - _generateRowsFromPitchBlocks, _collectNotesToPlay, and _mergeConsecutiveColorSegments are actually fully tested in this PR (5, 5, and 6 tests respectively). You can see their describe blocks starting after line 348 in the test file.

and on _addColorSegment ! I've now added a dedicated test suite for it in the latest commit that:

• Tests the append path (adding to an existing colorData entry)
• Explicitly exposes the this.this.matrixData production bug - the test proves that calling _addColorSegment on an uninitialized row throws a TypeError due to the typo.
• Tests the pre-populated entry path (no crash when colorData[index] already exists)
• Verifies correct performance.now() timestamp recording

Total tests are now 112 (was 108). All passing!

[NAME-ASHWANIYADAV]

The new tests are a great step forward. You've covered the main functions and helpers really well. I agree with vanshika there is a need of test for how "addColorSegment" starts up. That's the exact place where the "this.this" bug would show up if a row needs to be created from scratch ..

Thanks @Chaitu7032! Agreed - I've added the _addColorSegment initialization tests in the latest commit .

The test confirms that the this.this bug causes a TypeError when a new row needs to be created from scratch, exactly as you mentioned.

[Ashutoshx7]

just a small change this.this.matrixData to this.metrixData and then flip the test from .toThrow(TypeError) to a real assertion that the new entry gets created correctly.

neat picking

after that good to go

[github-actions]

This PR has merge conflicts with master.

Please rebase your branch:

# Add upstream remote (one-time setup)
git remote add upstream https://github.com/sugarlabs/musicblocks.git

# Fetch latest master and rebase
git fetch upstream
git rebase upstream/master

# Resolve any conflicts, then:
git push --force-with-lease origin YOUR_BRANCH

Tip: Enable "Allow edits from maintainers" on this PR so we can auto-rebase for you next time. This only grants access to your PR branch. Your fork's other branches are not affected.

[Ashutoshx7]

@walterbender lgtm