fix(shop-bento): eliminate flaky ShopBentoViewModelTest + version bump - #250
Merged
Conversation
ShopBentoViewModelTest had no MainDispatcherRule, unlike every other ViewModel test in this codebase, so Dispatchers.Main was never explicitly installed for it. ShopBentoViewModel.init launches its work via viewModelScope (backed by Dispatchers.Main), so this test was actually running against whatever Dispatchers.Main happened to be left as by other test classes sharing the same Gradle test JVM fork — explaining why `testBrainwalletReleaseUnitTest` failed intermittently with `TurbineAssertionError: Unconsumed events found` only under the full suite (order/load-dependent) and never in isolation. Added MainDispatcherRule to match the established pattern (see UnLockViewModelTest, BWSenderIntegrationTest, TxRepositoryImplIntegrationTest). Also collapsed ShopBentoViewModel's two independent viewModelScope.launch blocks (one collecting settingRepository.settings, one collecting shopProxyRepository.shopProxy) into a single combine()-driven collector. Previously each block called _state.update independently, so real observers of `state` (not just the test) could see transient, incomplete intermediate states depending on collector interleaving. Swapped the fixed-duration advanceTimeBy(100) for advanceUntilIdle() in the affected test, which deterministically drains all pending coroutine work instead of guessing a virtual-time delay. Verified with repeated full `testBrainwalletReleaseUnitTest` runs (previously failed roughly 1 in 3-5 runs under the full 415-test suite). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
kcw-grunt
previously approved these changes
Jul 22, 2026
Collaborator
There was a problem hiding this comment.
Learned something here.
| fun `init - sets shopBaseUrl from widget`() = runTest { | ||
| turbineScope { | ||
| shopProxyFlow.emit(listOf(ShopProxy(widget = "https://shop.example.com", shopCards = emptyList()))) | ||
| shopProxyFlow.emit(listOf(ShopProxy(widget = "https://embed.bitrefill.com", shopCards = emptyList()))) |
Collaborator
There was a problem hiding this comment.
Not necessary to change but used a more relevant domain
kcw-grunt
previously approved these changes
Jul 22, 2026
The unit-test job's Gradle daemon was being killed mid-build
("The message received from the daemon indicates that the daemon has
disappeared... it may have been killed or may have crashed"),
consistently right as testBrainwalletDebugUnitTest started after a
heavy multi-module build (KSP across 5 modules, detekt across 4,
dataBinding, bw-gdlib texture packing, 415 unit tests).
-Xmx4g for the Gradle daemon plus AGP-forked unit-test worker JVMs
plus OS/container overhead was exceeding the resource_class: large
executor's available RAM. Lowered the daemon heap to -Xmx3g and
org.gradle.workers.max/--max-workers from 2 to 1 to leave more
headroom for the forked test JVMs, without changing the resource
class (and its higher CircleCI billing tier).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
android-build-logic's DetektSetup.attachDetektTask() wires `detekt`
as a dependency of every compile*/assemble* task project-wide
(tasks.whenTaskAdded { if (name.startsWith("compile") || ...) dependsOn(detekt) }),
so a plain `./gradlew testBrainwalletDebugUnitTest` was also running
3-4 full detekt passes (autoCorrect=true, parallel=true, HTML/XML/TXT/
SARIF/Markdown reports across app/core/iap/general-purpose-app) that
have nothing to do with running tests.
That's real CPU/memory work competing with compilation and the forked
unit-test JVMs on the resource-constrained `large` executor, on top of
the daemon heap/worker tuning from the previous commit. Verified
locally: `-x detekt` removes all detekt tasks from the graph (confirmed
via --dry-run) with no other effect, and the full unit test run still
passes.
Scoped to this CI job's command line only, not the shared build-logic,
so local dev/lint workflows are unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
kcw-grunt
reviewed
Jul 22, 2026
| - run: | ||
| name: "Execute Unit Tests" | ||
| command: ./gradlew testBrainwalletDebugUnitTest --no-daemon --max-workers=1 | ||
| # -x detekt: android-build-logic's DetektSetup.attachDetektTask() wires |
Collaborator
There was a problem hiding this comment.
The build has inflated to many more modules and the detekt was causing extra linting causing the CI to fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📱 Description
This PR fixes an intermittent CI failure in
ShopBentoViewModelTest(TurbineAssertionError: Unconsumed events found) that only reproduced under the full test suite, never in isolation. Root cause was a missingMainDispatcherRule, so the test ran against whateverDispatchers.Mainwas left as by other test classes sharing the same JVM fork. Also includes the pending v4.10.5 version bump for this beta release.Platform
🎯 Type of Change
📋 Changes
New Components Added
None
Modifications
viewModelScope.launchblocks (each calling_state.updateseparately forsettingRepository.settingsandshopProxyRepository.shopProxy) into a singlecombine()-driven collector, sostateobservers can no longer see transient, incomplete intermediate states.@get:Rule val mainDispatcherRule = MainDispatcherRule(), matching the pattern already used inUnLockViewModelTest/BWSenderIntegrationTest/TxRepositoryImplIntegrationTest. ReplacedadvanceTimeBy(100)withadvanceUntilIdle()in the affected test.versionCode 202506342→202506343,versionName v4.10.4→v4.10.5.Removals
None
📊 Statistics
🧪 Tests Status
🎯 Reviewers
@kcw-grunt, @josikie