test: add coverage suite for the onboarding PermissionsStep#333
Conversation
quiet-node
left a comment
quiet-node
left a comment
There was a problem hiding this comment.
Thanks for shipping this for #137. Looking great but some feedback below.
Btw one thing is could you add Closes #137 or Fixes #137 in the PR description so this PR auto-close the issue on merge. Addresses #137 links it but leaves the issue open.
| check_accessibility_permission: false, | ||
| }; | ||
|
|
||
| describe('PermissionsStep', () => { |
There was a problem hiding this comment.
Non-blocking: Co-located path is right for #137, but src/__tests__/OnboardingView.test.tsx already tests only PermissionsStep (misnamed). Heavy overlap with this suite.
Prefer migrate/trim the old file, or drop duplicated cases here, so we don't maintain two copies.
01b5eb9 to
71c278a
Compare
| expect(invoke).toHaveBeenCalledWith('request_screen_recording_access'); | ||
| expect(invoke).toHaveBeenCalledWith('open_screen_recording_settings'); | ||
| expect( | ||
| screen.getAllByRole('button', { name: 'Checking...' }).length, |
There was a problem hiding this comment.
Nit: getAllByRole already throws on zero matches, so .length).toBeGreaterThan(0) can't fail. Use getByRole. Same at 275.
| it('skips starting Accessibility polling when unmounted right after settings succeed', async () => { | ||
| let resolveOpenSettings: () => void = () => {}; | ||
| enableChannelCaptureWithResponses(BASE_RESPONSES); | ||
| vi.useFakeTimers(); |
There was a problem hiding this comment.
Fake timers restored only by the trailing vi.useRealTimers(). setup.ts restores mocks, not timers, so a mid-test throw leaks fake timers into the rest of the file. Move the restore into an afterEach. Same at 449, 495, 578, 626, 675.
| ...BASE_RESPONSES, | ||
| reset_and_relaunch_for_grant: false, | ||
| open_accessibility_settings: undefined, | ||
| }); |
There was a problem hiding this comment.
Nit: open_accessibility_settings: undefined is a no-op, fallthrough returns undefined anyway. Same at 101.
|
btw Thanks, Blocking: the duplicate suite is still here. Withdrawing my |
|
Hi Logan, thanks for your feedback! Just pushed some changes to address your comments |
Heyy @andrewholzbach LGTM! Merged! Thanks much for the effort! |
2 participants
Description
Closes #137. Adds a test suite for
PermissionsStep, the onboarding screen that gates the app on macOS Accessibility and Screen Recording permissions, which previously had no dedicated tests.Changes
src/view/onboarding/__tests__/PermissionsStep.test.tsx: covers initial render states, button interactions, the Accessibility/Screen Recording grant-and-poll flows, the resume-after-relaunch flow, and eachmountedRef/in-flight guard, bringing the component to 100% line/branch/function/statement coverage.PermissionsStep.tsxitself this is test-only. Note: the component currently has notry/catcharound itsinvoke()calls, so there's no reject-path/error-handling behavior to test yet; the suite covers every guard and branch that actually exists in the source.Checklist
bun run test:all:coveragepasses with 100% coveragebun run validate-buildpasses with zero warnings and zero errorsfeat:,fix:, etc.) so the changelog is generated automatically