Repository navigation
test(router): stop the hashtag route test from hanging cache recovery tests in CI - #9050
Merged
Merged
Conversation
The /hashtag/rust test in app_shell_integration_test mounts
HashtagFeedScreen against the real hashtagServiceProvider, which builds
HashtagCacheService and calls Hive.openBox('hashtag_stats'). Inside
testWidgets' fake async that open never completes, so it stays pending
in Hive's private opening registry for the rest of the isolate. Any
later suite in the merged CI bundle that opens hashtag_stats then waits
on it until the 10-minute timeout: the cache_recovery_service_test hang
in #9022.
Override hashtagServiceProvider with a mock stubbed for the two calls
the screen makes on open, as the hashtag_feed_* screen tests already
do. The unmount() comment named HashtagService's 60s timer as its
example; with the service mocked no provider timer is left pending in
this file, so the example goes.
realmeylisdev
requested review from
mbradley and
rabble
and removed request for
a team
September 10, 2026 21:13
Mobile PR PreviewPreview refreshed for Last refresh:
|
5 tasks
NotThatKindOfDrLiz
approved these changes
Sep 10, 2026
NotThatKindOfDrLiz
left a comment
Member
There was a problem hiding this comment.
The point fix is correct and fully verified on 04995d6f125de84916589a7694f210e001fa9a3f. #9053 now tracks the pending-openBox harness gap left outside this PR.
SSMG64
pushed a commit
to IBackupStuff/divine-mobile
that referenced
this pull request
Oct 22, 2026
…ivinevideo#9581) * refactor(services): delete the dead HashtagCacheService Hive cache getPopularHashtags() was the only method reading HashtagCacheService, and it has had zero production callers since it was written. The provider never wires ref.onDispose(service.dispose), so autoDispose drops the object without closing its Hive box, verified live via the Dart VM Service: the box opens on-demand, stays empty forever (nothing ever wrote to it), and remains open long after the last screen that touched it unmounts. divinevideo#9050 already had to mock hashtagServiceProvider in a widget test to work around this exact box hanging fake-async suites (divinevideo#9022); deleting the dead code removes the hazard at its root instead of leaving the next test to rediscover it. Also renames the app-layer HashtagStats model to HashtagVideoStats, resolving its name collision with the still-inert Drift HashtagStats table (tracked separately as a follow-up: the table has no production writer either, but dropping it needs a real schema migration). Closes divinevideo#6988 * fix(cache): clean up retired hashtag cache references * fix(cache): delete stranded hashtag_stats files at startup * refactor(services): give the legacy hashtag_stats box name one owner CacheRecoveryService and HiveStorageService each independently declared their own 'hashtag_stats' string literal to clean up the leftover Hive box file from before HashtagCacheService was deleted. Give it a single shared owner (HiveBoxNames.legacyHashtagStats, deliberately outside `all` since nothing opens the box anymore) so the two can't drift. Addresses rabble's review nit on divinevideo#9581. --------- Co-authored-by: Liz Sweigart <127434495+NotThatKindOfDrLiz@users.noreply.github.com> Co-authored-by: Daniel Cadenas <dcadenas@gmail.com>
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
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.
Problem
Tests (shard 2/4)keeps timing out on pull requests that have nothing to do with caching. Two tests intest/services/cache_recovery_service_test.dart— "preserves pending uploads while clearing disposable Hive caches" and "preserves durable notification preferences while clearing caches" — each wait out the full 10-minute test timeout, so the shard loses 20 minutes and needs a re-run. #9022 records it on #8540, #8994 and #9018 within four hours, each under a different shuffle seed.The hang is caused by a different test. CI runs every untagged test file in one merged process, and Hive tracks boxes by name for the whole process. The test
navigating to /hashtag/rust leaves the shell entirelyintest/router/app_shell_integration_test.dart(re-enabled by #8950) mounts the realHashtagFeedScreen. That screen's provider chain buildsHashtagCacheService, whoseinitialize()callsHive.openBox('hashtag_stats'). Inside the fake-async zonetestWidgetsruns in, that open never completes. Hive then keepshashtag_statsin its private registry of in-flight opens for the rest of the process.Hive.isBoxOpencannot see that registry, so neither the victim's own cleanup nor the root Hive harness influtter_test_config.dartcan clear it. Any later test that openshashtag_statswaits on the stranded open until it times out, and both hanging tests openhashtag_stats. Whether the victim runs after the leaker depends on the shuffle seed, which is why only some seeds hang.Fix
app_shell_integration_test.dartnow overrideshashtagServiceProviderwith a mock stubbed for the two calls the screen makes when it opens (getVideosByHashtagsandsubscribeToHashtagVideos). The fivehashtag_feed_*screen tests already use this pattern. The test still asserts that the route lands onHashtagFeedScreenoutside the shell; it just no longer touches Hive.The
unmount()doc comment cited HashtagService's 60-second periodic timer as the reason the container has to be disposed inside the test body. With the service mocked, no provider timer is left pending in this file, so the comment keeps the general rule and drops that example.What this leaves alone
HashtagCacheServiceopens its box normally in the app; the hang exists only because a widget test started real Hive I/O inside fake async.openBox. This PR removes the test behind the current occurrences. The harness gap that lets a future leaker hide the same way stays open.hashtag_feed_*screen tests already override the provider. The router tests that navigate to a hashtag path never mount a widget tree, so none of them reach Hive.Verification
A two-file bundle reproduces the merged-process order locally. It runs the leaker first and then the victim in one process, calls
Hive.initinsetUpAllthe way an earlier suite does in CI, passes--dart-define=DIVINE_STRICT_CHANNELS=trueas CI does, and uses--timeout=120sso the hang stops after two minutes instead of ten:TimeoutException after 0:02:00(4m04s)/hashtag/rusttest, plus the victim fileThe two narrowed runs pin the leak to that single test.
On the rebased head:
app_shell_integration_test.dart(8/8) andcache_recovery_service_test.dart(11/11) pass on their own.type 'Null' is not a subtypelines, which come from unrelated mocks, are the same set with the same counts before and after. The new mock swallows no errors of its own.c.dispose()removed passed 8/8, which confirms no provider timer is left pending in this file.flutter analyze lib test integration_testanddart formatare clean.check_shared_setup_stubs,check_placeholder_tests,check_unpinned_unchanged_assertions,check_ungrouped_tests,check_skip_ceiling,check_l10n_delegates_ceiling,check_nostr_id_log_truncation,check_future_delayed_ceiling,check_process_global_mutations,check_shared_channel_overridesandcheck_http_overrides_isolation.Closes #9022
Follow-up: #9053 tracks the pending-
openBoxharness gap left outside this point fix.