test(ci): repair CI breakages introduced by #461#471
Merged
Conversation
After #461 merged, CI showed 17 test failures that did not surface locally due to test-ordering / parallelisation differences. All 17 were direct consequences of the new wiring shipped in #461. Concrete fixes: - buy_sell_dtos_test: PaymentInfoError variant count 4 → 5 + add bitboxDisconnected to the asserted set. #461 added the variant but did not update the pinning test. - real_unit_sell_payment_info_service_validation_test: confirmPayment now calls appStore.ensureUnlocked() before reading credentials. Stub the call on _MockAppStore so mocktail returns Future<void> instead of null. - settings_seed_page_test: SettingsSeedPage builds a real SettingsSeedCubit via BlocProvider(create:), which calls ensureUnlocked() before reading the seed. Stub it on the shared MockAppStore. - settings_seed_cubit_test: replace the static wait: 10 ms in the two toggleShowSeed blocTests with an explicit stream wait so the _loadSeed() promotion settles before toggleShowSeed runs. The 10 ms budget races on slow CI runners. - create_wallet_page_test, restore_wallet_page_test: register MockDfxKycService in setUpAll. #461 added DfxKycService as a cubit dependency for ensureSignatureFor; without registration BlocProvider fails when the cubit is first read. - bitbox_credentials_test (queue continues after a sign throws): stub manager.devices to a non-empty list so _runOrThrowDisconnect's disconnect-probe returns false and the original Exception survives instead of being relabelled BitboxNotConnectedException.
Contributor
BitBox02 simulator checkHost: Firmware:
Summary: 15 total · 15 passed · 0 failed |
bitbox_flutter.dart re-exports BitboxManager so the explicit bitbox_manager.dart import is redundant. flutter analyze (CI version) flags this as unnecessary_import; locally a slightly older analyzer let it through.
…ender assert #461 changed SettingsSeedCubit from constructor-arg SoftwareWallet to async appStore + ensureUnlocked + _loadSeed. Initial state became SettingsSeedState('') with the seed only arriving after _loadSeed emitted. The first render of SettingsSeedView feeds that empty string into MnemonicReadOnlyField, whose constructor asserts seedWords.length == 12 — debug builds crash the screen on open, release builds would hit out-of-bounds in elementAt. Fix: seed the initial state from appStore.wallet when it's already a SoftwareWallet (the common case — software-wallet users land here from settings). _loadSeed still runs to cover the view-wallet case and to preserve the ensureUnlocked() invocation; the redundant emit for already-loaded software wallets is suppressed. Test impact: - initial-state test no longer needs stream.firstWhere — the seed is there synchronously; a Duration.zero microtask drain is enough to observe the ensureUnlocked() call. - toggleShowSeed blocTests no longer need to wait for the async load.
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
1 participant
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.
Summary
developCI is red after #461 merged:Run flutter test --coveragereports1342 tests passed, 17 failed. All 17 are direct consequences of the new wiring shipped by #461 — they did not surface locally because Flutter's test runner parallelises per file andGetIt's singleton state leaks across files differently than on CI.This PR repairs the test suite without touching any production code.
Failure groups & fixes
1.
PaymentInfoErrorvariant count drifted (1 test)test/packages/service/dfx/models/payment/buy_sell_dtos_test.dartpins the enum to "four documented variants". #461 addedbitboxDisconnected(now five) but did not update the pinning test.→ Change
hasLength(4)→hasLength(5), addbitboxDisconnectedto the asserted set, rename the test description.2.
MockAppStore.ensureUnlockednot stubbed (9 + 1 tests)#461 added
await appStore.ensureUnlocked()toRealUnitSellPaymentInfoService.confirmPaymentand theSettingsSeedCubit._loadSeed()path. Tests using_MockAppStore/MockAppStorewithout stubbingensureUnlockedgotnullback from mocktail and threwTypeError: type 'Null' is not a subtype of type 'Future<void>'.Affected:
test/packages/service/dfx/real_unit_sell_payment_info_service_validation_test.dart(9confirmPayment validation guardtests)test/screens/settings_seed/settings_seed_page_test.dart(1 page test)→ Add
when(() => appStore.ensureUnlocked()).thenAnswer((_) async {});to eachsetUp.3. Page tests don't register
DfxKycService(4 tests)#461 added
DfxKycServiceas a cubit dependency forensureSignatureFor. The existingcreate_wallet_page_testandrestore_wallet_page_testonly registerWalletServiceinsetUpAll, soBlocProvider(create:)fails withBad state: GetIt: Object/factory with type DfxKycService is not registeredwhen the cubit is first read.→ Add
MockDfxKycServiceand register it insetupDependencyInjectionof both files.4.
settings_seed_cubit_testrace on slow CI (1 test)toggleShowSeed flips showSeed and keeps seed unchangedrelies onwait: const Duration(milliseconds: 10)betweenactandverify. The async_loadSeed()chain (ensureUnlocked→ seed emit) takes longer than 10 ms on CI, so the toggle races with the post-_loadSeedemit andshowSeedends upfalseinstead oftrue.→ Replace the static
wait:with an explicitawait c.stream.firstWhere((s) => s.seed.isNotEmpty)insideactso the test only toggles after_loadSeedhas settled. Same fix on the sibling "toggleShowSeed twice" test.5.
bitbox_credentials_testqueue test relabels error (1 test)queue continues after a sign throws (slot released in finally)expects the originalException('first sign explodes')to surface. #461's new_runOrThrowDisconnectwrapper probesmanager.deviceson a thrown sign and relabels the error asBitboxNotConnectedExceptionif the device list is empty. The mock did not stubdevices, so it defaulted tonull→_deviceLost()returnedtrue→ the original exception got relabelled.→ Add a one-line stub
when(() => manager.devices).thenAnswer((_) async => [_FakeDevice()])so the disconnect probe returns "still connected" and the original exception survives.Verification
flutter test --coveragematches the CI's failure profile (1342 / 17) ondevelop.Why this needs to land fast
developCI is currently red, which blocks the auto-tag + release pipelines and propagates to any PR (e.g. #466) that runs against this branch. All seven changes are scoped totest/only.Test plan
flutter analyze— cleanflutter test --coverage— green (1359/0)