Skip to content

test: add node:test coverage for active-note windowing/sustain fade logic - #17

Merged
OmikronApex merged 1 commit into
mainfrom
chore/add-basic-tests
Jul 8, 2026
Merged

OmikronApex merged 1 commit into
mainfrom
chore/add-basic-tests

Conversation

@OmikronApex

@OmikronApex OmikronApex commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Twenty-sixth repo from the org test-coverage audit. screen.js has no top-level wrapping IIFE for its function declarations; only the hooks block at the tail is wrapped. The module.exports hook wraps that block in an else-branch (same pattern as feedBack-plugin-sectionmap#9) so it never runs under Node.

10 tests against _fbGetActiveNotes, the function deciding which fretboard dots light up at a given playhead time:

  • Standalone notes: within/outside the 80ms window, sustain fade toward a 0.3 floor, stays active through full sustain duration, the sorted-by-time early-break guard doesn't suppress an earlier reachable note
  • Chords: members within window all included, a chord onset outside the 0.3s lookback excluded entirely, individual chord-member sustain still governs per-member inclusion once the chord-level gate passes
  • Combines standalone notes + chords in one list; tolerates missing/undefined notes and chords

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added automated coverage for note-selection behavior across timing windows, sustain fade handling, chord membership, and mixed note/chord output.
    • Included checks for edge cases such as missing data and efficient handling of far-future notes.

…ogic

Node-only module.exports hook wraps the playSong-hooking IIFE in
an else-branch, mirroring the sectionmap plugin's pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3df72ad4-0609-4188-8afc-7a4be7180703

📥 Commits

Reviewing files that changed from the base of the PR and between cf92671 and ab383a7.

📒 Files selected for processing (2)
  • screen.js
  • tests/screen.test.js

📝 Walkthrough

Walkthrough

screen.js adds a conditional Node/CommonJS export of the internal _fbGetActiveNotes helper, wrapping existing browser-only Hooks initialization in an else branch. A new test file, tests/screen.test.js, adds Node-based test coverage for note/chord activation, sustain behavior, iteration guards, and edge cases.

Changes

Node export and test coverage for _fbGetActiveNotes

Layer / File(s) Summary
Node/CommonJS export guard
screen.js
Adds a guard exporting _fbGetActiveNotes via module.exports in Node, moving browser Hooks/playSong initialization into an else branch.
Test setup and standalone note tests
tests/screen.test.js
Adds test bootstrap (require-cache clearing, global.window) and tests for note activation window, exclusion, sustain fade with alpha floor, and duration cutoff.
Iteration break guard test
tests/screen.test.js
Verifies early stopping over sorted-by-time notes when a far-future note is encountered.
Chord activation and sustain tests
tests/screen.test.js
Tests chord inclusion/exclusion by lookback window and per-member sustain causing independent chord member drop-out.
Robustness and combined output tests
tests/screen.test.js
Tests empty-array handling for missing notes/chords and correct merging of standalone notes with chord members.

Estimated code review effort: 2 (Simple) | ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: new node:test coverage for active-note windowing and sustain fade logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/add-basic-tests

Comment @coderabbitai help to get the list of available commands.

@OmikronApex
OmikronApex merged commit 6a4c246 into main Jul 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant