Increase unit test coverage and optimize Paparazzi snapshots#6555
Increase unit test coverage and optimize Paparazzi snapshots#6555andremion wants to merge 6 commits into
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
SDK Size Comparison 📏
|
WalkthroughThis PR adds extensive new unit test coverage across client offline mapping/converters, core model classes, ui-common features, and Compose UI/ViewModels, and migrates Compose Paparazzi snapshot tests from ChangesClient and Core Test Coverage
Compose Paparazzi Migration and UI Tests
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatarTest.kt (1)
21-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMagic
screenHeight = 2182lacks explanatory comment.The switch from
DeviceConfig.PIXEL_4A.copy(screenHeight = 4000)toPIXEL_4A_HDPI.copy(screenHeight = 2182)is consistent with the PR's HDPI migration, but the specific pixel value isn't self-explanatory. A brief comment on why 2182 was chosen (vs. the default HDPI height) would aid future maintainers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatarTest.kt` around lines 21 - 32, Add a brief explanatory comment in ChannelAvatarTest around the Paparazzi deviceConfig setup so the custom PIXEL_4A_HDPI screenHeight = 2182 value is clearly justified. Keep the note near the paparazzi rule and mention that this height was chosen to match the HDPI migration/test baseline rather than using the default height, so future readers understand why the magic number is needed.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.kt (1)
210-305: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
randomAttachmentEntity/randomReplyAttachmentEntitybodies.Both factories share identical parameter lists and construction logic, differing only in the return type. Could extract a shared parameter holder or builder to avoid drift between the two as
AttachmentEntity/ReplyAttachmentEntityevolve.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.kt` around lines 210 - 305, The two factory helpers randomAttachmentEntity and randomReplyAttachmentEntity are duplicated and can drift apart as AttachmentEntity and ReplyAttachmentEntity change. Refactor the shared parameter list and construction logic into a common helper or parameter holder, then have both functions delegate to it while preserving their distinct return types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.kt`:
- Around line 210-305: The two factory helpers randomAttachmentEntity and
randomReplyAttachmentEntity are duplicated and can drift apart as
AttachmentEntity and ReplyAttachmentEntity change. Refactor the shared parameter
list and construction logic into a common helper or parameter holder, then have
both functions delegate to it while preserving their distinct return types.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatarTest.kt`:
- Around line 21-32: Add a brief explanatory comment in ChannelAvatarTest around
the Paparazzi deviceConfig setup so the custom PIXEL_4A_HDPI screenHeight = 2182
value is clearly justified. Keep the note near the paparazzi rule and mention
that this height was chosen to match the HDPI migration/test baseline rather
than using the default height, so future readers understand why the magic number
is needed.
|
1 participant
Goal
Increase unit test coverage with a package of quick wins: tests for classes that are plain JVM logic and currently have low or zero coverage. While adding snapshot tests, we also found ways to make the Paparazzi suite cheaper and more complete, so this PR includes that work too.
Resolves AND-1297
📈 Coverage highlights
About 220 new unit tests across 4 modules:
coreclientui-commoncomposeBlended line and branch coverage per class, measured from the merged Kover report:
AudioPlayerControllerHttpLoggingInterceptorQuerySortParserAnswer/Option/Vote)UserAttachmentMapperReactionDraftMessageChannelMessageChannelOptionsEstimated overall Sonar impact: about +3 percentage points (68.9% today). The exact number comes from the CI Sonar run.
📸 Paparazzi snapshot optimization
All snapshot tests now render at hdpi with the SHRINK rendering mode (shared
PIXEL_2_HDPI/PIXEL_4A_HDPIconfigs), and the dark/light helpers useweight(fill = false). Goldens contain only content, no device frame filler, while visual quality stays on par with the previous snapshots.xychart-beta title "Golden set size (millions of pixels)" x-axis ["Before", "After"] y-axis "Megapixels" 0 --> 200 bar [178.6, 126.6]Fewer pixels means cheaper snapshot comparisons on every CI run, and the set grew by 15 goldens at the same time.
An audit of all snapshot tests also:
Implementation
random*helpers, sibling converter/mapper tests, theFakeChainharness, andPaparazziComposeTest. Plain JUnit 5 everywhere, no new Robolectric usage, so CI time stays flat.randomAnswerEntity/randomOptionEntity/randomVoteEntityfixtures in the client testMother.kt.MessageReminder.Builder(reminder)now copiescidandmessageId(the setters existed but the copy constructor skipped both, so a builder round trip silently dropped them).Channel.Builderdoes not carrypendingMessagesandReaction.Builderdoes not carryskipPush. Fixing those needs new setters (API additions), so they are candidates for separate tickets.🎨 UI Changes
No production UI changes. All golden image changes come from the rendering density and layout changes described above.
Testing
./gradlew :testCoveragepasses (all unit tests of the coverage modules, including the new ones)../gradlew :stream-chat-android-compose:verifyPaparazziDebugpasses against the re-recorded goldens (394 goldens, one per snapshot test).spotlessCheck,detekt, andapiCheckare clean (the production fix does not change the API surface).To review the snapshot changes: open a few re-recorded goldens in the diff and check that content is complete and readable, and that dark and light variants are present.