[MOB-12193] add tests for iterableinboxmessagedisplay #762
Conversation
lposen
added
the
tests
|
Diff Coverage: The code coverage on the diff in this pull request is 100.0%. Total Coverage: This PR will increase coverage by 10.34%. File Coverage Changes
🛟 Help
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
There was a problem hiding this comment.
Pull Request Overview
Adds comprehensive test suites for IterableInboxMessageDisplay and (existing) IterableInboxMessageList, plus minor formatting/documentation changes in Iterable core class and updates eslint ignore patterns.
- Introduces extensive rendering, interaction, and edge-case tests for IterableInboxMessageDisplay.
- Adds broad (but shallow) rendering and prop variation tests for IterableInboxMessageList.
- Adjusts require style in Iterable.ts and removes an Android-specific doc tag; adds coverage/ to .eslintignore.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| src/inbox/components/IterableInboxMessageList.test.tsx | New test suite covering rendering and prop variations for message list. |
| src/inbox/components/IterableInboxMessageDisplay.test.tsx | New detailed test suite for display component interactions, async content, and handlers. |
| src/core/classes/Iterable.ts | Reformats lazy require block and removes Android-only doc tag from wakeApp JSDoc. |
| .eslintignore | Adds coverage/ directory to ignore list. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| it('should handle updateVisibleMessageImpressions with different implementations', () => { | ||
| const customUpdateCallback = jest.fn((rowInfos: IterableInboxImpressionRowInfo[]) => { | ||
| // Custom implementation | ||
| rowInfos.forEach(rowInfo => { | ||
| console.log(`Message ${rowInfo.messageId} is visible`); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Using console.log in tests adds noise and slows execution. Remove the logging or replace with assertions on rowInfos after invoking the viewability handler to keep tests focused and deterministic.
| it('should handle very large number of row view models', () => { | ||
| const largeRowViewModels = Array.from({ length: 1000 }, (_, i) => ({ | ||
| inAppMessage: new IterableInAppMessage( | ||
| `messageId${i}`, | ||
| i, | ||
| new IterableInAppTrigger(IterableInAppTriggerType.immediate), | ||
| new Date(), | ||
| undefined, | ||
| true, | ||
| new IterableInboxMetadata(`Title ${i}`, `Subtitle ${i}`, `imageUrl${i}.png`), | ||
| undefined, | ||
| false, | ||
| i | ||
| ), | ||
| title: `Title ${i}`, | ||
| subtitle: `Subtitle ${i}`, | ||
| imageUrl: `imageUrl${i}.png`, | ||
| read: false, | ||
| createdAt: new Date(), | ||
| })); | ||
|
|
||
| const propsWithLargeData = { ...defaultProps, rowViewModels: largeRowViewModels }; | ||
| expect(() => render(<IterableInboxMessageList {...propsWithLargeData} />)).not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
Generating 1000 complex objects increases test time; reduce to a representative subset (e.g., 50–100) or mock rowViewModels minimally to validate scalability while keeping suite fast.
| deleteRow: undefined as unknown as (messageId: string) => void, | ||
| handleMessageSelect: undefined as unknown as (messageId: string, index: number) => void, | ||
| messageListItemLayout: undefined as unknown as (last: boolean, rowViewModel: IterableInboxRowViewModel) => [React.ReactElement | null, number], |
There was a problem hiding this comment.
Casting undefined to function types with unknown as masks type safety; if these props are optional, pass undefined directly, otherwise provide no-op jest.fn() to clearly express intent.
| deleteRow: undefined as unknown as (messageId: string) => void, | |
| handleMessageSelect: undefined as unknown as (messageId: string, index: number) => void, | |
| messageListItemLayout: undefined as unknown as (last: boolean, rowViewModel: IterableInboxRowViewModel) => [React.ReactElement | null, number], | |
| deleteRow: undefined, | |
| handleMessageSelect: undefined, | |
| messageListItemLayout: undefined, |
| expect(Iterable.trackInAppClose).toHaveBeenCalledWith( | ||
| mockRowViewModel.inAppMessage, | ||
| 1, // IterableInAppLocation.inbox | ||
| 0 // IterableInAppCloseSource.back | ||
| ); |
There was a problem hiding this comment.
Hard-coded enum numeric values reduce clarity and may break if enum ordering changes; import and use the actual enum constants (e.g., IterableInAppLocation.inbox, IterableInAppCloseSource.back) in the expectation.
| expect(Iterable.trackInAppClick).toHaveBeenCalledWith( | ||
| mockRowViewModel.inAppMessage, | ||
| 1, // IterableInAppLocation.inbox | ||
| 'https://example.com' | ||
| ); |
There was a problem hiding this comment.
Replace the magic number 1 with the corresponding enum constant for location to improve readability and resilience to enum changes.
| expect(Iterable.trackInAppClose).toHaveBeenCalledWith( | ||
| mockRowViewModel.inAppMessage, | ||
| 1, // IterableInAppLocation.inbox | ||
| 1, // IterableInAppCloseSource.link | ||
| 'https://example.com' | ||
| ); |
There was a problem hiding this comment.
Use enum constants instead of raw numeric values (1) for location and close source to avoid magic numbers in assertions.
| type: 'customAction', | ||
| data: 'action://customAction', | ||
| userInput: '', | ||
| }), | ||
| expect.objectContaining({ | ||
| action: expect.objectContaining({ | ||
| type: 'customAction', | ||
| data: 'action://customAction', | ||
| userInput: '', | ||
| }), | ||
| source: 2, // IterableActionSource.inApp | ||
| }) |
There was a problem hiding this comment.
The magic number 2 for action source should be replaced with the enum (IterableActionSource.inApp) to make the test self-explanatory and resilient.
| 'myapp://deep-link', | ||
| expect.objectContaining({ | ||
| action: expect.objectContaining({ | ||
| type: 'openUrl', | ||
| data: 'myapp://deep-link', | ||
| userInput: '', | ||
| }), | ||
| source: 2, // IterableActionSource.inApp | ||
| }) |
There was a problem hiding this comment.
Replace numeric source value 2 with the corresponding enum constant for clarity and to avoid brittle coupling to enum ordering.
| const originalError = console.error; | ||
| beforeAll(() => { | ||
| console.error = jest.fn(); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| console.error = originalError; |
There was a problem hiding this comment.
Overriding console.error globally can hide unexpected errors; prefer jest.spyOn(console, 'error') and selectively filtering known warnings or restoring after each test to avoid masking failures.
| const originalError = console.error; | |
| beforeAll(() => { | |
| console.error = jest.fn(); | |
| }); | |
| afterAll(() => { | |
| console.error = originalError; | |
| beforeEach(() => { | |
| jest.spyOn(console, 'error').mockImplementation((...args) => { | |
| // Suppress only act() warnings, log others | |
| const actWarning = /Warning: An update to .* inside a test was not wrapped in act/; | |
| if (typeof args[0] === 'string' && actWarning.test(args[0])) { | |
| return; | |
| } | |
| // Call the original error for other messages | |
| // @ts-ignore | |
| return console.error.mock.calls.length ? undefined : console.error(...args); | |
| }); | |
| }); | |
| afterEach(() => { | |
| // @ts-ignore | |
| if (console.error.mockRestore) { | |
| console.error.mockRestore(); | |
| } |
| returnToInbox: undefined as unknown as (callback?: () => void) => void, | ||
| deleteRow: undefined as unknown as (id: string) => void, |
There was a problem hiding this comment.
Casting undefined to function types reduces type safety; if these props are optional pass undefined directly, or use no-op jest.fn() to communicate intent without unsafe casts.
| returnToInbox: undefined as unknown as (callback?: () => void) => void, | |
| deleteRow: undefined as unknown as (id: string) => void, | |
| returnToInbox: undefined, | |
| deleteRow: undefined, |
…l' into MOB-12193-add-tests-for-iterableinboxmessagedisplay
|
…l' into MOB-12193-add-tests-for-iterableinboxmessagedisplay
…mprove test readability
…l' into MOB-12192-add-tests-for-iterableinboxmessagelist
…MOB-12193-add-tests-for-iterableinboxmessagedisplay
🔹 JIRA Ticket(s) if any
✏️ Description
Created tests for
IterableInboxMessageDisplay