fix: update SpeechProvider tests for ElevenLabs API key integration#2135
fix: update SpeechProvider tests for ElevenLabs API key integration#2135rustforrecess wants to merge 2 commits into
Conversation
- Add missing ElevenLabs fields to reducer test initialState - Add CHANGE_ELEVENLABS_API_KEY test to reducer tests - Mock tts module in actions tests to fix store.getState() crash - Mock window.speechSynthesis for JSDOM compatibility - Update changeVoice test to use mockStore dispatch (now a thunk) - Add changeElevenLabsApiKey action test Closes cboard-org#2052 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates SpeechProvider unit tests to align with the recent ElevenLabs API key/state integration so the Speech-related Jest suites pass again.
Changes:
- Updated reducer test
initialStateto include newly added ElevenLabs-related state fields. - Added reducer coverage for
CHANGE_ELEVENLABS_API_KEY. - Updated actions tests to accommodate thunk-based
changeVoiceand mockedtts/speech synthesis dependencies.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/providers/SpeechProvider/__tests__/SpeechProvider.reducer.test.js |
Aligns reducer test expectations with updated reducer initial state and adds API key action coverage. |
src/providers/SpeechProvider/__tests__/SpeechProvider.actions.test.js |
Adjusts action tests for thunk dispatch and adds mocks/new action coverage for ElevenLabs API key changes. |
| elevenLabsApiKey: 'sk-test-key' | ||
| }; | ||
| expect(speechProviderReducer(initialState, changeElevenLabsApiKey)).toEqual( | ||
| { | ||
| ...initialState, | ||
| elevenLabsApiKey: 'sk-test-key' |
There was a problem hiding this comment.
The test value "sk-test-key" looks like a real secret/API-key prefix and can trigger secret scanning / false-positive credential alerts. Prefer a clearly non-secret placeholder (e.g., "test-api-key" / "ELEVENLABS_API_KEY") to avoid CI/security tooling noise.
| elevenLabsApiKey: 'sk-test-key' | |
| }; | |
| expect(speechProviderReducer(initialState, changeElevenLabsApiKey)).toEqual( | |
| { | |
| ...initialState, | |
| elevenLabsApiKey: 'sk-test-key' | |
| elevenLabsApiKey: 'test-api-key' | |
| }; | |
| expect(speechProviderReducer(initialState, changeElevenLabsApiKey)).toEqual( | |
| { | |
| ...initialState, | |
| elevenLabsApiKey: 'test-api-key' |
There was a problem hiding this comment.
Good catch! Renamed to test-api-key in the latest push.
| global.window.speechSynthesis = { | ||
| getVoices: jest.fn(() => [ | ||
| { voiceURI: 'test-voice-en', lang: 'en-US', name: 'Test Voice' } | ||
| ]), | ||
| speak: jest.fn(), | ||
| cancel: jest.fn(), | ||
| pause: jest.fn(), | ||
| resume: jest.fn() | ||
| }; |
There was a problem hiding this comment.
This file already gets a window.speechSynthesis mock from src/setupTests.js. Overriding it here with a new plain object is redundant and can drift from the shared mock, making failures harder to diagnose. Prefer relying on the shared setup mock, or patch only the specific methods/return values needed for this suite (e.g., adjust getVoices) in a beforeAll/beforeEach.
| global.window.speechSynthesis = { | |
| getVoices: jest.fn(() => [ | |
| { voiceURI: 'test-voice-en', lang: 'en-US', name: 'Test Voice' } | |
| ]), | |
| speak: jest.fn(), | |
| cancel: jest.fn(), | |
| pause: jest.fn(), | |
| resume: jest.fn() | |
| }; | |
| global.window.speechSynthesis.getVoices.mockImplementation(() => [ | |
| { voiceURI: 'test-voice-en', lang: 'en-US', name: 'Test Voice' } | |
| ]); |
There was a problem hiding this comment.
Thanks for the suggestion! I looked into this but unfortunately setupTests.js defines getVoices as a plain function (not a jest.fn()), so .mockImplementation() isn't available on it. The cancel mock on line 36 of setupTests.js is a jest.fn(), but getVoices on line 33-35 is just a regular arrow function attached via Object.defineProperty.
Since speechSynthesis is set with writable: true in setupTests.js, overriding the whole object here is safe and each test file runs in its own Jest sandbox, so it won't affect other tests.
| it('should create an action to change ElevenLabs API key', () => { | ||
| const elevenLabsApiKey = 'sk-test-key'; | ||
| const expectedAction = { | ||
| type: types.CHANGE_ELEVENLABS_API_KEY, | ||
| elevenLabsApiKey | ||
| }; |
There was a problem hiding this comment.
The test value "sk-test-key" resembles a real API key prefix and may trigger secret scanning / credential-detection tooling. Use an unmistakably fake placeholder (e.g., "test-api-key" / "ELEVENLABS_API_KEY") to avoid false positives.
There was a problem hiding this comment.
Fixed in the latest push — renamed to test-api-key.
Addresses review feedback on cboard-org#2135. Renames sk-test-key to test-api-key in both test files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Fixes failing SpeechProvider tests after the ElevenLabs API key integration (#2052).
initialStateto include the 5 new fields added by the ElevenLabs integration (elevenLabsApiKey,elevenLabsVoiceSettings, and three newoptionsfields). Added test case forCHANGE_ELEVENLABS_API_KEYaction.ttsmodule to fix thestore.getState()crash (tts now depends on the Redux store). Mockedwindow.speechSynthesisfor JSDOM compatibility. UpdatedchangeVoicetest to usemockStoredispatch since it was refactored into a thunk. Added test forchangeElevenLabsApiKeyaction creator.Test plan
npx react-scripts test --watchAll=false --testPathPattern=Speech— all 3 suites should pass (20 tests)Closes #2052
🤖 Generated with Claude Code