From 4ba5964a3bfb96121e51e9fdb6fffb82a8a103b7 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:26:55 +1100 Subject: [PATCH 1/2] test(mobile): keep late callbacks and fixed sleeps from failing tests Fixes three mobile flakes found in the 2026-10-21..28 CI runs. The planned AppDelegate/Runner.xcscheme test-host change is not included: it could not be verified locally (see "Test host" below). BuzzPushSnapshotEnrichmentTests (RunnerTests) - Runs: 40362671989 (job 121712747802, main @ 369ffa038), 39681273328 attempt 1 (job 119504560518), 40658588384 (job 122715950541, PR #8640). - Cause: the 1 s waits ran out while the test host's Flutter app held the main thread. The test then returned and deleted its temp directory. The bridge callback's assertions ran later, and XCTest blamed them on whichever test was running: :342 landed in ProfileSetupRecoveryTests and IdentityFilePluginTests, :381 in the Restriction test. - Change: callbacks only record the value and fulfill. Every assertion, including the isRestricted checks, now runs after the wait, so a late callback writes into a dead local. The seven waits cap at 10 s (callbackTimeout) instead of 1 s. A passing test still returns as soon as its callback runs. - Before/after: the full RunnerTests suite (107 tests) with Self.complete (PushSnapshotBridge.swift:233) wrapped in a delayed main-queue dispatch: - 1.5 s delay: before, 6 failures (5x "Exceeded timeout of 1 seconds" plus :342); after, 107/107. - 4 s delay: before, 6 failures (5 timeouts plus :342 blamed on IdentityFilePluginTests); after, 107/107. - 12 s delay, after: 8 failures, all in the owning tests (5x "Exceeded timeout of 10 seconds" plus :357, :403, :434) and none in later classes. - Bridge error code mutated: 1 failure at :403, so the moved assertions still catch a wrong code. - The :381 path (NewBridge's queued retry after its directory is gone) was not reproduced. Moving the assertion after the wait removes it by construction. ProfileSetupRecoveryTests testStartedEditorWithoutReadyRestoresProfileAndAllowsRetry - Run: 40585805357 (job 122477274956, PR #8568; its :180 is main's :158). - Cause: engine release and editor dismissal must happen within 2 s, on the same main thread the host app keeps busy. - Change: both predicate waits (engine == nil, presented controller dismissed) cap at 10 s. - Before/after, ProfileSetupRecoveryTests (4 tests) with the engine release held 4 s (NativeProfileSetup.swift:226): before, 2 failures, both "Exceeded timeout of 2 seconds ... Expect predicate"; after, 4/4. With the engine leaked, the test still fails (2x "Exceeded timeout of 10 seconds"). The dismissal cap was not falsified separately. Flutter avatar poster/autoplay tests (channel_detail_page_test.dart "profile sheet shows the poster before autoplay and restores it on tap"; settings_profile_header_test.dart "shows the poster until the animated avatar is ready") - Run: 40368315739 (job 121732711851, main @ b8b1ab5b3). - Cause: after releasing the image response, both tests slept a fixed 50 ms of real time before pumpAndSettle. A slower real decode left no frame scheduled, so the ready key was missing. - Change: a new WidgetHelpers.pumpUntilFound pumps in 10 ms real-time steps until the ready key appears, capped at about 5 s. Both tests use it. - Before/after: - Under load (32 `yes` processes, 4 parallel loops): channel test before 94/96, with 2 failures at :10127 showing the CI message; after 96/96. Settings test 64/64 before and after. - Image delivery delayed 200 ms with a Zone.root timer: before 0/5 for both tests, all with the CI message; after 5/5 for both. - Widget changed to never signal ready: after 0/2 for both, with the CI message at about 7 s, so a real regression still fails. Test host: AppDelegate / Runner.xcscheme change not landed - Planned: an AppDelegate configurationForConnecting override returning a Flutter-free scene configuration when a --buzz-unit-test-host launch argument is set, so the Dart VM would not start in the RunnerTests host. - Tried locally: the Dart VM still started. A simulator with a persisted scene session restores it without calling configurationForConnecting; on a fresh simulator the nil storyboard falls back to UIMainStoryboardFile (Main), which instantiates the Flutter view controller anyway; and a normal app launch afterwards came up black. Normal launches could not be shown unchanged, so this commit does not modify AppDelegate.swift, SceneDelegate.swift or Runner.xcscheme. - Follow-up: give the test-host configuration an explicit Flutter-free storyboard and handle restored sessions, or host RunnerTests in a separate app target. IdentityFilePluginTests testSyntheticDocumentsReadInPlaceAndRejectInvalidOrOversize: unchanged - Run: 40441255307 attempt 1 (job 121982118992). - Expected to be covered by the test-host change, which did not land, so it stays open. 10 full RunnerTests runs with and 10 without this change under 16 `yes` processes were all 107/107; CPU load alone does not reproduce it here. Its :342 failure in 39681273328 was the enrichment late callback fixed above. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- .../BuzzCommunicationNotificationTests.swift | 59 +++++++++++++------ .../ProfileSetupRecoveryTests.swift | 5 +- .../channels/channel_detail_page_test.dart | 13 ++-- .../profile/settings_profile_header_test.dart | 10 ++-- mobile/test/helpers/widget_helpers.dart | 13 ++++ 5 files changed, 65 insertions(+), 35 deletions(-) diff --git a/mobile/ios/RunnerTests/BuzzCommunicationNotificationTests.swift b/mobile/ios/RunnerTests/BuzzCommunicationNotificationTests.swift index 28e5a34bbe8..dc336bed414 100644 --- a/mobile/ios/RunnerTests/BuzzCommunicationNotificationTests.swift +++ b/mobile/ios/RunnerTests/BuzzCommunicationNotificationTests.swift @@ -242,7 +242,12 @@ final class BuzzCommunicationNotificationTests: XCTestCase { } } +/// Callbacks only record what they receive and assertions run after the wait, +/// so a callback that lands after its test returned cannot fail a later test. +/// The timeout is a cap; a passing test returns as soon as its callback runs. final class BuzzPushSnapshotEnrichmentTests: XCTestCase { + private let callbackTimeout: TimeInterval = 10 + func testAgeSignalRequestDoesNotDependOnNotificationStorage() { let delegate = MissingAppGroupDelegate() var completed = false @@ -268,28 +273,32 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { BuzzAgeSignalPayload.sharing(exclusiveUpperBound: 18, lowerBound: 13) } let minor = expectation(description: "native minor response") + var minorValue: Any? delegate.handleAgeSignalMethodCall( FlutterMethodCall(methodName: "requestAgeSignal", arguments: nil), viewController: UIViewController() ) { value in - XCTAssertEqual((value as? [String: Any])?["status"] as? String, "signal") - XCTAssertEqual((value as? [String: Any])?["ageUpper"] as? Int, 17) + minorValue = value minor.fulfill() } - await fulfillment(of: [minor], timeout: 1) + await fulfillment(of: [minor], timeout: callbackTimeout) + XCTAssertEqual((minorValue as? [String: Any])?["status"] as? String, "signal") + XCTAssertEqual((minorValue as? [String: Any])?["ageUpper"] as? Int, 17) delegate.requestPlatformAgeSignal = { _ in throw NSError(domain: "InjectedAgeFailure", code: 1) } let failed = expectation(description: "native error response") + var failedValue: Any? delegate.handleAgeSignalMethodCall( FlutterMethodCall(methodName: "requestAgeSignal", arguments: nil), viewController: UIViewController() ) { value in - XCTAssertEqual((value as? FlutterError)?.code, "age_signal_unavailable") + failedValue = value failed.fulfill() } - await fulfillment(of: [failed], timeout: 1) + await fulfillment(of: [failed], timeout: callbackTimeout) + XCTAssertEqual((failedValue as? FlutterError)?.code, "age_signal_unavailable") } func testAllowedNotificationRestoreNeedsNoAppGroupStore() { @@ -299,13 +308,15 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { keychainAccessGroup: nil ) let completed = expectation(description: "allowed without storage") + var received: Any? XCTAssertTrue(bridge.handle( FlutterMethodCall(methodName: "restoreAgeRestrictedNotifications", arguments: nil) ) { value in - XCTAssertNil(value) + received = value completed.fulfill() }) - wait(for: [completed], timeout: 1) + wait(for: [completed], timeout: callbackTimeout) + XCTAssertNil(received) } func testRestrictionCleanupFailurePreservesSnapshotAndRestoreReleasesAuthority() throws { @@ -335,25 +346,29 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { ) ) let restricted = expectation(description: "restriction despite cleanup failure") + var purgeResult: Any? XCTAssertTrue(bridge.handle( FlutterMethodCall(methodName: "purgeAgeRestrictedNotifications", arguments: nil) ) { value in - XCTAssertEqual((value as? FlutterError)?.code, "age_restriction_purge_failed") - XCTAssertTrue(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) + purgeResult = value restricted.fulfill() }) - wait(for: [restricted], timeout: 1) + wait(for: [restricted], timeout: callbackTimeout) + XCTAssertEqual((purgeResult as? FlutterError)?.code, "age_restriction_purge_failed") + XCTAssertTrue(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) XCTAssertEqual(try Data(contentsOf: snapshotURL), original) let restored = expectation(description: "allowed without restoring snapshot") + var restoreResult: Any? XCTAssertTrue(bridge.handle( FlutterMethodCall(methodName: "restoreAgeRestrictedNotifications", arguments: nil) ) { value in - XCTAssertNil(value) - XCTAssertFalse(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) + restoreResult = value restored.fulfill() }) - wait(for: [restored], timeout: 1) + wait(for: [restored], timeout: callbackTimeout) + XCTAssertNil(restoreResult) + XCTAssertFalse(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) XCTAssertEqual(deletionCalls, 2) XCTAssertEqual(try Data(contentsOf: snapshotURL), original) } @@ -365,9 +380,11 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { let failure = NSError(domain: "InjectedCleanupFailure", code: 1) let deletion = BuzzInteractionDeletionDeadline(timeout: 5, deleteAllInteractions: { $0(failure) }, scheduleTimeout: { _, _ in }) + var seedError: Error? BuzzInteractionCleanupRetry(containerURL: directory, deletion: deletion).request { - XCTAssertNotNil($0) + seedError = $0 } + XCTAssertNotNil(seedError) let bridge = BuzzPushSnapshotBridge( appGroupIdentifier: nil, endpointGrantStore: BuzzPushEndpointGrantKeychainStore(accessGroup: nil), @@ -375,14 +392,16 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { interactionDeletionDeadline: deletion ) let completed = expectation(description: "retry failure stays allowed") + var received: Any? XCTAssertTrue(bridge.handle( FlutterMethodCall(methodName: "restoreAgeRestrictedNotifications", arguments: nil) ) { value in - XCTAssertEqual((value as? FlutterError)?.code, "interaction_cleanup_retry_failed") - XCTAssertFalse(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) + received = value completed.fulfill() }) - wait(for: [completed], timeout: 1) + wait(for: [completed], timeout: callbackTimeout) + XCTAssertEqual((received as? FlutterError)?.code, "interaction_cleanup_retry_failed") + XCTAssertFalse(BuzzAgeRestrictionSession.isRestricted(containerURL: directory)) } func testStrictAgeGateWriteFailsWhenAppGroupStoreIsUnavailable() { @@ -392,6 +411,7 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { keychainAccessGroup: nil ) let completed = expectation(description: "strict write rejected") + var received: Any? XCTAssertTrue( bridge.handle( @@ -405,12 +425,13 @@ final class BuzzPushSnapshotEnrichmentTests: XCTestCase { ] ) ) { value in - XCTAssertEqual((value as? FlutterError)?.code, "snapshot_sync_unavailable") + received = value completed.fulfill() } ) - wait(for: [completed], timeout: 1) + wait(for: [completed], timeout: callbackTimeout) + XCTAssertEqual((received as? FlutterError)?.code, "snapshot_sync_unavailable") } func testMetadataAuthorityUsesNewestGrantForMatchingRelay() { diff --git a/mobile/ios/RunnerTests/ProfileSetupRecoveryTests.swift b/mobile/ios/RunnerTests/ProfileSetupRecoveryTests.swift index b7065ef2746..0f9a48a1217 100644 --- a/mobile/ios/RunnerTests/ProfileSetupRecoveryTests.swift +++ b/mobile/ios/RunnerTests/ProfileSetupRecoveryTests.swift @@ -163,8 +163,9 @@ final class ProfileSetupRecoveryTests: XCTestCase { XCTAssertFalse(avatar.isHidden) } // UIKit releases popped controllers at the end of its autorelease turn. + // The cap absorbs a busy main thread; a leaked engine never becomes nil. let released = XCTNSPredicateExpectation(predicate: NSPredicate { _, _ in engine == nil }, object: nil) - await fulfillment(of: [released], timeout: 2) + await fulfillment(of: [released], timeout: 10) } XCTAssertEqual(watchdogs.count, 2) skip.sendActions(for: .touchUpInside) @@ -178,7 +179,7 @@ final class ProfileSetupRecoveryTests: XCTestCase { XCTAssertTrue(notNow.isEnabled) notNow.sendActions(for: .touchUpInside) let dismissed = XCTNSPredicateExpectation(predicate: NSPredicate { _, _ in parent.presentedViewController == nil }, object: nil) - await fulfillment(of: [dismissed], timeout: 2) + await fulfillment(of: [dismissed], timeout: 10) XCTAssertEqual(messenger.completionAction, "skip") withExtendedLifetime(coordinator) {} nav.dismiss(animated: false) diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 55f02451a2a..0ae2f7fb11b 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -90,6 +90,7 @@ import 'package:buzz/shared/widgets/skeleton.dart'; import 'package:shared_preferences/shared_preferences.dart'; import '../../helpers/thread_ids.dart'; +import '../../helpers/widget_helpers.dart'; part 'thread_reply_refresh_cases.dart'; part 'thread_title_capsule_cases.dart'; @@ -10119,17 +10120,13 @@ void main() { ); animationResponse.complete(http.Response.bytes(_transparentPng, 200)); - await tester.runAsync( - () => Future.delayed(const Duration(milliseconds: 50)), + final animationReady = find.byKey( + const ValueKey('progressive-animated-avatar-animation-ready'), ); + await WidgetHelpers.pumpUntilFound(tester, animationReady); await tester.pumpAndSettle(); - expect( - find.byKey( - const ValueKey('progressive-animated-avatar-animation-ready'), - ), - findsOneWidget, - ); + expect(animationReady, findsOneWidget); expect( find.byKey(const ValueKey('progressive-animated-avatar-poster')), findsNothing, diff --git a/mobile/test/features/profile/settings_profile_header_test.dart b/mobile/test/features/profile/settings_profile_header_test.dart index 79ec509e721..5ee803206c7 100644 --- a/mobile/test/features/profile/settings_profile_header_test.dart +++ b/mobile/test/features/profile/settings_profile_header_test.dart @@ -128,15 +128,13 @@ void main() { containsAll([posterUrl, animationUrl]), ); animationResponse.complete(http.Response.bytes(_transparentPng, 200)); - await tester.runAsync( - () => Future.delayed(const Duration(milliseconds: 50)), + final animationReady = find.byKey( + const ValueKey('progressive-animated-avatar-animation-ready'), ); + await WidgetHelpers.pumpUntilFound(tester, animationReady); await tester.pumpAndSettle(); - expect( - find.byKey(const ValueKey('progressive-animated-avatar-animation-ready')), - findsOneWidget, - ); + expect(animationReady, findsOneWidget); expect( find.byKey(const ValueKey('progressive-animated-avatar-poster')), findsNothing, diff --git a/mobile/test/helpers/widget_helpers.dart b/mobile/test/helpers/widget_helpers.dart index 11b4b421410..1f15c544176 100644 --- a/mobile/test/helpers/widget_helpers.dart +++ b/mobile/test/helpers/widget_helpers.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:hooks_riverpod/misc.dart'; import 'package:buzz/shared/theme/theme.dart'; @@ -24,4 +25,16 @@ class WidgetHelpers { ), ); } + + /// Pumps until [finder] matches, letting real async work such as an image + /// decode run between frames. Gives up after about 5 s so the caller's + /// `expect` still reports a real regression. + static Future pumpUntilFound(WidgetTester tester, Finder finder) async { + for (var i = 0; i < 500 && finder.evaluate().isEmpty; i++) { + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 10)), + ); + await tester.pump(); + } + } } From 429b312f4f1972e0617443fae47cfae959910e5b Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 20:44:43 +1100 Subject: [PATCH 2/2] ci(flake-check): key Flutter results by suite file, not helper URL Every `testWidgets` test reports `package:flutter_test/...` as its `url`, so `fileURLToPath` threw, the report parsed as empty, and every Flutter flake-check iteration failed even when all tests passed. Prefer the suite's `root_url`, which the JSON reporter sets whenever `url` points outside the suite file. Co-authored-by: Jitter <2b2e6415e748c35180847a37aadae06a11e30dddc76b784e7fd9354c4eb42e7a@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- scripts/flake-check.mjs | 6 +++++- scripts/flake-check.test.mjs | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index 4c8693ea6c8..13a0bbd53b8 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -1115,6 +1115,9 @@ function repeatNode(files, iterations) { // Per-test statuses of a `dart test` JSON report (one event per line), keyed // ` › `. Hidden entries are the runner's own file-loading steps. +// A test declared through a helper (every `testWidgets` is) has the helper's +// `package:` URL as its `url`, so its file comes from its suite, else from +// `root_url`. export function parseDartReport(text, mobileRoot = `${root}mobile/`) { const started = new Map(); const suites = new Map(); @@ -1131,8 +1134,9 @@ export function parseDartReport(text, mobileRoot = `${root}mobile/`) { if (event.type !== "testDone" || event.hidden) continue; const test = started.get(event.testID); if (!test) continue; + const url = test.root_url ?? test.url; const source = suites.get(test.suiteID) ?? - (test.url?.startsWith("file:") ? fileURLToPath(test.url) : test.url ?? "unknown"); + (url?.startsWith("file:") ? fileURLToPath(url) : url ?? "unknown"); const file = source.replace(mobileRoot, ""); statuses.set( `${file} › ${test.name}`, diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index 5688a914781..304668d77a3 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -953,6 +953,15 @@ test("reports give per-test statuses per iteration", () => { type: "testStart", test: { id: 4, name: "later", url: "file:///m/test/b_test.dart" }, }), + event({ + type: "testStart", + test: { + id: 5, + name: "widget renders", + url: "package:flutter_test/src/widget_tester.dart", + root_url: "file:///m/test/b_test.dart", + }, + }), "not json", event({ type: "testDone", @@ -975,6 +984,13 @@ test("reports give per-test statuses per iteration", () => { hidden: false, skipped: true, }), + event({ + type: "testDone", + testID: 5, + result: "success", + hidden: false, + skipped: false, + }), event({ type: "done", success: false }), ].join("\n"), "/m/", @@ -983,6 +999,7 @@ test("reports give per-test statuses per iteration", () => { ["test/a_test.dart › group renders", "passed"], ["test/a_test.dart › group fails", "failed"], ["test/b_test.dart › later", "skipped"], + ["test/b_test.dart › widget renders", "passed"], ]), ); // Playwright lists a --repeat-each journey once per repetition, in order.