🧪 [testing improvement] Add error path test for getRecentAppIds

[sunnylqm]

🎯 What: The testing gap addressed was the missing error path test for getRecentAppIds when localStorage contains invalid JSON. This PR adds that test case and comprehensive coverage for the entire recent apps feature.

📊 Coverage:

• getRecentAppIds:
  • Handles window being undefined.
  • Successfully parses valid JSON arrays from localStorage.
  • Returns an empty array on invalid JSON (the requested error path).
  • Returns an empty array when the stored value is not an array.
  • Filters out non-integer values from the stored array.
• rememberRecentApp:
  • Corrects adds new app IDs to the front.
  • Handles deduplication of app IDs.
  • Enforces the MAX_RECENT_APP_COUNT limit.
  • Handles error cases like window being undefined or invalid appId types.

✨ Result: Increased reliability and test coverage for the recent apps management utility.

---

PR created automatically by Jules for task 11677145689250762709 started by @sunnylqm

Summary by CodeRabbit

• Tests

  • Expanded test coverage for recent app tracking functionality, including edge cases and data validation scenarios.

• Refactor

  • Exposed internal storage constants for broader reuse across the codebase.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR exports two storage-related constants from src/utils/helper.ts and adds comprehensive test coverage in src/utils/helper.test.ts for recent app ID management functions, including parsing, filtering, insertion, deduplication, and input validation logic.

Changes

Recent App Storage Constants and Testing

Layer / File(s)	Summary
Export storage constants src/utils/helper.ts	RECENT_APP_STORAGE_KEY and MAX_RECENT_APP_COUNT constants are converted from non-exported to exported module constants.
Test suite for recent app functionality src/utils/helper.test.ts	Test imports updated to use Bun's mock and import newly exported constants; new RecentAppIds describe block validates getRecentAppIds parsing, JSON error handling, array filtering, and rememberRecentApp insertion, deduplication, limit enforcement, and input validation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Constants now shared across the land,
Tests ensure they work as planned,
Recent apps stored with careful grace,
Deduplication keeps its pace! 📝✨

🚥 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 directly matches the main objective of the PR: adding comprehensive tests for getRecentAppIds error paths and expanding test coverage.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 testing-getRecentAppIds-error-path-11677145689250762709

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/utils/helper.test.ts (1)

111-217: ⚡ Quick win

Restore window in an afterEach hook to avoid leaking global state.

Every test restores (global as any).window = originalWindow only at the end of the test body. If any assertion throws first, the restore is skipped, leaving window undefined or mocked and corrupting subsequent tests. An afterEach hook runs even when a test fails and also removes the duplicated restore lines.

♻️ Proposed refactor

-import { describe, expect, test, mock } from 'bun:test';
+import { afterEach, describe, expect, test, mock } from 'bun:test';

 describe('RecentAppIds', () => {
   const originalWindow = (global as any).window;
 
+  afterEach(() => {
+    (global as any).window = originalWindow;
+  });
+
   test('getRecentAppIds should return empty array when window is undefined', () => {
     (global as any).window = undefined;
     expect(getRecentAppIds()).toEqual([]);
-    (global as any).window = originalWindow;
   });

Then drop the trailing (global as any).window = originalWindow; line from each remaining test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/helper.test.ts` around lines 111 - 217, Tests set and restore
(global as any).window inline which can leak global state on failures; add an
afterEach hook that always restores (global as any).window = originalWindow
(using the existing originalWindow constant declared in the
describe('RecentAppIds') block) and then remove the per-test trailing restores
from tests that currently reset window manually (so tests like those exercising
getRecentAppIds and rememberRecentApp only set (global as any).window and rely
on the shared afterEach to restore it).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/utils/helper.test.ts`:
- Around line 111-217: Tests set and restore (global as any).window inline which
can leak global state on failures; add an afterEach hook that always restores
(global as any).window = originalWindow (using the existing originalWindow
constant declared in the describe('RecentAppIds') block) and then remove the
per-test trailing restores from tests that currently reset window manually (so
tests like those exercising getRecentAppIds and rememberRecentApp only set
(global as any).window and rely on the shared afterEach to restore it).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 337e24d4-ebd8-435f-9128-986db44bf81a

📥 Commits

Reviewing files that changed from the base of the PR and between 972bf24 and 338833c.

📒 Files selected for processing (2)

• src/utils/helper.test.ts
• src/utils/helper.ts