test(medium): Fix component test errors and suppress console noise

[arii]

Description

This change addresses the issue of cluttered test logs by suppressing expected console messages in various unit tests using Jest spies. It specifically fixes the reported 'QuotaExceeded' error noise and other identified log points across hooks, components, and API route tests.

Fixes #9442

Change Type: 🐛 Bug fix (non-breaking change fixing an issue)

Related Issues

Closes #9442

Changes Made

• Suppressed expected console messages in various unit tests using Jest spies.
• Fixed 'QuotaExceeded' error noise and other identified log points across hooks, components, and API route tests.

Testing

• The changes primarily impact testing by improving test log clarity and resolving console noise.

Original PR Body

This change addresses the issue of cluttered test logs by suppressing expected console messages in various unit tests using Jest spies. It specifically fixes the reported 'QuotaExceeded' error noise and other identified log points across hooks, components, and API route tests.

Fixes #9442

---

PR created automatically by Jules for task 12837448378175141024 started by @arii

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

[github-actions]

👋 Welcome to HRM!

Thanks for your contribution. This repository uses Gemini AI for automated triage, code review, and generation.

🤖 Gemini Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message
@pr-squash	Squash PR commits (PR only)
@conflict-resolve	Resolve merge conflicts (PR only)

For more details and GitHub CLI examples, see the Manual Trigger Guide.

[github-actions]

ℹ️ Gemini Review Disabled

Automatic review is currently disabled via GEMINI_ENABLE_PR_REVIEW. To run review manually:

Via Comment:

@gemini-bot

Via GitHub Actions:

1. Go to Actions → Reusable Gemini Review
2. Click "Run workflow"
3. Enter PR number: 9444

Via GitHub CLI:

gh workflow run reusable-gemini-review.yml -f pr_number=9444

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Full reference: Manual Trigger Guide

To re-enable: Set repository variable GEMINI_ENABLE_PR_REVIEW=true

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is disabled

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is disabled

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[arii]

🤖 AI Technical Audit

Code Review: PR #9444

Summary

This PR addresses test hygiene by suppressing expected console noise in Jest tests. While the intent is correct, the implementation introduces a significant risk of test pollution due to unsafe spy restoration handling. If an assertion fails before mockRestore() is called, the console spies will leak into subsequent tests, making debugging difficult.

⚠️ ANTI-AI-SLOP DIRECTIVES

1. UNSAFE SPY RESTORATION (CRITICAL): In almost every modified test file (e.g., api/spotify/access-token/route.test.ts), mockRestore() is placed at the end of the test function. If any expect assertion fails earlier in the test, the function exits immediately, and mockRestore() is never executed. This permanently suppresses console output for all subsequent tests in the suite.
2. CODE RATIO / REPETITION: The PR adds repetitive setup/teardown logic inside individual tests (approx. 4-6 lines per test). This should be abstracted into a beforeAll/afterAll block or a custom helper to reduce boilerplate and ensure safety.
3. LOGGER MOCK PATH: In tests/unit/app/api/internal/token-delivery/route.test.ts, the mock path was changed from @/utils/logger.server to @/utils/logger. Ensure the implementation file actually imports from @/utils/logger, otherwise, the mock will be ignored.

---

📁 File-by-File Analysis

🚨 General Issue (Multiple Files)

Problem: Unsafe mockRestore calls. If an assertion throws, the spy remains active.
Files:

• tests/unit/app/api/spotify/access-token/route.test.ts
• tests/unit/app/api/spotify/devices/route.test.ts
• tests/unit/app/api/spotify/playlists/route.test.ts
• tests/unit/components/GoogleDocViewer.test.tsx
• tests/unit/components/SpotifyDisplay.test.tsx
• tests/unit/hooks/usePersistentStorage.test.ts
• tests/unit/lib/env.test.ts

Implementation Sample (Current - Unsafe):

it('should return 401', async () => {
  const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {})
  // ... code ...
  expect(response.status).toBe(401) // ⬅️ If this fails, the test stops here.
  errorSpy.mockRestore() // ⬅️ This line is never reached.
})

