diff --git a/src/renderer/__helpers__/test-utils.tsx b/src/renderer/__helpers__/test-utils.tsx index e6cd951e7..b631e63d0 100644 --- a/src/renderer/__helpers__/test-utils.tsx +++ b/src/renderer/__helpers__/test-utils.tsx @@ -110,10 +110,3 @@ export function renderWithAppContext( ), }); } - -/** - * Ensure stable snapshots for our randomized emoji use-cases - */ -export function ensureStableEmojis() { - globalThis.Math.random = vi.fn(() => 0.1); -} diff --git a/src/renderer/__helpers__/vitest.setup.ts b/src/renderer/__helpers__/vitest.setup.ts index 30b4e80a5..4de8bbb7b 100644 --- a/src/renderer/__helpers__/vitest.setup.ts +++ b/src/renderer/__helpers__/vitest.setup.ts @@ -11,6 +11,11 @@ vi.mock('react-router-dom', async () => ({ useNavigate: () => navigateMock, })); +// Ensure stability in EmojiSplash component snapshots +vi.mock('../utils/core/random', () => ({ + randomElement: vi.fn((arr: unknown[]) => arr[0]), +})); + // Sets timezone to UTC for consistent date/time in tests and snapshots process.env.TZ = 'UTC'; diff --git a/src/renderer/components/AllRead.test.tsx b/src/renderer/components/AllRead.test.tsx index e1ae22955..1f474a42e 100644 --- a/src/renderer/components/AllRead.test.tsx +++ b/src/renderer/components/AllRead.test.tsx @@ -1,19 +1,12 @@ import { act } from '@testing-library/react'; -import { - ensureStableEmojis, - renderWithAppContext, -} from '../__helpers__/test-utils'; +import { renderWithAppContext } from '../__helpers__/test-utils'; import { mockSettings } from '../__mocks__/state-mocks'; import { useFiltersStore } from '../stores'; import { AllRead } from './AllRead'; describe('renderer/components/AllRead.tsx', () => { - beforeEach(() => { - ensureStableEmojis(); - }); - it('should render itself & its children - no filters', async () => { let tree: ReturnType | null = null; diff --git a/src/renderer/components/AllRead.tsx b/src/renderer/components/AllRead.tsx index bd9b7347f..f56526abe 100644 --- a/src/renderer/components/AllRead.tsx +++ b/src/renderer/components/AllRead.tsx @@ -5,6 +5,7 @@ import { Constants } from '../constants'; import { EmojiSplash } from './layout/EmojiSplash'; import { useFiltersStore } from '../stores'; +import { randomElement } from '../utils/core/random'; interface AllReadProps { fullHeight?: boolean; @@ -15,13 +16,7 @@ export const AllRead: FC = ({ }: AllReadProps) => { const hasFilters = useFiltersStore((s) => s.hasActiveFilters()); - const emoji = useMemo( - () => - Constants.EMOJIS.ALL_READ[ - Math.floor(Math.random() * Constants.EMOJIS.ALL_READ.length) - ], - [], - ); + const emoji = useMemo(() => randomElement(Constants.EMOJIS.ALL_READ), []); const heading = `No new ${hasFilters ? 'filtered ' : ''} notifications`; diff --git a/src/renderer/components/Oops.test.tsx b/src/renderer/components/Oops.test.tsx index 4801bfa13..a7c79f8ae 100644 --- a/src/renderer/components/Oops.test.tsx +++ b/src/renderer/components/Oops.test.tsx @@ -3,19 +3,11 @@ import userEvent from '@testing-library/user-event'; import { PersonIcon } from '@primer/octicons-react'; -import { - ensureStableEmojis, - navigateMock, - renderWithAppContext, -} from '../__helpers__/test-utils'; +import { navigateMock, renderWithAppContext } from '../__helpers__/test-utils'; import { Oops } from './Oops'; describe('renderer/components/Oops.tsx', () => { - beforeEach(() => { - ensureStableEmojis(); - }); - it('should render itself & its children - specified error', async () => { const mockError = { title: 'Error title', diff --git a/src/renderer/components/Oops.tsx b/src/renderer/components/Oops.tsx index cc2d50212..3c6dbf71d 100644 --- a/src/renderer/components/Oops.tsx +++ b/src/renderer/components/Oops.tsx @@ -8,6 +8,7 @@ import { EmojiSplash } from './layout/EmojiSplash'; import type { GitifyError } from '../types'; import { Errors } from '../utils/core/errors'; +import { randomElement } from '../utils/core/random'; interface OopsProps { error: GitifyError; @@ -21,10 +22,7 @@ export const Oops: FC = ({ const err = error ?? Errors.UNKNOWN; const navigate = useNavigate(); - const emoji = useMemo( - () => err.emojis[Math.floor(Math.random() * err.emojis.length)], - [err], - ); + const emoji = useMemo(() => randomElement(err.emojis), [err]); const actions = err.actions?.length ? err.actions.map((action) => ( diff --git a/src/renderer/components/__snapshots__/AllRead.test.tsx.snap b/src/renderer/components/__snapshots__/AllRead.test.tsx.snap index 71b0bdce3..dcc9d188e 100644 --- a/src/renderer/components/__snapshots__/AllRead.test.tsx.snap +++ b/src/renderer/components/__snapshots__/AllRead.test.tsx.snap @@ -41,10 +41,10 @@ exports[`renderer/components/AllRead.tsx > should render itself & its children - class="text-7xl" > 🎊
should render itself & its children - class="text-7xl" > 🎊
({ - ...(await vi.importActual('react-router-dom')), - useNavigate: () => navigateMock, -})); - vi.mock('./RepositoryNotifications', () => ({ RepositoryNotifications: () =>
RepositoryNotifications
, })); describe('renderer/components/notifications/AccountNotifications.tsx', () => { - beforeEach(() => { - ensureStableEmojis(); - }); - it('should render itself - group notifications by repositories', () => { const props: AccountNotificationsProps = { account: mockGitHubCloudAccount, diff --git a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap index 3fe7a42a4..11c50885c 100644 --- a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap @@ -1808,10 +1808,10 @@ exports[`renderer/components/notifications/AccountNotifications.tsx > should ren class="text-7xl" > 🎊
{ + it('should return an element from the array', () => { + for (let i = 0; i < 100; i++) { + const result = randomElement(FRUITS); + expect(FRUITS).toContain(result); + } + }); + + it('should use crypto.getRandomValues', () => { + const spy = vi.spyOn(globalThis.crypto, 'getRandomValues'); + randomElement(FRUITS); + expect(spy).toHaveBeenCalledOnce(); + spy.mockRestore(); + }); + + it('should return first element when crypto returns 0', () => { + vi.spyOn(globalThis.crypto, 'getRandomValues').mockImplementation( + (array: T): T => { + if (array instanceof Uint32Array) { + array[0] = 0; + } + return array; + }, + ); + + expect(randomElement(FRUITS)).toBe('apple'); + vi.restoreAllMocks(); + }); + + it('should wrap large values via modulo', () => { + vi.spyOn(globalThis.crypto, 'getRandomValues').mockImplementation( + (array: T): T => { + if (array instanceof Uint32Array) { + array[0] = 13; + } + return array; + }, + ); + + expect(randomElement(FRUITS)).toBe('date'); // 13 % 5 === 3 + vi.restoreAllMocks(); + }); +}); diff --git a/src/renderer/utils/core/random.ts b/src/renderer/utils/core/random.ts new file mode 100644 index 000000000..99df3597b --- /dev/null +++ b/src/renderer/utils/core/random.ts @@ -0,0 +1,10 @@ +/** + * Returns a cryptographically random element from the given array. + * Uses crypto.getRandomValues instead of Math.random to satisfy + * secure-coding requirements. + */ +export function randomElement(array: T[]): T { + const buf = new Uint32Array(1); + crypto.getRandomValues(buf); + return array[buf[0] % array.length]; +}