325: Add a Hilt screenshot test sample - #334
DanielJette wants to merge 2 commits into
Conversation
AndroidTestifyBot
left a comment
There was a problem hiding this comment.
Changes requested
The sample builds, the baseline matches, and the reasoning for a separate module is sound. But it doesn't exercise the recipe it exists to back, one of its stated reasons is wrong, and the Bitrise edit changes an unrelated step.
1. It doesn't verify what #325 asked for
#325 lists six unverified steps from the recipe. Steps 1–4 are: the activity in src/debug/java, the debugCompileOnly pair, that compileOnly suffices for a subclass declared in the debug manifest, and the src/debug/AndroidManifest.xml declaration. This sample puts the activity in main with plain compileOnly, so those four are still unverified, while #321 now describes this module as "this recipe, compiled and running in CI".
A library module has a debug source set too. Moving the activity and its manifest entry to src/debug and using debugCompileOnly would make the sample mirror the recipe exactly, and "Fixes #325" would then be true.
2. "cannot reference test sources" is not true here
HiltComposableTestActivity's KDoc says it lives in main "because the activity has to be declared in a merged manifest, which cannot reference test sources". I moved the activity to src/androidTest/java, declared it in src/androidTest/AndroidManifest.xml, deleted both compileOnly lines, and ran FlixHilt:screenshotTest:
OK (1 test)
Same committed baseline, pixel-identical. For a library module the test APK is the app, so the androidTest manifest is merged into it. The main + compileOnly arrangement is a choice, not a constraint; please fix the comment (and the matching sentence in #321), whichever layout you keep.
3. The Bitrise edit moves verbose: true off the FlixLibrary step
The new block is inserted between FlixLibrary's test_runner and its - verbose: true, so that input now belongs to the FlixHilt step and the FlixLibrary step silently loses it. Insert after the verbose line and give the new step its own.
4. GitHub Actions does not cover it
flix_sample.ymlalready covers it through itsSamples/Flix/**path filter.
The filter only triggers the workflow. Its steps assemble and run :FlixSample; FlixHilt is never built or tested there. Bitrise test_flix is the only coverage.
Smaller points
- New files carry
Copyright (c) 2023. androidx.test:core-ktx:1.4.0sits next torules:1.7.0andext:junit:1.3.0.
Verified
./gradlew FlixHilt:screenshotTeston a clean install, API 371080x2220@440dp:OK (1 test)against the committed baseline.- The APK lands at
Samples/Flix/FlixHilt/build/outputs/apk/androidTest/debug/FlixHilt-debug-androidTest.apk, matching the Bitrise paths. settings.gradlemerges cleanly with #333.
|
All four points addressed. Agreed on every one; two were defects I introduced. 1. The sample now mirrors the recipeYou were right that leaving the activity in Verified after uninstalling the test APK first, so it is a clean install rather than a leftover one: Same committed baseline, pixel-identical — 2. The "cannot reference test sources" claim is goneYour 3. The Bitrise
|
AndroidTestifyBot
left a comment
There was a problem hiding this comment.
Approved
All four points addressed and verified.
- The host activity is now in
src/debug/java, declared insrc/debug/AndroidManifest.xml, with both Testify artifacts asdebugCompileOnly. The sample mirrors recipe steps 1–4, soFixes #325holds. - The KDoc no longer claims a constraint, and it records the
androidTestalternative accurately. - FlixLibrary's Bitrise step has its
verbose: trueback, and FlixHilt has its own. - The description now credits Bitrise
test_flixas the only coverage. - Keeping
core-ktx:1.4.0in line with the other two samples is fine by me.
Local, on the new head, API 37 emulator, test APK uninstalled first:
$ ./gradlew FlixHilt:screenshotTest OK (1 test)
$ ./gradlew FlixSample:screenshotTest OK (22 tests)
Re-run CI before merging. Run Screenshot Tests 0 and 1 on flix_sample.yml failed with INSTRUMENTATION_RESULT: shortMsg=Process crashed. while running the FlixSample suite. Your new commit only touches Samples/Flix/FlixHilt/** and bitrise.yml, neither of which that workflow builds. The previous head passed the same workflow, and FlixSample passes locally on this head, so it looks like an emulator flake, but it should be green before merge.
What does this change accomplish?
Fixes #325
The Hilt recipe in #321 was approved but held back for having no compiled example behind it. This adds
one, so the recipe has the same footing as #318's
FullscreenCaptureExampleTests.kt.It also gives #207 an executable regression test. Comment out
@AndroidEntryPointon the hostactivity and the test reproduces that issue's exception verbatim:
How have you achieved it?
A new
Samples/Flix/FlixHiltlibrary module with:HiltComposableTestActivity— an@AndroidEntryPointsubclass ofComposableTestActivity.HiltTestRunner— substitutesHiltTestApplication.SampleViewModel— an@HiltViewModelwith no dependencies, so the render is deterministic and afailure means the injection broke rather than the data changed.
HiltComposableScreenshotTest—@HiltAndroidTestwithHiltAndroidRuleordered beforeComposableScreenshotScenarioRule, plus its recorded baseline.ComposableScreenshotRulecan't be used: it hardcodesactivityClass = ComposableTestActivity.ComposableScreenshotScenarioRulelets the test choose the host.Added to the Bitrise
test_flixworkflow, which is the only thing that exercises it.flix_sample.yml'sSamples/Flix/**path filter triggers that workflow but its steps only assembleand run
:FlixSample, so it does not cover this module.Why a separate module, rather than adding this to
:FlixSampleI tried
:FlixSamplefirst, since it already has Hilt, KSP andhiltViewModel()in use. It breakstwo existing tests, and the reason is worth recording:
testInstrumentationRunneris module-wide, so pointing Flix atHiltTestRunnerreplacesFlixApplicationwithHiltTestApplicationfor every test in the module.FlixApplicationimplements
ImageLoaderFactoryand supplies the CoilImageLoaderthatFoundationModulebuilds onan
IdlingThreadPoolExecutor. Without it, Coil falls back to its default loader on an ordinarybackground dispatcher, and the image-backed tests capture before their images have drawn:
Both pass on
main, individually and as a suite, so this was caused by the runner change and notpre-existing.
HomeScreenTestsurvived only because #327 gave it its ownDispatchers.Unconfinedloader in
@Before.A separate module leaves that suite alone. The host activity is in
src/debug/javawithdebugCompileOnly, matching the recipe step for step, so #321 can point at this as a worked examplewithout caveats. A library module's test APK is the application, so
androidTestwith anandroidTest manifest and no
compileOnlyalso works —debugis chosen to mirror the recipe, notbecause it is the only arrangement.
Scope of Impact and Testing instructions
Purely additive — a new sample module and one line in
settings.gradle. No library, plugin orexisting sample code is touched, so no CHANGELOG entry.
Verified locally on an API 37 emulator:
FlixHilt:ktlintCheckreports the same "Initial star should align" violation on the license headerthat every other sample file has, including
FlixSample's onmain. Samples are not gated onktlint; I kept the house header rather than diverging from it.