Recommended Fix (Using finally or afterEach):

// Option A: Try/Finally (Local)
it('should return 401', async () => {
  const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {})
  try {
    // ... code ...
    expect(response.status).toBe(401)
  } finally {
    errorSpy.mockRestore()
  }
})

// Option B: Lifecycle (Cleaner)
describe('API Route', () => {
  beforeEach(() => {
    jest.spyOn(console, 'error').mockImplementation(() => {})
  })
  afterEach(() => {
    jest.restoreAllMocks()
  })
  // ... tests without manual spy handling ...
})

tests/unit/app/api/internal/token-delivery/route.test.ts

Problem: Verify the import change.

// Old
jest.mock('@/utils/logger.server', ...)
// New
jest.mock('@/utils/logger', ...)

Action: Verify that the route.ts file under test imports logger from @/utils/logger and not @/utils/logger.server. If it imports the .server version, this change breaks the mock.

tests/unit/hooks/useBluetoothHRM.race.test.ts

Observation: This file correctly uses beforeAll and afterAll. This is the preferred approach over the per-test manual restoration seen in other files.

🛠️ Architectural Impact

Low. This is test-only code. However, improper test teardown can lead to flaky CI pipelines, which slows down development velocity.

🎯 Best Practices

• Test Isolation: Ensure side effects (like global console mocks) are cleaned up even when tests fail.
• DRY (Don't Repeat Yourself): Use jest.restoreAllMocks() in afterEach rather than manually restoring specific spies in every test case.

Review automatically published via RepoAuditor.

[arii]

🤖 AI Technical Audit

Code Review for PR #9444

🛡️ Anti-AI-Slop Directives

1. CODE RATIO: The file tests/unit/lib/env.test.ts contains unnecessary churn. You moved two test cases ('should throw an error for invalid environment variables' and 'should throw an error if Spotify credentials...') from the bottom of the file to the top just to wrap them in spies. This creates a noisy diff (deleting them and re-adding them). Recommendation: Keep them in place or minimize the move to reduce diff size.
2. TEST POLLUTION RISK (CRITICAL): The pattern used to suppress console logs is unsafe and likely to cause "test pollution" where one failing test silences the console for all subsequent tests.

---

🚨 Critical Architectural Issues

1. Unsafe Spy Restoration (Test Pollution)

In almost all updated test files (e.g., access-token/route.test.ts, GoogleDocViewer.test.tsx, SpotifyDisplay.test.tsx), you are instantiating a spy, running assertions, and then restoring the spy.

Problem: If an expect(...) call fails, the test throws an error immediately and execution stops. The line spy.mockRestore() is never reached. This leaves console.error (or info/log) mocked/suppressed for every subsequent test in the suite. This makes debugging CI failures impossible because error logs will be swallowed by the leaked spy.

Implementation Sample (Fix):
Use try/finally blocks or beforeEach/afterEach to guarantee restoration.

// BAD
it('should return 401', async () => {
  const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
  // ... test logic ...
  expect(response.status).toBe(401); // If this fails, spy is never restored
  spy.mockRestore();
});

// GOOD (Option A: Local Try/Finally)
it('should return 401', async () => {
  const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
  try {
    // ... test logic ...
    expect(response.status).toBe(401);
  } finally {
    spy.mockRestore(); // Runs even if expect fails
  }
});

// GOOD (Option B: Lifecycle methods - Preferred for repeated usage)
describe('API Route', () => {
  let errorSpy: jest.SpyInstance;
  beforeEach(() => {
    errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
  });
  afterEach(() => {
    errorSpy.mockRestore();
  });
  // ... tests ...
});

2. Mock Path Consistency

File: tests/unit/app/api/internal/token-delivery/route.test.ts

Problem: You changed the mock path from @/utils/logger.server to @/utils/logger.

- jest.mock('@/utils/logger.server', () => ({
+ jest.mock('@/utils/logger', () => ({

If the source code (token-delivery/route.ts) explicitly imports logger.server, mocking logger (a different file) might not apply the mock to the system under test, depending on how your exports are structured. Please verify that the SUT actually uses the base logger or if you are now mocking the wrong file.

---

File-by-File Feedback

tests/unit/hooks/useBluetoothHRM.race.test.ts

• Good: This file correctly uses beforeAll and afterAll to handle the spies. This is the correct pattern. Please align the other files to this standard (or use try/finally).

tests/unit/lib/env.test.ts

• Diff Noise: As mentioned in Anti-AI-Slop, please revert the reordering of tests unless strictly necessary. It obscures the actual changes (adding the spies).
• Spies: Apply the try/finally fix here as well.

tests/unit/components/GoogleDocViewer.test.tsx

• Assertion Logic:

  expect(errorSpy).toHaveBeenCalledWith('Invalid URL:', 'invalid-url')

  Ensuring the error is logged is good practice, but ensure this expectation is inside the try block so failures are reported correctly without leaking the spy.

Summary

This PR fixes console noise but introduces a significant risk of swallowing CI error logs if tests fail. Please wrap all temporary spies in try/finally blocks or use lifecycle hooks to ensure the console is restored even when assertions fail.

Review automatically published via RepoAuditor.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is disabled

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[arii]

🤖 AI Technical Audit

Code Review: PR #9444

This PR effectively addresses test log noise by implementing jest.spyOn strategies across various suites. The approach significantly improves the Developer Experience (DX) by ensuring CI logs only show genuine failures. I particularly appreciate the pattern in usePersistentStorage.test.ts where the suppression is coupled with an assertion (expect(consoleSpy).toHaveBeenCalledWith), verifying that error handling is functioning correctly rather than just silencing it.

However, there is a potential regression regarding test isolation. In several files, jest.clearAllMocks() was replaced with jest.restoreAllMocks(). While restoreAllMocks is necessary to reset console spies, it does not clear mock call history (like mock.calls.length) for standard Jest mocks. Unless your Jest config has clearMocks: true globally, this change will cause tests to leak state (call counts) into each other, leading to flaky toHaveBeenCalledTimes assertions.

⚠️ ANTI-AI-SLOP DIRECTIVES

1. CODE RATIO (Redundant Lines):
   In tests/unit/app/api/spotify/devices/route.test.ts:

   beforeEach(() => {
     jest.restoreAllMocks() // REDUNDANT: afterEach handles this.
     jest.spyOn(console, 'error').mockImplementation(() => {})
     // ...
   })

   The jest.restoreAllMocks() inside beforeEach is unnecessary because afterEach already guarantees restoration. Removing this reduces visual noise.

2. STALE FEATURES (Missing Cleanup):
   In tests/unit/app/api/spotify/access-token/route.test.ts and tests/unit/app/api/spotify/playlists/route.test.ts, the explicit jest.clearAllMocks() was removed. If you rely on mock call counts, this functionality must be preserved.

⚡ Major Issues

1. Potential Test Pollution (Missing clearAllMocks)

Problem: You replaced jest.clearAllMocks() with jest.restoreAllMocks() in afterEach. restoreAllMocks only restores original implementations (useful for spies); it does not clear the usage data of other mocks (e.g., getServerSession). If subsequent tests check expect(mock).toHaveBeenCalledTimes(1), they might fail because the count accumulates from previous tests.

Implementation Sample:
Current (Risk of pollution):

afterEach(() => {
  jest.restoreAllMocks()
})

Recommended (Safe):

afterEach(() => {
  jest.restoreAllMocks() // Restores console
  jest.clearAllMocks()   // Clears mock history
})

✅ Best Practices

• Explicit Error Assertions: Great job in tests/unit/hooks/usePersistentStorage.test.ts and GoogleDocViewer.test.tsx. Using expect(errorSpy).toHaveBeenCalledWith(...) transforms "noise suppression" into "behavior verification".
• Import Fix: The fix in token-delivery/route.test.ts (logger.server -> logger) seems to correct a drift between the test mock and the actual implementation. Ensure the source code indeed imports from @/utils/logger.

Review automatically published via RepoAuditor.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is disabled

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide