From 4a25d831fba9a49ad7cfd9c869847e9bbd58fdf3 Mon Sep 17 00:00:00 2001 From: Thomas SharedInbox Date: Fri, 15 May 2026 19:54:39 +0200 Subject: [PATCH] fix(sync-health): checkNow() now runs regardless of start() (#95) checkNow() previously delegated to _runAll(), which gated each account on the _running flag (only true after start() is called). This meant the manual "Verify sync health" action silently did nothing if start() had not yet been called, or in any context where the periodic runner was not active (e.g. widget tests). Fix: checkNow() now iterates accounts directly and calls _runForAccount() with force:true, bypassing the _running guard. The guard is still respected during periodic runs for graceful shutdown. Adds three unit tests that reproduce the bug and verify the fix. Co-Authored-By: Claude Sonnet 4.6 --- lib/core/sync/reliability_runner.dart | 13 +- .../reliability_runner_check_now_test.dart | 204 ++++++++++++++++++ 2 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 test/unit/reliability_runner_check_now_test.dart diff --git a/lib/core/sync/reliability_runner.dart b/lib/core/sync/reliability_runner.dart index b6f3bae..90d8014 100644 --- a/lib/core/sync/reliability_runner.dart +++ b/lib/core/sync/reliability_runner.dart @@ -50,7 +50,7 @@ class ReliabilityRunner { } } - Future _runForAccount(String accountId) async { + Future _runForAccount(String accountId, {bool force = false}) async { try { final mailboxes = await _mailboxes.observeMailboxes(accountId).first; var totalMissingLocally = 0; @@ -59,7 +59,7 @@ class ReliabilityRunner { final details = {}; for (final mailbox in mailboxes) { - if (!_running) break; + if (!force && !_running) break; final result = await _emails.verifySyncReliability( accountId, mailbox.path, @@ -103,7 +103,14 @@ class ReliabilityRunner { } /// Forces a reliability check for all accounts immediately. + /// + /// Works regardless of whether [start] has been called, so the UI can + /// trigger a manual check at any time without depending on the periodic + /// runner being active. Future checkNow() async { - await _runAll(); + final accounts = await _accounts.observeAccounts().first; + for (final account in accounts) { + await _runForAccount(account.id, force: true); + } } } diff --git a/test/unit/reliability_runner_check_now_test.dart b/test/unit/reliability_runner_check_now_test.dart new file mode 100644 index 0000000..8e7dc19 --- /dev/null +++ b/test/unit/reliability_runner_check_now_test.dart @@ -0,0 +1,204 @@ +// Tests for ReliabilityRunner.checkNow() — the manual "Verify sync health" +// trigger. Specifically guards against regression of issue #95 where +// checkNow() silently did nothing because it delegated to _runAll(), which +// checked the _running flag (only true after start() is called). + +import 'package:flutter_test/flutter_test.dart'; +import 'package:sharedinbox/core/models/account.dart'; +import 'package:sharedinbox/core/models/email.dart'; +import 'package:sharedinbox/core/models/mailbox.dart'; +import 'package:sharedinbox/core/repositories/account_repository.dart'; +import 'package:sharedinbox/core/repositories/email_repository.dart'; +import 'package:sharedinbox/core/repositories/mailbox_repository.dart'; +import 'package:sharedinbox/core/sync/reliability_runner.dart'; +import 'package:sharedinbox/data/db/database.dart' + hide Account, Email, EmailBody; + +import 'db_test_helper.dart'; + +// --------------------------------------------------------------------------- +// Minimal fakes +// --------------------------------------------------------------------------- + +const _kAccount = Account( + id: 'test-account', + displayName: 'Test', + email: 'test@example.com', + imapHost: 'localhost', +); + +const _kMailbox = Mailbox( + id: 'test-account:INBOX', + accountId: 'test-account', + path: 'INBOX', + name: 'INBOX', + unreadCount: 0, + totalCount: 0, +); + +class _FakeAccounts implements AccountRepository { + @override + Stream> observeAccounts() => Stream.value([_kAccount]); + @override + Future getAccount(String id) async => _kAccount; + @override + Future addAccount(Account account, String password) async {} + @override + Future updateAccount(Account account, {String? password}) async {} + @override + Future removeAccount(String id) async {} + @override + Future getPassword(String id) async => 'secret'; +} + +class _FakeMailboxes implements MailboxRepository { + @override + Stream> observeMailboxes(String? accountId) => + Stream.value([_kMailbox]); + @override + Future syncMailboxes(String accountId) async => 0; + @override + Future findMailboxByRole(String accountId, String role) async => + null; + @override + Future clearForResync(String accountId) async {} +} + +class _FakeEmails implements EmailRepository { + int verifyCallCount = 0; + + @override + Future verifySyncReliability( + String accountId, + String mailboxPath, + ) async { + verifyCallCount++; + return ReliabilityResult.healthy; + } + + // All remaining methods are unused by ReliabilityRunner. + @override + Stream> observeEmails(String a, String m, {int limit = 50}) => + Stream.value([]); + @override + Stream> observeThreads( + String a, + String m, { + int limit = 50, + }) => + Stream.value([]); + @override + Stream> observeEmailsInThread(String a, String m, String t) => + Stream.value([]); + @override + Future getEmail(String id) async => null; + @override + Future getEmailBody(String id) async => + const EmailBody(emailId: '', attachments: []); + @override + Future syncEmails(String a, String m) async => + SyncEmailsResult.zero; + @override + Future setFlag(String id, {bool? seen, bool? flagged}) async {} + @override + Future markAllAsRead(String a, String m) async {} + @override + Future moveEmail(String id, String dest) async {} + @override + Future deleteEmail(String id) async => null; + @override + Future sendEmail(String a, EmailDraft d) async {} + @override + Future downloadAttachment(String id, EmailAttachment att) async => ''; + @override + Future fetchRawRfc822(String id) async => ''; + @override + Future> searchEmails(String a, String m, String q) async => []; + @override + Future> searchEmailsGlobal(String? a, String q) async => []; + @override + Future> getEmailsByAddress(String? a, String addr) async => []; + @override + Future> searchAddresses( + String? a, + String q, { + int limit = 10, + }) async => + []; + @override + Stream> observeFailedMutations(String a) => + Stream.value([]); + @override + Future discardMutation(int id) async {} + @override + Future retryMutation(int id) async {} + @override + Future cancelPendingChange(String id, String type) async => false; + @override + Future snoozeEmail(String id, DateTime until) async {} + @override + Future restoreEmails(List emails) async {} + @override + Future findEmailByMessageId(String a, String messageId) async => null; + @override + Stream get onChangesQueued => const Stream.empty(); + @override + Stream watchJmapPush(String a, String password) => const Stream.empty(); + @override + Future flushPendingChanges(String a, String password) async => 0; + @override + Future wakeUpEmails(String accountId) async => 0; + @override + Future clearForResync(String accountId) async {} +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +void main() { + configureSqliteForTests(); + + group('ReliabilityRunner.checkNow()', () { + late AppDatabase db; + late _FakeEmails emails; + late ReliabilityRunner runner; + + setUp(() { + db = openTestDatabase(); + emails = _FakeEmails(); + runner = ReliabilityRunner(db, _FakeAccounts(), _FakeMailboxes(), emails); + }); + + tearDown(() => db.close()); + + test('writes sync-health row even when start() was never called', () async { + // Do NOT call runner.start() — this was the bug: checkNow() only ran + // when _running was true, which required start() to have been called. + await runner.checkNow(); + + final rows = await db.select(db.syncHealth).get(); + expect(rows, hasLength(1), reason: 'checkNow() must write to the DB'); + expect(rows.first.accountId, 'test-account'); + expect(rows.first.isHealthy, isTrue); + }); + + test('calls verifySyncReliability for each mailbox', () async { + await runner.checkNow(); + + expect( + emails.verifyCallCount, + 1, + reason: 'one mailbox → one verifySyncReliability call', + ); + }); + + test('also works when start() was called beforehand', () async { + runner.start(); + await runner.checkNow(); + + final rows = await db.select(db.syncHealth).get(); + expect(rows, hasLength(1)); + }); + }); +}