test(infrastructure): add unit tests for background services and handlers - #125
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d49fd5565
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Import background module to register handlers on the MessageBus. | ||
| // The module creates singleton service instances and registers all | ||
| // messageBus.on() handlers at import time. | ||
| import '../../src/background/index'; |
There was a problem hiding this comment.
Importing the full background module during Jest evaluation immediately executes chrome.commands.onCommand.addListener and later chrome.alarms.onAlarm.addListener, but tests/mocks/chromeMock.ts defines neither commands nor alarms. Consequently this suite, and the storage-handler suite with the same import, aborts before any test runs with an undefined-property error; extend the Chrome mock or isolate the handler registration from service-worker startup.
Useful? React with 👍 / 👎.
| const listeners = (chrome.runtime.onMessage.addListener as jest.Mock).mock.calls; | ||
| // The last registered listener is the background handler (singleton MessageBus). | ||
| const listener = listeners[listeners.length - 1]?.[0]; |
There was a problem hiding this comment.
Even after the missing Chrome APIs are stubbed, the last runtime.onMessage listener registered by src/background/index.ts is the raw scheduler-control listener at line 2452, not the MessageBus listener created near startup. Every auth message passed here therefore returns false and is converted to NOT_HANDLED; the duplicated helper in background-storage-handlers.test.ts has the same problem. Capture the MessageBus listener explicitly rather than relying on registration order.
Useful? React with 👍 / 👎.
| const storedOrg = await storage.getOrg(mockOrg.orgId); | ||
| expect(storedOrg).toBeUndefined(); |
There was a problem hiding this comment.
When the logout handler successfully removes this org, StorageService.getOrg() returns null for a missing entry (orgs[orgId] ?? null), not undefined. Thus this assertion fails in the exact successful-logout scenario the test is intended to verify; assert null or use a nullish matcher.
Useful? React with 👍 / 👎.
| (chrome.tabs.query as jest.Mock).mockImplementation( | ||
| (_queryInfo: unknown, callback: (tabs: unknown[]) => void) => { | ||
| callback([]); | ||
| }, |
There was a problem hiding this comment.
SalesforceAuth.getActiveSalesforceTab() calls the promise form await chrome.tabs.query(queryInfo) without a callback, so this mock invokes undefined as a function and auth.login() rejects with a TypeError rather than the expected AuthError. Use mockResolvedValue([]) (or otherwise support the promise overload) so the test reaches the no-Salesforce-tab branch.
Useful? React with 👍 / 👎.
1 participant
Summary
Addresses #40, #105 - test-infrastructure
🤖 Generated with Claude Code