Add unit tests for inactivity tracking and auto-saving

[NAME-ASHWANIYADAV]

Description

This pull request introduces comprehensive unit tests for the idle-watcher.js module. It verifies the inactivity tracking and auto-saving logic, ensuring proper initialization, teardown without memory leaks, and edge case coverage (like correct framerate dropping to save battery and restoring on activity).

PR Category

• Bug Fix - Fixes a bug or incorrect behavior
• Feature - Adds new functionality
• Performance - Improves performance (load time, memory, rendering, etc.)
• Tests - Adds or updates test coverage
• Documentation - Updates to docs, comments, or README
• Chore / Refactor - Maintenance, cleanup, or refactoring with no behavior change
• CI/CD - Changes to CI/CD workflows and automation

Changes Made

• Created js/activity/__tests__/idle-watcher.test.js to cover _initIdleWatcher, _stopIdleWatcher, _initAutoSave, and _stopAutoSave.
• Mocked dependencies like createjs, setInterval, and global window events.
• Validated framerate throttling (dropping from 60 fps to 1 fps on idle).
• Validated wake-up logic on user activity or when music starts playing.
• Validated logic to prevent memory leaks from duplicate interval setups and event listeners.

Testing Performed

• Ran npx jest js/activity/__tests__/idle-watcher.test.js locally.
• Verified that all 13 test cases pass successfully with proper assertions.

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 updated the documentation to reflect these changes, if applicable.
• I have followed the project's coding style guidelines.
• I have run npm run lint and npx prettier --check . with no errors.
• I have addressed the code review feedback from the previous submission, if applicable.
• I have enabled "Allow edits from maintainers" (required for auto-rebase; this only affects the PR branch, not your fork).

Additional Notes for Reviewers

The tests cover all edge cases mentioned in the issue requirements, including double initialization safety and threshold timings.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.06%. Comparing base (865c4a1) to head (208a3ed).
⚠️ Report is 21 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7645   +/-   ##
=======================================
  Coverage   55.06%   55.06%           
=======================================
  Files         172      172           
  Lines       57383    57383           
=======================================
  Hits        31597    31597           
  Misses      25786    25786           

☔ 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.

[walterbender]

@vanshika2720 what do you think?

[vanshika2720]

@walterbender I went through it, and overall the tests look solid. The main behaviors are covered well, and I don't see any blocking issues. I just have a couple of minor suggestions: strengthen the saveLocally null test by also asserting that ErrorHandler.recoverable isn't called, and add a stageDirty assertion in the wake-up-on-music test. The remaining edge cases are nice-to-have improvements rather than blockers.

[NAME-ASHWANIYADAV]

@walterbender I went through it, and overall the tests look solid. The main behaviors are covered well, and I don't see any blocking issues. I just have a couple of minor suggestions: strengthen the saveLocally null test by also asserting that ErrorHandler.recoverable isn't called, and add a stageDirty assertion in the wake-up-on-music test. The remaining edge cases are nice-to-have improvements rather than blockers.

@vanshika2720 Thanks for the review! I have been addressed both - added the ErrorHandler.recoverable assertion in the null test and the stageDirty check in the wake-up-on-music test. Also rebased on latest master.

[NAME-ASHWANIYADAV]

@walterbender @vanshika2720 PTAL !!

[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.

[NAME-ASHWANIYADAV]

@walterbender @vanshika2720 Since another PR (#7729) added the base idle-watcher tests to master, I've resolved the conflict by keeping the master version and adding your requested improvements on top of it. Specifically: 1. Added the stageDirty assertion in the wakes up if music starts playing while idle test. 2. Added a new test for saveLocally being null to ensure it doesn't crash and ErrorHandler.recoverable is not called

[vanshika2720]

@NAME-ASHWANIYADAV Thanks for the updates! The tests look good overall. I just have one small suggestion: could we add a regression test for calling _initIdleWatcher() twice to ensure listeners/intervals don't accumulate? Also, jest.restoreAllMocks() in afterEach would make the cleanup a bit more robust. Other than that, this looks good to me.

[NAME-ASHWANIYADAV]

Thanks @vanshika2720! Both suggestions addressed in the latest commit:

1. jest.restoreAllMocks() - Added in afterEach alongside the existing clearAllMocks() for more robust spy cleanup.

2. Double-init regression test - Added a new test "does not accumulate listeners or intervals when called twice" that verifies:

   • The old interval is cleared via clearInterval before creating a new one
   • All 5 event listeners are removed (via removeEventListener) before re-registering
   • The new interval handle is different from the first
   • addEventListener is called exactly 10 times total (5 per init, no accumulation)

All 13 tests passing ✅

[Ashutoshx7]

@walterbender good to go now