From 82a0dc1b336a051b9cf1f620e278aa5f23f952a6 Mon Sep 17 00:00:00 2001 From: "Javier G. Montoya S." Date: Thu, 21 May 2026 00:48:16 -0400 Subject: [PATCH] test(integration): add messaging-interaction test with shared harness, single-build bundle and simulator auto-detection (#684) --- .github/PULL_REQUEST_TEMPLATE.md | 1 + integration_test/_support/app_flows.dart | 263 +++++++++++ integration_test/_support/app_setup.dart | 100 +++++ integration_test/_support/harness.dart | 5 + integration_test/_support/tester_helpers.dart | 56 +++ integration_test/all_tests.dart | 8 + .../basic_messaging_flow_test.dart | 420 +----------------- .../messaging_interactions_test.dart | 264 +++++++++++ justfile | 60 +-- lib/src/rust/api/error.freezed.dart | 30 +- lib/src/rust/api/markdown.freezed.dart | 48 +- lib/widgets/chat_list_tile.dart | 37 +- lib/widgets/chat_message_bubble.dart | 1 + lib/widgets/wn_chat_list_item.dart | 1 + pubspec.yaml | 6 +- .../PR-517-start-chat-flow-improvements.md | 201 --------- 16 files changed, 820 insertions(+), 681 deletions(-) create mode 100644 integration_test/_support/app_flows.dart create mode 100644 integration_test/_support/app_setup.dart create mode 100644 integration_test/_support/harness.dart create mode 100644 integration_test/_support/tester_helpers.dart create mode 100644 integration_test/all_tests.dart create mode 100644 integration_test/messaging_interactions_test.dart delete mode 100644 reviews/PR-517-start-chat-flow-improvements.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 8568a36..21fdcae 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -41,3 +41,4 @@ Describe what changed and why. Link the related issue: `Closes #NNN` - [ ] Related issue linked (`Closes #NNN`) - [ ] `CHANGELOG.md` updated (if user-visible change) - [ ] Screenshots added (for UI changes) +- [ ] Integration tests pass (`just int-test` with an open simulator and local Nostr relays running โ€” `docker compose up -d`) diff --git a/integration_test/_support/app_flows.dart b/integration_test/_support/app_flows.dart new file mode 100644 index 0000000..76a5738 --- /dev/null +++ b/integration_test/_support/app_flows.dart @@ -0,0 +1,263 @@ +// Whitenoise user-journey helpers: identity, groups, messaging, navigation. +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:whitenoise/providers/auth_provider.dart'; +import 'package:whitenoise/providers/message_debug_log_provider.dart'; +import 'package:whitenoise/widgets/wn_message_bubble.dart'; + +import 'tester_helpers.dart'; + +Future createIdentity( + WidgetTester tester, + ProviderContainer container, + String displayName, +) async { + await tapKey(tester, const Key('auth_signup_button')); + await enterTextInWidget( + tester, + const Key('signup_display_name_field'), + displayName, + ); + await tapKey(tester, const Key('signup_create_profile_button')); + await pumpUntilFound( + tester, + find.byKey(const Key('chat_add_button')), + timeout: const Duration(seconds: 90), + ); + final pubkey = container.read(authProvider).value; + expect(pubkey, isNotNull); + return pubkey!; +} + +Future createAdditionalIdentity( + WidgetTester tester, + ProviderContainer container, + String displayName, +) async { + await returnToChatList(tester); + await openSettings(tester); + await tapKey(tester, const Key('settings_switch_profile_button')); + await tapKey(tester, const Key('connect_another_profile_button')); + return createIdentity(tester, container, displayName); +} + +Future copyPublicKey(WidgetTester tester) async { + await returnToChatList(tester); + await openSettings(tester); + await tapKey(tester, const Key('settings_profile_keys_menu_item')); + final publicKeyField = find.byKey(const Key('profile_keys_public_key_field')); + await pumpUntilFound(tester, publicKeyField); + await tester.tap( + find.descendant( + of: publicKeyField, + matching: find.byKey(const Key('copy_button')), + ), + ); + await tester.pump(const Duration(milliseconds: 200)); + final clipboardData = await Clipboard.getData('text/plain'); + final npub = clipboardData?.text; + expect(npub, isNotNull); + expect(npub, startsWith('npub1')); + await returnToChatList(tester); + return npub!; +} + +Future switchProfile(WidgetTester tester, String pubkey) async { + await returnToChatList(tester); + await openSettings(tester); + await tapKey(tester, const Key('settings_switch_profile_button')); + await tapKey( + tester, + Key('profile_switcher_item_$pubkey'), + ); + await returnToChatList(tester); +} + +Future startGroupChat( + WidgetTester tester, { + required String groupName, + required String inviteeNpub, + required String inviteePubkey, +}) async { + await tapKey(tester, const Key('chat_add_button')); + await tapKey(tester, const Key('create_group_menu_item')); + await enterTextInWidget( + tester, + const Key('user_picker_search_field'), + inviteeNpub, + ); + await tapKey( + tester, + Key('user_picker_user_$inviteePubkey'), + timeout: const Duration(seconds: 60), + ); + await pumpUntilFound( + tester, + find.byKey(Key('user_picker_bubble_$inviteePubkey')), + ); + await tapKey(tester, const Key('user_picker_submit_button')); + await enterTextInWidget( + tester, + const Key('set_up_group_name_field'), + groupName, + ); + await pumpUntilFound( + tester, + find.byKey(Key('member_$inviteePubkey')), + timeout: const Duration(seconds: 60), + ); + await tapKey(tester, const Key('set_up_group_create_button')); + await waitForChatReady(tester, timeout: const Duration(seconds: 90)); +} + +Future sendMessage( + WidgetTester tester, + ProviderContainer container, + String message, +) async { + final input = find.descendant( + of: find.byKey(const Key('chat_message_input')), + matching: find.byType(TextField), + ); + await pumpUntilFound(tester, input); + await tester.enterText(input, message); + await tester.pump(const Duration(milliseconds: 200)); + await tester.tap( + find.descendant( + of: find.byKey(const Key('chat_message_input')), + matching: find.byKey(const Key('send_button')), + ), + ); + try { + await expectMessageVisible(tester, message); + } catch (_) { + fail( + 'Timed out waiting for sent message "$message".\n' + '${messageDebugSummary(container)}', + ); + } +} + +String messageDebugSummary(ProviderContainer container) { + final state = container.read(messageDebugLogProvider); + final sendLines = state.sendLog + .take(8) + .map((entry) { + final details = [ + entry.status.name, + 'group=${entry.groupId}', + if (entry.contentLen != null) 'len=${entry.contentLen}', + if (entry.resultId != null) 'result=${entry.resultId}', + if (entry.error != null) 'error=${entry.error}', + ]; + return 'send: ${details.join(' ')}'; + }) + .join('\n'); + final streamLines = state.streamLog + .take(12) + .map((entry) { + final details = [ + entry.eventType.name, + 'group=${entry.groupId}', + if (entry.messageCount != null) 'count=${entry.messageCount}', + if (entry.trigger != null) 'trigger=${entry.trigger}', + if (entry.messageId != null) 'message=${entry.messageId}', + if (entry.error != null) 'error=${entry.error}', + ]; + return 'stream: ${details.join(' ')}'; + }) + .join('\n'); + return [ + 'Message debug log:', + if (sendLines.isEmpty) 'send: ' else sendLines, + if (streamLines.isEmpty) 'stream: ' else streamLines, + ].join('\n'); +} + +Future expectMessageVisible(WidgetTester tester, String message) { + return pumpUntilFound( + tester, + find.descendant( + of: find.byType(WnMessageBubble), + matching: find.textContaining(message, findRichText: true), + ), + timeout: const Duration(seconds: 90), + ); +} + +Future waitForChatReady( + WidgetTester tester, { + Duration timeout = const Duration(seconds: 30), +}) async { + await pumpUntilFound( + tester, + find.byKey(const Key('chat_message_input')), + timeout: timeout, + ); + await pumpUntilNotFound( + tester, + find.byType(CircularProgressIndicator), + timeout: timeout, + ); +} + +Future openInvite(WidgetTester tester, String groupName) async { + await pumpUntilFound( + tester, + find.text(groupName), + timeout: const Duration(seconds: 90), + ); + await tester.tap(find.text(groupName).first); + await pumpUntilFound( + tester, + find.byKey(const Key('chat_invite_accept_button')), + timeout: const Duration(seconds: 60), + ); +} + +Future openChat(WidgetTester tester, String groupName) async { + await pumpUntilFound( + tester, + find.text(groupName), + timeout: const Duration(seconds: 60), + ); + await tester.tap(find.text(groupName).first); + await waitForChatReady(tester, timeout: const Duration(seconds: 60)); +} + +Future openSettings(WidgetTester tester) async { + await tapKey(tester, const Key('avatar_button')); + await pumpUntilFound( + tester, + find.byKey(const Key('settings_switch_profile_button')), + ); +} + +Future returnToChatList( + WidgetTester tester, { + Duration timeout = const Duration(seconds: 30), +}) async { + final deadline = DateTime.now().add(timeout); + while (DateTime.now().isBefore(deadline)) { + await tester.pump(const Duration(milliseconds: 200)); + if (find.byKey(const Key('chat_add_button')).evaluate().isNotEmpty) { + return; + } + + final chatBackButton = find.byKey(const Key('back_button')); + if (chatBackButton.evaluate().isNotEmpty) { + await tester.tap(chatBackButton.first); + continue; + } + + final slateBackButton = find.byKey(const Key('slate_back_button')); + if (slateBackButton.evaluate().isNotEmpty) { + await tester.tap(slateBackButton.first); + continue; + } + } + + fail('Timed out returning to the chat list'); +} diff --git a/integration_test/_support/app_setup.dart b/integration_test/_support/app_setup.dart new file mode 100644 index 0000000..81e6109 --- /dev/null +++ b/integration_test/_support/app_setup.dart @@ -0,0 +1,100 @@ +// Per-test backend lifecycle: FFI/Whitenoise init, reset, app mount, relay checks. +import 'dart:io'; + +import 'package:connectivity_plus/connectivity_plus.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:whitenoise/main.dart' show WnApp; +import 'package:whitenoise/providers/auth_provider.dart'; +import 'package:whitenoise/providers/notification_provider.dart'; +import 'package:whitenoise/providers/offline_provider.dart'; +import 'package:whitenoise/providers/push_registration_provider.dart'; +import 'package:whitenoise/src/rust/api.dart' as rust_api; +import 'package:whitenoise/src/rust/frb_generated.dart'; + +import '../../test/mocks/mock_secure_storage.dart'; +import 'tester_helpers.dart'; + +const _relayUrls = ['ws://localhost:8080', 'ws://localhost:7777']; + +bool _rustBridgeInitialized = false; +Directory? _backendRoot; + +/// Resets the backend so each test starts from a clean, logged-out state. The +/// bundled `all_tests.dart` entrypoint runs every test in one process: +/// `RustLib` (the FFI bridge) initialises once, while `deleteAllData` clears +/// the process-global Whitenoise instance โ€” which must then be re-installed +/// with a fresh `initializeWhitenoise`, per that API's documented contract. +Future _resetBackend() async { + if (_rustBridgeInitialized) { + await rust_api.deleteAllData(); + } else { + await RustLib.init(); + _rustBridgeInitialized = true; + } + + final root = _backendRoot ??= await Directory.systemTemp.createTemp('whitenoise_integration_'); + final dataDir = Directory('${root.path}/data'); + final logsDir = Directory('${root.path}/logs'); + await dataDir.create(recursive: true); + await logsDir.create(recursive: true); + + final config = await rust_api.createWhitenoiseConfig( + dataDir: dataDir.path, + logsDir: logsDir.path, + defaultRelayUrls: _relayUrls, + ); + await rust_api.initializeWhitenoise(config: config); +} + +/// Mounts a fresh app for one test, after resetting the backend so the test +/// starts logged-out with an empty database regardless of run order. +Future mountApp(WidgetTester tester) async { + await _resetBackend(); + + final container = ProviderContainer( + overrides: [ + secureStorageProvider.overrideWithValue(MockSecureStorage()), + checkConnectivityFunctionProvider.overrideWithValue( + () async => [ConnectivityResult.wifi], + ), + connectivityStreamProvider.overrideWithValue(const Stream.empty()), + reachAnyRelayHostFunctionProvider.overrideWithValue((_) async => true), + // No-op the notification and push controllers so the run never + // triggers the iOS notification-permission prompt. + notificationListenerProvider.overrideWith((_) {}), + pushRegistrationControllerProvider.overrideWith((_) {}), + ], + ); + addTearDown(container.dispose); + await container.read(authProvider.future); + + await tester.pumpWidget( + UncontrolledProviderScope(container: container, child: const WnApp()), + ); + await pumpUntilFound(tester, find.byKey(const Key('auth_signup_button'))); + return container; +} + +Future expectLocalRelaysAvailable() async { + await _expectLocalRelayAvailable(8080); + await _expectLocalRelayAvailable(7777); +} + +Future _expectLocalRelayAvailable(int port) async { + try { + final socket = await Socket.connect( + '127.0.0.1', + port, + timeout: const Duration(seconds: 1), + ); + socket.destroy(); + } catch (error) { + fail( + 'Expected a local Nostr relay on 127.0.0.1:$port before running this integration test. ' + 'Run `docker compose up -d`, then run the test again. ' + 'Connection error: $error', + ); + } +} diff --git a/integration_test/_support/harness.dart b/integration_test/_support/harness.dart new file mode 100644 index 0000000..5063ea9 --- /dev/null +++ b/integration_test/_support/harness.dart @@ -0,0 +1,5 @@ +// Barrel for the integration-test support library. Test files import this one +// file; the implementation is split by concern across the files below. +export 'app_flows.dart'; +export 'app_setup.dart'; +export 'tester_helpers.dart'; diff --git a/integration_test/_support/tester_helpers.dart b/integration_test/_support/tester_helpers.dart new file mode 100644 index 0000000..f6503e7 --- /dev/null +++ b/integration_test/_support/tester_helpers.dart @@ -0,0 +1,56 @@ +// Generic Flutter widget-test primitives โ€” no Whitenoise knowledge. +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +Future enterTextInWidget( + WidgetTester tester, + Key key, + String text, +) async { + final field = find.descendant( + of: find.byKey(key), + matching: find.byType(TextField), + ); + await pumpUntilFound(tester, field); + await tester.enterText(field, text); + await tester.pump(const Duration(milliseconds: 200)); +} + +Future tapKey( + WidgetTester tester, + Key key, { + Duration timeout = const Duration(seconds: 30), +}) async { + final finder = find.byKey(key); + await pumpUntilFound(tester, finder, timeout: timeout); + await tester.ensureVisible(finder); + await tester.pump(const Duration(milliseconds: 300)); + await tester.tap(finder); + await tester.pump(const Duration(milliseconds: 200)); +} + +Future pumpUntilFound( + WidgetTester tester, + Finder finder, { + Duration timeout = const Duration(seconds: 30), +}) async { + final deadline = DateTime.now().add(timeout); + while (DateTime.now().isBefore(deadline)) { + await tester.pump(const Duration(milliseconds: 250)); + if (finder.evaluate().isNotEmpty) return; + } + fail('Timed out waiting for $finder'); +} + +Future pumpUntilNotFound( + WidgetTester tester, + Finder finder, { + Duration timeout = const Duration(seconds: 30), +}) async { + final deadline = DateTime.now().add(timeout); + while (DateTime.now().isBefore(deadline)) { + await tester.pump(const Duration(milliseconds: 250)); + if (finder.evaluate().isEmpty) return; + } + fail('Timed out waiting for $finder to disappear'); +} diff --git a/integration_test/all_tests.dart b/integration_test/all_tests.dart new file mode 100644 index 0000000..e7be8ff --- /dev/null +++ b/integration_test/all_tests.dart @@ -0,0 +1,8 @@ +// Aggregated entrypoint so the iOS suite builds once, not once per file. +import 'basic_messaging_flow_test.dart' as basic_messaging_flow; +import 'messaging_interactions_test.dart' as messaging_interactions; + +void main() { + basic_messaging_flow.main(); + messaging_interactions.main(); +} diff --git a/integration_test/basic_messaging_flow_test.dart b/integration_test/basic_messaging_flow_test.dart index 50e27a0..668989d 100644 --- a/integration_test/basic_messaging_flow_test.dart +++ b/integration_test/basic_messaging_flow_test.dart @@ -1,28 +1,16 @@ -import 'dart:io'; - -import 'package:connectivity_plus/connectivity_plus.dart'; import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:integration_test/integration_test.dart'; -import 'package:whitenoise/main.dart' show WnApp; import 'package:whitenoise/providers/auth_provider.dart'; -import 'package:whitenoise/providers/message_debug_log_provider.dart'; -import 'package:whitenoise/providers/offline_provider.dart'; -import 'package:whitenoise/src/rust/api.dart' as rust_api; -import 'package:whitenoise/src/rust/frb_generated.dart'; import 'package:whitenoise/utils/encoding.dart'; -import 'package:whitenoise/widgets/wn_message_bubble.dart'; -import '../test/mocks/mock_secure_storage.dart'; +import '_support/harness.dart'; const _firstDisplayName = 'Integration Alice'; const _secondDisplayName = 'Integration Bob'; const _groupName = 'Integration Test Group'; const _initialMessage = 'Hello, testing initial message'; const _secondMessage = 'Hello, testing second message'; -const _relayUrls = ['ws://localhost:8080', 'ws://localhost:7777']; void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); @@ -30,409 +18,49 @@ void main() { testWidgets('two identities exchange messages through a group invite', ( tester, ) async { - await _expectLocalRelaysAvailable(); - final container = await _mountApp(tester); + await expectLocalRelaysAvailable(); + final container = await mountApp(tester); - final firstPubkey = await _createIdentity( + final firstPubkey = await createIdentity( tester, container, _firstDisplayName, ); - await _createAdditionalIdentity(tester, container, _secondDisplayName); + await createAdditionalIdentity(tester, container, _secondDisplayName); final activeSecondPubkey = container.read(authProvider).value; expect(activeSecondPubkey, isNotNull); final secondPubkey = activeSecondPubkey!; - await _switchProfile(tester, firstPubkey); - final firstNpub = await _copyPublicKey(tester); + await switchProfile(tester, firstPubkey); + final firstNpub = await copyPublicKey(tester); expect(hexFromNpub(firstNpub), firstPubkey); - await _switchProfile(tester, secondPubkey); - await _startGroupChat( + await switchProfile(tester, secondPubkey); + await startGroupChat( tester, + groupName: _groupName, inviteeNpub: firstNpub, inviteePubkey: firstPubkey, ); - await _sendMessage(tester, container, _initialMessage); - await _expectMessageVisible(tester, _initialMessage); + await sendMessage(tester, container, _initialMessage); + await expectMessageVisible(tester, _initialMessage); - await _returnToChatList(tester); - await _switchProfile(tester, firstPubkey); - await _openInvite(tester); - await _expectMessageVisible(tester, _initialMessage); - await _tapKey( + await returnToChatList(tester); + await switchProfile(tester, firstPubkey); + await openInvite(tester, _groupName); + await expectMessageVisible(tester, _initialMessage); + await tapKey( tester, const Key('chat_invite_accept_button'), timeout: const Duration(seconds: 60), ); - await _waitForChatReady(tester, timeout: const Duration(seconds: 60)); - await _sendMessage(tester, container, _secondMessage); - await _expectMessageVisible(tester, _secondMessage); + await waitForChatReady(tester, timeout: const Duration(seconds: 60)); + await sendMessage(tester, container, _secondMessage); + await expectMessageVisible(tester, _secondMessage); - await _returnToChatList(tester); - await _switchProfile(tester, secondPubkey); - await _openChat(tester); - await _expectMessageVisible(tester, _secondMessage); + await returnToChatList(tester); + await switchProfile(tester, secondPubkey); + await openChat(tester, _groupName); + await expectMessageVisible(tester, _secondMessage); }); } - -Future _mountApp(WidgetTester tester) async { - await RustLib.init(); - - final root = await Directory.systemTemp.createTemp('whitenoise_integration_'); - addTearDown(() { - if (root.existsSync()) { - root.deleteSync(recursive: true); - } - }); - - final dataDir = Directory('${root.path}/data'); - final logsDir = Directory('${root.path}/logs'); - await dataDir.create(recursive: true); - await logsDir.create(recursive: true); - final config = await rust_api.createWhitenoiseConfig( - dataDir: dataDir.path, - logsDir: logsDir.path, - defaultRelayUrls: _relayUrls, - ); - await rust_api.initializeWhitenoise(config: config); - - final container = ProviderContainer( - overrides: [ - secureStorageProvider.overrideWithValue(MockSecureStorage()), - checkConnectivityFunctionProvider.overrideWithValue( - () async => [ConnectivityResult.wifi], - ), - connectivityStreamProvider.overrideWithValue(const Stream.empty()), - reachAnyRelayHostFunctionProvider.overrideWithValue((_) async => true), - ], - ); - addTearDown(container.dispose); - await container.read(authProvider.future); - - await tester.pumpWidget( - UncontrolledProviderScope(container: container, child: const WnApp()), - ); - await _pumpUntilFound(tester, find.byKey(const Key('auth_signup_button'))); - return container; -} - -Future _createIdentity( - WidgetTester tester, - ProviderContainer container, - String displayName, -) async { - await _tapKey(tester, const Key('auth_signup_button')); - await _enterTextInWidget( - tester, - const Key('signup_display_name_field'), - displayName, - ); - await _tapKey(tester, const Key('signup_create_profile_button')); - await _pumpUntilFound( - tester, - find.byKey(const Key('chat_add_button')), - timeout: const Duration(seconds: 90), - ); - final pubkey = container.read(authProvider).value; - expect(pubkey, isNotNull); - return pubkey!; -} - -Future _createAdditionalIdentity( - WidgetTester tester, - ProviderContainer container, - String displayName, -) async { - await _returnToChatList(tester); - await _openSettings(tester); - await _tapKey(tester, const Key('settings_switch_profile_button')); - await _tapKey(tester, const Key('connect_another_profile_button')); - return _createIdentity(tester, container, displayName); -} - -Future _copyPublicKey(WidgetTester tester) async { - await _returnToChatList(tester); - await _openSettings(tester); - await _tapKey(tester, const Key('settings_profile_keys_menu_item')); - final publicKeyField = find.byKey(const Key('profile_keys_public_key_field')); - await _pumpUntilFound(tester, publicKeyField); - await tester.tap( - find.descendant( - of: publicKeyField, - matching: find.byKey(const Key('copy_button')), - ), - ); - await tester.pump(const Duration(milliseconds: 200)); - final clipboardData = await Clipboard.getData('text/plain'); - final npub = clipboardData?.text; - expect(npub, isNotNull); - expect(npub, startsWith('npub1')); - await _returnToChatList(tester); - return npub!; -} - -Future _switchProfile(WidgetTester tester, String pubkey) async { - await _returnToChatList(tester); - await _openSettings(tester); - await _tapKey(tester, const Key('settings_switch_profile_button')); - await _tapKey( - tester, - Key('profile_switcher_item_$pubkey'), - ); - await _returnToChatList(tester); -} - -Future _startGroupChat( - WidgetTester tester, { - required String inviteeNpub, - required String inviteePubkey, -}) async { - await _tapKey(tester, const Key('chat_add_button')); - await _tapKey(tester, const Key('create_group_menu_item')); - await _enterTextInWidget( - tester, - const Key('user_selection_search_field'), - inviteeNpub, - ); - await _tapKey( - tester, - Key(inviteePubkey), - timeout: const Duration(seconds: 60), - ); - await _pumpUntilFound(tester, find.byKey(Key('bubble_$inviteePubkey'))); - await _tapKey(tester, const Key('user_selection_continue_button')); - await _enterTextInWidget( - tester, - const Key('set_up_group_name_field'), - _groupName, - ); - await _pumpUntilFound( - tester, - find.byKey(Key('member_$inviteePubkey')), - timeout: const Duration(seconds: 60), - ); - await _tapKey(tester, const Key('set_up_group_create_button')); - await _waitForChatReady(tester, timeout: const Duration(seconds: 90)); -} - -Future _sendMessage( - WidgetTester tester, - ProviderContainer container, - String message, -) async { - final input = find.descendant( - of: find.byKey(const Key('chat_message_input')), - matching: find.byType(TextField), - ); - await _pumpUntilFound(tester, input); - await tester.enterText(input, message); - await tester.pump(const Duration(milliseconds: 200)); - await tester.tap( - find.descendant( - of: find.byKey(const Key('chat_message_input')), - matching: find.byKey(const Key('send_button')), - ), - ); - try { - await _expectMessageVisible(tester, message); - } catch (_) { - fail( - 'Timed out waiting for sent message "$message".\n' - '${_messageDebugSummary(container)}', - ); - } -} - -String _messageDebugSummary(ProviderContainer container) { - final state = container.read(messageDebugLogProvider); - final sendLines = state.sendLog - .take(8) - .map((entry) { - final details = [ - entry.status.name, - 'group=${entry.groupId}', - if (entry.contentLen != null) 'len=${entry.contentLen}', - if (entry.resultId != null) 'result=${entry.resultId}', - if (entry.error != null) 'error=${entry.error}', - ]; - return 'send: ${details.join(' ')}'; - }) - .join('\n'); - final streamLines = state.streamLog - .take(12) - .map((entry) { - final details = [ - entry.eventType.name, - 'group=${entry.groupId}', - if (entry.messageCount != null) 'count=${entry.messageCount}', - if (entry.trigger != null) 'trigger=${entry.trigger}', - if (entry.messageId != null) 'message=${entry.messageId}', - if (entry.error != null) 'error=${entry.error}', - ]; - return 'stream: ${details.join(' ')}'; - }) - .join('\n'); - return [ - 'Message debug log:', - if (sendLines.isEmpty) 'send: ' else sendLines, - if (streamLines.isEmpty) 'stream: ' else streamLines, - ].join('\n'); -} - -Future _openInvite(WidgetTester tester) async { - await _pumpUntilFound( - tester, - find.text(_groupName), - timeout: const Duration(seconds: 90), - ); - await tester.tap(find.text(_groupName).first); - await _pumpUntilFound( - tester, - find.byKey(const Key('chat_invite_accept_button')), - timeout: const Duration(seconds: 60), - ); -} - -Future _openChat(WidgetTester tester) async { - await _pumpUntilFound( - tester, - find.text(_groupName), - timeout: const Duration(seconds: 60), - ); - await tester.tap(find.text(_groupName).first); - await _waitForChatReady(tester, timeout: const Duration(seconds: 60)); -} - -Future _expectMessageVisible(WidgetTester tester, String message) { - return _pumpUntilFound( - tester, - find.descendant( - of: find.byType(WnMessageBubble), - matching: find.textContaining(message, findRichText: true), - ), - timeout: const Duration(seconds: 90), - ); -} - -Future _waitForChatReady( - WidgetTester tester, { - Duration timeout = const Duration(seconds: 30), -}) async { - await _pumpUntilFound( - tester, - find.byKey(const Key('chat_message_input')), - timeout: timeout, - ); - await _pumpUntilNotFound( - tester, - find.byType(CircularProgressIndicator), - timeout: timeout, - ); -} - -Future _openSettings(WidgetTester tester) async { - await _tapKey(tester, const Key('avatar_button')); - await _pumpUntilFound( - tester, - find.byKey(const Key('settings_switch_profile_button')), - ); -} - -Future _returnToChatList(WidgetTester tester) async { - for (var attempt = 0; attempt < 8; attempt++) { - await tester.pump(const Duration(milliseconds: 200)); - if (find.byKey(const Key('chat_add_button')).evaluate().isNotEmpty) { - return; - } - - final chatBackButton = find.byKey(const Key('back_button')); - if (chatBackButton.evaluate().isNotEmpty) { - await tester.tap(chatBackButton.first); - continue; - } - - final slateBackButton = find.byKey(const Key('slate_back_button')); - if (slateBackButton.evaluate().isNotEmpty) { - await tester.tap(slateBackButton.first); - continue; - } - } - - fail('Timed out returning to the chat list'); -} - -Future _enterTextInWidget( - WidgetTester tester, - Key key, - String text, -) async { - final field = find.descendant( - of: find.byKey(key), - matching: find.byType(TextField), - ); - await _pumpUntilFound(tester, field); - await tester.enterText(field, text); - await tester.pump(const Duration(milliseconds: 200)); -} - -Future _tapKey( - WidgetTester tester, - Key key, { - Duration timeout = const Duration(seconds: 30), -}) async { - final finder = find.byKey(key); - await _pumpUntilFound(tester, finder, timeout: timeout); - await tester.ensureVisible(finder); - await tester.pump(const Duration(milliseconds: 300)); - await tester.tap(finder); - await tester.pump(const Duration(milliseconds: 200)); -} - -Future _pumpUntilFound( - WidgetTester tester, - Finder finder, { - Duration timeout = const Duration(seconds: 30), -}) async { - final deadline = DateTime.now().add(timeout); - while (DateTime.now().isBefore(deadline)) { - await tester.pump(const Duration(milliseconds: 250)); - if (finder.evaluate().isNotEmpty) return; - } - fail('Timed out waiting for $finder'); -} - -Future _pumpUntilNotFound( - WidgetTester tester, - Finder finder, { - Duration timeout = const Duration(seconds: 30), -}) async { - final deadline = DateTime.now().add(timeout); - while (DateTime.now().isBefore(deadline)) { - await tester.pump(const Duration(milliseconds: 250)); - if (finder.evaluate().isEmpty) return; - } - fail('Timed out waiting for $finder to disappear'); -} - -Future _expectLocalRelaysAvailable() async { - await _expectLocalRelayAvailable(8080); - await _expectLocalRelayAvailable(7777); -} - -Future _expectLocalRelayAvailable(int port) async { - try { - final socket = await Socket.connect( - '127.0.0.1', - port, - timeout: const Duration(seconds: 1), - ); - socket.destroy(); - } catch (error) { - fail( - 'Expected a local Nostr relay on 127.0.0.1:$port before running this integration test. ' - 'Run `docker compose up -d`, then run the test again. ' - 'Connection error: $error', - ); - } -} diff --git a/integration_test/messaging_interactions_test.dart b/integration_test/messaging_interactions_test.dart new file mode 100644 index 0000000..d82942e --- /dev/null +++ b/integration_test/messaging_interactions_test.dart @@ -0,0 +1,264 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:integration_test/integration_test.dart'; +import 'package:whitenoise/providers/auth_provider.dart'; +import 'package:whitenoise/widgets/wn_message_bubble.dart'; + +import '_support/harness.dart'; + +const _aliceDisplayName = 'Interactions Alice'; +const _bobDisplayName = 'Interactions Bob'; +const _groupName = 'Interactions Test Group'; +const _creatorSeedMessage = 'Bob seed'; +const _seedMessage = 'hello from Alice'; +const _replyMessage = 'Replying to you'; +const _deleteMessage = 'delete me'; + +class _Identities { + const _Identities({ + required this.container, + required this.aliceKey, + required this.bobKey, + }); + + final ProviderContainer container; + final String aliceKey; + final String bobKey; +} + +void main() { + IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + + // One testWidgets: the three phases share one costly setup. + testWidgets( + 'reaction, reply and deletion propagate between two identities', + (tester) async { + final ids = await _setUp(tester); + + await _reactionPhase(tester, ids); + await _replyPhase(tester, ids); + await _deletePhase(tester, ids); + }, + timeout: const Timeout(Duration(minutes: 12)), + ); +} + +Future<_Identities> _setUp(WidgetTester tester) async { + await expectLocalRelaysAvailable(); + final container = await mountApp(tester); + + final aliceKey = await createIdentity(tester, container, _aliceDisplayName); + await createAdditionalIdentity(tester, container, _bobDisplayName); + final activeBobKey = container.read(authProvider).value; + expect(activeBobKey, isNotNull); + final bobKey = activeBobKey!; + + await switchProfile(tester, aliceKey); + final aliceNpub = await copyPublicKey(tester); + + await switchProfile(tester, bobKey); + await startGroupChat( + tester, + groupName: _groupName, + inviteeNpub: aliceNpub, + inviteePubkey: aliceKey, + ); + await sendMessage(tester, container, _creatorSeedMessage); + await returnToChatList(tester); + + await switchProfile(tester, aliceKey); + await openInvite(tester, _groupName); + await tapKey( + tester, + const Key('chat_invite_accept_button'), + timeout: const Duration(seconds: 60), + ); + await waitForChatReady(tester, timeout: const Duration(seconds: 60)); + await sendMessage(tester, container, _seedMessage); + await returnToChatList(tester); + + await switchProfile(tester, bobKey); + await openChat(tester, _groupName); + await expectMessageVisible(tester, _seedMessage); + await returnToChatList(tester); + + return _Identities( + container: container, + aliceKey: aliceKey, + bobKey: bobKey, + ); +} + +Future _reactionPhase(WidgetTester tester, _Identities ids) async { + await switchProfile(tester, ids.aliceKey); + await openChat(tester, _groupName); + final seedId = await _messageIdForText(tester, _seedMessage); + await _longPressMessage(tester, seedId, _seedMessage); + await tapKey(tester, const Key('reaction_๐Ÿ‘')); + await _expectReactionOnMessage(tester, seedId, '๐Ÿ‘'); + + await returnToChatList(tester); + await switchProfile(tester, ids.bobKey); + await openChat(tester, _groupName); + final bobSeedId = await _messageIdForText(tester, _seedMessage); + await _expectReactionOnMessage(tester, bobSeedId, '๐Ÿ‘'); +} + +Future _replyPhase(WidgetTester tester, _Identities ids) async { + await switchProfile(tester, ids.bobKey); + await openChat(tester, _groupName); + final seedId = await _messageIdForText(tester, _seedMessage); + await _longPressMessage(tester, seedId, _seedMessage); + await tapKey(tester, const Key('reply_button')); + await pumpUntilFound(tester, find.byKey(const Key('cancel_quote_button'))); + + await _typeAndSend(tester, _replyMessage); + await expectMessageVisible(tester, _replyMessage); + _expectReplyQuotes(tester, replyText: _replyMessage, quoted: _seedMessage); + + await returnToChatList(tester); + await switchProfile(tester, ids.aliceKey); + await openChat(tester, _groupName); + await expectMessageVisible(tester, _replyMessage); + _expectReplyQuotes(tester, replyText: _replyMessage, quoted: _seedMessage); +} + +Future _deletePhase(WidgetTester tester, _Identities ids) async { + await switchProfile(tester, ids.aliceKey); + await openChat(tester, _groupName); + await sendMessage(tester, ids.container, _deleteMessage); + await returnToChatList(tester); + + // Capture the id while the message text is still rendered: after deletion the + // bubble no longer carries its original content, so a text lookup would fail. + await switchProfile(tester, ids.bobKey); + await openChat(tester, _groupName); + await expectMessageVisible(tester, _deleteMessage); + final bobDeleteId = await _messageIdForText(tester, _deleteMessage); + await returnToChatList(tester); + + await switchProfile(tester, ids.aliceKey); + await openChat(tester, _groupName); + final aliceDeleteId = await _messageIdForText(tester, _deleteMessage); + await _longPressMessage(tester, aliceDeleteId, _deleteMessage); + await tapKey(tester, const Key('delete_button')); + await _expectMessageDeleted(tester, aliceDeleteId); + + await returnToChatList(tester); + await switchProfile(tester, ids.bobKey); + await openChat(tester, _groupName); + await _expectMessageDeleted(tester, bobDeleteId); +} + +Future _longPressMessage( + WidgetTester tester, + String messageId, + String messageText, +) async { + final bubble = find.byKey(Key('message_$messageId')); + await pumpUntilFound(tester, bubble); + // The keyed WnMessageBubble spans the full row width, so its centre is empty + // space beside a content-sized, side-aligned bubble. Long-press the message + // text instead โ€” it always sits inside the visible bubble. + final content = find + .descendant( + of: bubble, + matching: find.textContaining(messageText, findRichText: true), + ) + .first; + await tester.ensureVisible(content); + await tester.pump(const Duration(milliseconds: 300)); + await tester.longPress(content); + await tester.pump(const Duration(milliseconds: 500)); + await pumpUntilFound(tester, find.byKey(const Key('reply_button'))); +} + +Future _typeAndSend(WidgetTester tester, String message) async { + final input = find.descendant( + of: find.byKey(const Key('chat_message_input')), + matching: find.byType(TextField), + ); + await pumpUntilFound(tester, input); + await tester.enterText(input, message); + await tester.pump(const Duration(milliseconds: 200)); + await tester.tap( + find.descendant( + of: find.byKey(const Key('chat_message_input')), + matching: find.byKey(const Key('send_button')), + ), + ); + await tester.pump(const Duration(milliseconds: 200)); +} + +/// Resolves the `message.id` of the bubble currently rendering [text] by reading +/// it back from that bubble's own `Key`. The id is derived from the live widget +/// rather than the send result, so it always matches the key the test searches +/// for afterwards. +Future _messageIdForText(WidgetTester tester, String text) async { + final textFinder = find.textContaining(text, findRichText: true); + await pumpUntilFound(tester, textFinder); + + final bubbleFinder = find.ancestor( + of: textFinder, + matching: find.byType(WnMessageBubble), + ); + final bubble = tester.widgetList(bubbleFinder).first; + final key = bubble.key; + + const prefix = 'message_'; + if (key is! ValueKey || !key.value.startsWith(prefix)) { + fail('WnMessageBubble for "$text" has an unexpected key: $key'); + } + return key.value.substring(prefix.length); +} + +Future _expectReactionOnMessage( + WidgetTester tester, + String messageId, + String emoji, +) async { + await pumpUntilFound( + tester, + find.descendant( + of: find.byKey(Key('message_$messageId')), + matching: find.byKey(ValueKey(emoji)), + ), + timeout: const Duration(seconds: 90), + ); +} + +Future _expectMessageDeleted( + WidgetTester tester, + String messageId, +) async { + await pumpUntilFound( + tester, + find.descendant( + of: find.byKey(Key('message_$messageId')), + matching: find.byKey(const Key('deleted_bubble_border')), + ), + timeout: const Duration(seconds: 90), + ); +} + +void _expectReplyQuotes( + WidgetTester tester, { + required String replyText, + required String quoted, +}) { + final replyBubble = find + .ancestor( + of: find.textContaining(replyText, findRichText: true), + matching: find.byType(WnMessageBubble), + ) + .first; + expect(replyBubble, findsOneWidget); + expect( + find.descendant( + of: replyBubble, + matching: find.textContaining(quoted, findRichText: true), + ), + findsWidgets, + ); +} diff --git a/justfile b/justfile index c410bee..bfe4c40 100644 --- a/justfile +++ b/justfile @@ -168,41 +168,51 @@ test-flutter-quiet: echo "No test directory found."; \ fi +# Resolves the integration-test device: the given id, else the one booted simulator. +_resolve-device device: + @device="{{ device }}"; \ + if [ -n "$device" ]; then echo "$device"; exit 0; fi; \ + booted=$(xcrun simctl list devices booted 2>/dev/null | grep -oiE '[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}'); \ + count=$(printf '%s' "$booted" | grep -c .); \ + if [ "$count" -eq 1 ]; then \ + echo "Using booted simulator $booted" >&2; \ + echo "$booted"; \ + elif [ "$count" -eq 0 ]; then \ + echo "No device id given and no booted simulator found. Boot one, pass a device id, or set WHITENOISE_INTEGRATION_DEVICE." >&2; \ + exit 1; \ + else \ + echo "Multiple booted simulators โ€” pass a device id or set WHITENOISE_INTEGRATION_DEVICE:" >&2; \ + xcrun simctl list devices booted >&2; \ + exit 1; \ + fi + # Run Flutter integration tests. Requires local Nostr relays on ports 8080 and 7777. -# Pass an iOS/Android device id, or set WHITENOISE_INTEGRATION_DEVICE. -test-flutter-integration device=env("WHITENOISE_INTEGRATION_DEVICE", "") flavor="staging": +# Run one file by passing its path: `just int-test integration_test/messaging_interactions_test.dart`. +# Device: WHITENOISE_INTEGRATION_DEVICE, else the one booted simulator. +int-test target="integration_test/all_tests.dart" device=env("WHITENOISE_INTEGRATION_DEVICE", "") flavor="staging": @echo "๐Ÿงช Testing Flutter integration flows..." - @device="{{ device }}"; \ - if [ -z "$device" ]; then \ - echo "Pass a device id or set WHITENOISE_INTEGRATION_DEVICE."; \ - flutter devices; \ - exit 1; \ - fi; \ + @device=$(just _resolve-device "{{ device }}") || exit 1; \ if [ -n "{{ flavor }}" ]; then \ - flutter test -d "$device" --flavor {{ flavor }} integration_test; \ + flutter test -d "$device" --flavor {{ flavor }} {{ target }}; \ else \ - flutter test -d "$device" integration_test; \ + flutter test -d "$device" {{ target }}; \ fi # Run Flutter integration tests with minimal output. Requires local Nostr relays on ports 8080 and 7777. -# Pass an iOS/Android device id, or set WHITENOISE_INTEGRATION_DEVICE. -test-flutter-integration-quiet device=env("WHITENOISE_INTEGRATION_DEVICE", "") flavor="staging": - @if [ -d "integration_test" ]; then \ - device="{{ device }}"; \ - if [ -z "$device" ]; then \ - echo "Pass a device id or set WHITENOISE_INTEGRATION_DEVICE."; \ - flutter devices; \ - exit 1; \ - fi; \ - if [ -n "{{ flavor }}" ]; then \ - flutter test -d "$device" --flavor {{ flavor }} --no-pub --reporter=failures-only integration_test; \ - else \ - flutter test -d "$device" --no-pub --reporter=failures-only integration_test; \ - fi; \ +# Run one file by passing its path: `just int-test-quiet integration_test/messaging_interactions_test.dart`. +# Device: WHITENOISE_INTEGRATION_DEVICE, else the one booted simulator. +int-test-quiet target="integration_test/all_tests.dart" device=env("WHITENOISE_INTEGRATION_DEVICE", "") flavor="staging": + @if [ ! -e "{{ target }}" ]; then \ + echo "No integration test target found at {{ target }}."; \ + exit 1; \ + fi; \ + device=$(just _resolve-device "{{ device }}") || exit 1; \ + if [ -n "{{ flavor }}" ]; then \ + flutter test -d "$device" --flavor {{ flavor }} --no-pub --reporter=failures-only {{ target }}; \ else \ - echo "No integration_test directory found."; \ + flutter test -d "$device" --no-pub --reporter=failures-only {{ target }}; \ fi coverage min="99": diff --git a/lib/src/rust/api/error.freezed.dart b/lib/src/rust/api/error.freezed.dart index cda890f..4c61fa2 100644 --- a/lib/src/rust/api/error.freezed.dart +++ b/lib/src/rust/api/error.freezed.dart @@ -254,7 +254,7 @@ return other(_that.message);case _: class ApiError_Whitenoise extends ApiError { const ApiError_Whitenoise({required this.message}): super._(); - + final String message; @@ -320,7 +320,7 @@ as String, class ApiError_DatabasePoolTimedOut extends ApiError { const ApiError_DatabasePoolTimedOut({required this.message}): super._(); - + final String message; @@ -386,7 +386,7 @@ as String, class ApiError_InvalidKey extends ApiError { const ApiError_InvalidKey({required this.message}): super._(); - + final String message; @@ -452,7 +452,7 @@ as String, class ApiError_NostrUrl extends ApiError { const ApiError_NostrUrl({required this.message}): super._(); - + final String message; @@ -518,7 +518,7 @@ as String, class ApiError_NostrTag extends ApiError { const ApiError_NostrTag({required this.message}): super._(); - + final String message; @@ -584,7 +584,7 @@ as String, class ApiError_NostrEvent extends ApiError { const ApiError_NostrEvent({required this.message}): super._(); - + final String message; @@ -650,7 +650,7 @@ as String, class ApiError_NostrParse extends ApiError { const ApiError_NostrParse({required this.message}): super._(); - + final String message; @@ -716,7 +716,7 @@ as String, class ApiError_NostrHex extends ApiError { const ApiError_NostrHex({required this.message}): super._(); - + final String message; @@ -782,7 +782,7 @@ as String, class ApiError_LoginInvalidKeyFormat extends ApiError { const ApiError_LoginInvalidKeyFormat({required this.message}): super._(); - + final String message; @@ -848,7 +848,7 @@ as String, class ApiError_LoginNoRelayConnections extends ApiError { const ApiError_LoginNoRelayConnections(): super._(); - + @@ -880,7 +880,7 @@ String toString() { class ApiError_LoginTimeout extends ApiError { const ApiError_LoginTimeout({required this.message}): super._(); - + final String message; @@ -946,7 +946,7 @@ as String, class ApiError_LoginNoLoginInProgress extends ApiError { const ApiError_LoginNoLoginInProgress(): super._(); - + @@ -978,7 +978,7 @@ String toString() { class ApiError_LoginInternal extends ApiError { const ApiError_LoginInternal({required this.message}): super._(); - + final String message; @@ -1044,7 +1044,7 @@ as String, class ApiError_LoginKeyringUnavailable extends ApiError { const ApiError_LoginKeyringUnavailable({required this.message}): super._(); - + final String message; @@ -1110,7 +1110,7 @@ as String, class ApiError_Other extends ApiError { const ApiError_Other({required this.message}): super._(); - + final String message; diff --git a/lib/src/rust/api/markdown.freezed.dart b/lib/src/rust/api/markdown.freezed.dart index a3d0ced..369fb18 100644 --- a/lib/src/rust/api/markdown.freezed.dart +++ b/lib/src/rust/api/markdown.freezed.dart @@ -212,7 +212,7 @@ return mathBlock(_that.content);case _: class MarkdownBlock_Paragraph extends MarkdownBlock { const MarkdownBlock_Paragraph({required final List inlines}): _inlines = inlines,super._(); - + final List _inlines; List get inlines { @@ -284,7 +284,7 @@ as List, class MarkdownBlock_Heading extends MarkdownBlock { const MarkdownBlock_Heading({required this.level, required final List inlines}): _inlines = inlines,super._(); - + final int level; final List _inlines; @@ -358,7 +358,7 @@ as List, class MarkdownBlock_ThematicBreak extends MarkdownBlock { const MarkdownBlock_ThematicBreak(): super._(); - + @@ -390,7 +390,7 @@ String toString() { class MarkdownBlock_CodeBlock extends MarkdownBlock { const MarkdownBlock_CodeBlock({required this.kind, required this.info, required this.content}): super._(); - + final MarkdownCodeBlockKind kind; final String info; @@ -460,7 +460,7 @@ as String, class MarkdownBlock_BlockQuote extends MarkdownBlock { const MarkdownBlock_BlockQuote({required final List blocks}): _blocks = blocks,super._(); - + final List _blocks; List get blocks { @@ -532,7 +532,7 @@ as List, class MarkdownBlock_List extends MarkdownBlock { const MarkdownBlock_List({required this.kind, required this.tight, required final List items}): _items = items,super._(); - + final MarkdownListKind kind; final bool tight; @@ -605,7 +605,7 @@ as List, @override @pragma('vm:prefer-inline') $MarkdownListKindCopyWith<$Res> get kind { - + return $MarkdownListKindCopyWith<$Res>(_self.kind, (value) { return _then(_self.copyWith(kind: value)); }); @@ -617,7 +617,7 @@ $MarkdownListKindCopyWith<$Res> get kind { class MarkdownBlock_Table extends MarkdownBlock { const MarkdownBlock_Table({required final List alignments, required final List header, required final List> rows}): _alignments = alignments,_header = header,_rows = rows,super._(); - + final List _alignments; List get alignments { @@ -705,7 +705,7 @@ as List>, class MarkdownBlock_MathBlock extends MarkdownBlock { const MarkdownBlock_MathBlock({required this.content}): super._(); - + final String content; @@ -997,7 +997,7 @@ return nostrUri(_that.entity);case _: class MarkdownInline_Text extends MarkdownInline { const MarkdownInline_Text({required this.content}): super._(); - + final String content; @@ -1063,7 +1063,7 @@ as String, class MarkdownInline_SoftBreak extends MarkdownInline { const MarkdownInline_SoftBreak(): super._(); - + @@ -1095,7 +1095,7 @@ String toString() { class MarkdownInline_HardBreak extends MarkdownInline { const MarkdownInline_HardBreak(): super._(); - + @@ -1127,7 +1127,7 @@ String toString() { class MarkdownInline_Code extends MarkdownInline { const MarkdownInline_Code({required this.content}): super._(); - + final String content; @@ -1193,7 +1193,7 @@ as String, class MarkdownInline_Emph extends MarkdownInline { const MarkdownInline_Emph({required final List children}): _children = children,super._(); - + final List _children; List get children { @@ -1265,7 +1265,7 @@ as List, class MarkdownInline_Strong extends MarkdownInline { const MarkdownInline_Strong({required final List children}): _children = children,super._(); - + final List _children; List get children { @@ -1337,7 +1337,7 @@ as List, class MarkdownInline_Strikethrough extends MarkdownInline { const MarkdownInline_Strikethrough({required final List children}): _children = children,super._(); - + final List _children; List get children { @@ -1409,7 +1409,7 @@ as List, class MarkdownInline_Link extends MarkdownInline { const MarkdownInline_Link({required this.dest, this.title, required final List children}): _children = children,super._(); - + final String dest; final String? title; @@ -1485,7 +1485,7 @@ as List, class MarkdownInline_Image extends MarkdownInline { const MarkdownInline_Image({required this.dest, this.title, required final List alt}): _alt = alt,super._(); - + final String dest; final String? title; @@ -1561,7 +1561,7 @@ as List, class MarkdownInline_Autolink extends MarkdownInline { const MarkdownInline_Autolink({required this.url, required this.kind}): super._(); - + final String url; final MarkdownAutolinkKind kind; @@ -1629,7 +1629,7 @@ as MarkdownAutolinkKind, class MarkdownInline_Math extends MarkdownInline { const MarkdownInline_Math({required this.content}): super._(); - + final String content; @@ -1695,7 +1695,7 @@ as String, class MarkdownInline_NostrMention extends MarkdownInline { const MarkdownInline_NostrMention({required this.entity}): super._(); - + final MarkdownNostrEntity entity; @@ -1761,7 +1761,7 @@ as MarkdownNostrEntity, class MarkdownInline_NostrUri extends MarkdownInline { const MarkdownInline_NostrUri({required this.entity}): super._(); - + final MarkdownNostrEntity entity; @@ -1987,7 +1987,7 @@ return ordered(_that.start,_that.delimiter);case _: class MarkdownListKind_Bullet extends MarkdownListKind { const MarkdownListKind_Bullet({required this.marker}): super._(); - + final String marker; @@ -2053,7 +2053,7 @@ as String, class MarkdownListKind_Ordered extends MarkdownListKind { const MarkdownListKind_Ordered({required this.start, required this.delimiter}): super._(); - + final int start; final String delimiter; diff --git a/lib/widgets/chat_list_tile.dart b/lib/widgets/chat_list_tile.dart index 42bf823..66800c4 100644 --- a/lib/widgets/chat_list_tile.dart +++ b/lib/widgets/chat_list_tile.dart @@ -434,23 +434,26 @@ class ChatListTile extends HookConsumerWidget { ); } - return WnChatListItem( - key: itemKey, - onTap: isPending - ? () => Routes.pushToInvite(context, chatSummary.mlsGroupId) - : () => Routes.goToChat(context, chatSummary.mlsGroupId), - onLongPress: isPending ? null : showContextMenu, - title: display.title, - subtitle: display.subtitle, - timestamp: display.formattedTime, - avatarUrl: display.pictureUrl, - avatarName: display.avatarName, - avatarColor: display.avatarColor, - showPinned: display.showPinned, - status: display.status, - unreadCount: display.unreadCount, - prefixSubtitle: display.prefixSubtitle, - subtitleIcon: display.subtitleIcon, + return KeyedSubtree( + key: Key('chat_list_tile_${chatSummary.mlsGroupId}'), + child: WnChatListItem( + key: itemKey, + onTap: isPending + ? () => Routes.pushToInvite(context, chatSummary.mlsGroupId) + : () => Routes.goToChat(context, chatSummary.mlsGroupId), + onLongPress: isPending ? null : showContextMenu, + title: display.title, + subtitle: display.subtitle, + timestamp: display.formattedTime, + avatarUrl: display.pictureUrl, + avatarName: display.avatarName, + avatarColor: display.avatarColor, + showPinned: display.showPinned, + status: display.status, + unreadCount: display.unreadCount, + prefixSubtitle: display.prefixSubtitle, + subtitleIcon: display.subtitleIcon, + ), ); } } diff --git a/lib/widgets/chat_message_bubble.dart b/lib/widgets/chat_message_bubble.dart index d37cabe..1dd4c39 100644 --- a/lib/widgets/chat_message_bubble.dart +++ b/lib/widgets/chat_message_bubble.dart @@ -148,6 +148,7 @@ class ChatMessageBubble extends StatelessWidget { final showStatus = showTail || _deliveryStatusType == ChatStatusType.failed; return WnMessageBubble( + key: Key('message_${message.id}'), direction: isOwnMessage ? MessageDirection.outgoing : MessageDirection.incoming, isDeleted: message.isDeleted, deletedLabel: message.isDeleted diff --git a/lib/widgets/wn_chat_list_item.dart b/lib/widgets/wn_chat_list_item.dart index 1daca02..bc74856 100644 --- a/lib/widgets/wn_chat_list_item.dart +++ b/lib/widgets/wn_chat_list_item.dart @@ -114,6 +114,7 @@ class WnChatListItem extends HookWidget { children: [ Expanded( child: Text.rich( + key: const Key('chat_list_subtitle'), TextSpan( style: typography.medium14Compact.copyWith( color: colors.backgroundContentSecondary, diff --git a/pubspec.yaml b/pubspec.yaml index f3fa7ce..20f808c 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -2,7 +2,7 @@ name: whitenoise description: "Secure messaging app using the Marmot Protocol" # The following line prevents the package from being accidentally published to # pub.dev using `flutter pub publish`. This is preferred for private packages. -publish_to: 'none' # Remove this line if you wish to publish to pub.dev +publish_to: "none" # Remove this line if you wish to publish to pub.dev # The following defines the version and build number for your application. # A version number is three numbers separated by dots, like 1.2.43 @@ -113,8 +113,8 @@ flutter: uses-material-design: true assets: - - assets/images/ - - assets/svgs/ + - assets/images/ + - assets/svgs/ # An image asset can refer to one or more resolution-specific "variants", see # https://flutter.dev/to/resolution-aware-images diff --git a/reviews/PR-517-start-chat-flow-improvements.md b/reviews/PR-517-start-chat-flow-improvements.md deleted file mode 100644 index d05adc4..0000000 --- a/reviews/PR-517-start-chat-flow-improvements.md +++ /dev/null @@ -1,201 +0,0 @@ -# Code Review: PR #517 โ€” Start Chat Flow Improvements - -## Summary - -This PR improves the start-chat UX in several meaningful ways: the loading skeleton for `StartChatScreen` now shows the user avatar immediately while the key-package check runs; metadata is fetched freshly rather than passed through router `extra`; "Chat with support" moves from the search screen to settings; and the app-logs screen gains level-filter toggles. Performance timing is added throughout using a new `logDuration` utility. The changes are well-structured and test coverage is solid. There are a handful of issues worth addressing before merging, mostly around a subtle semantic inconsistency in the filtered-count display and a dead `AnimatedOpacity`. - ---- - -## Issues - -### Logic: `hasFilters` doesn't account for level filtering โ€” filtered count never shows for level-only filters - -**`lib/screens/app_logs_screen.dart:105โ€“108`** - -```dart -final hasFilters = - filter.searchQuery.isNotEmpty || - filter.includePatterns.isNotEmpty || - filter.excludePatterns.isNotEmpty; -``` - -`hasFilters` drives the "showing X of Y" count label. The PR adds level-filter toggles that actively hide entries, but since level selection isn't included in `hasFilters`, when the user filters by level only, the count text never appears โ€” they get no feedback that entries are being hidden. - -The fix is straightforward: - -```dart -final defaultLevels = {Level.WARNING, Level.SEVERE, Level.SHOUT}; -final hasFilters = - filter.searchQuery.isNotEmpty || - filter.includePatterns.isNotEmpty || - filter.excludePatterns.isNotEmpty || - !defaultLevels.containsAll(filter.selectedLevels) || - filter.selectedLevels.length != defaultLevels.length; -``` - -Or simpler: expose an `isDefaultLevels` getter on `AppLogFilterState`. - ---- - -### Bug: `AnimatedOpacity` with hardcoded `opacity: 1` is a no-op - -**`lib/screens/start_chat_screen.dart` (inside `isKeyPackageLoading` branch)** - -```dart -AnimatedOpacity( - opacity: 1, - duration: const Duration(milliseconds: 200), - child: CircularProgressIndicator(...), -), -``` - -The opacity is permanently `1` and never changes โ€” `AnimatedOpacity` has no effect here. The widget goes from not existing (when not loading) to existing at full opacity. Either: -- Remove `AnimatedOpacity` and use a plain widget (current behavior, simpler), or -- Animate it properly by tracking an opacity state variable that starts at 0 and transitions to 1. - -This is pure noise in the widget tree as written. - ---- - -### Semantic inconsistency: `totalEntries` in app logs screen now counts pre-level-filter entries - -**`lib/screens/app_logs_screen.dart:74โ€“76`** - -```dart -final rawEntries = paused.value ? frozenRawEntries.value : liveRawEntries; -final entries = applyFilter(rawEntries); -final totalEntries = rawEntries.length; -``` - -`totalEntries` is used in the "X of Y" filtered count label. Previously it was `liveRawEntries.length` (always live). Now it's `rawEntries.length`, which is correct for the paused case, but the label now reads as "X of Y" where Y is raw-unfiltered-by-level entries, even when INFO is toggled on and increases entry count. The filtered-count label intends to show "how many entries match your text/pattern search out of how many are visible given your log level", but `totalEntries` is still the fully unfiltered count. This is a mild inconsistency โ€” consider whether `totalEntries` should be post-level-filter. - ---- - -### Behavior change: `followState.isLoading` added to the button loading indicator - -**`lib/screens/start_chat_screen.dart` (inside `validActionsColumn`)** - -```dart -loading: showLoadingStates && (followState.isLoading || followState.isActionLoading), -``` - -The old code only showed loading on `followState.isActionLoading`. Adding `followState.isLoading` means the follow button spins during the initial data fetch โ€” which could feel jarring if the fetch is fast (a spinner flash). Confirm this is intentional. If `followState.isLoading` means "initial fetch is in progress", the button probably shouldn't exist at all yet, or should be disabled, rather than showing a spinner. - ---- - -### Missing test: settings screen "Chat with support" doesn't test the `isLoading` guard - -**`test/screens/settings_screen_test.dart`** - -The new `WnMenuItem` in `SettingsScreen` has an early-return guard: - -```dart -onTap: () { - if (helpState.isLoading) return; - ... -} -``` - -There's no test for the loading state scenario โ€” i.e., that tapping while `helpState.isLoading` is true does nothing and doesn't navigate. This is a covered branch in the code but untested. - ---- - -## Suggestions - -### Style: Inconsistent naming โ€” `stopWatch` vs `sw` - -Performance is instrumented in `use_start_dm.dart`, `use_user_has_key_package.dart`, and `start_chat_screen.dart` using `stopWatch`, while `user_service.dart` uses `sw`. Pick one convention and apply it consistently. Given the project emphasises self-documenting code, `stopwatch` (or `sw` uniformly) is fine โ€” just keep it consistent. - ---- - -### Style: Inner function `validActionsColumn` defined in `build` - -**`lib/screens/start_chat_screen.dart:101`** - -```dart -Widget validActionsColumn({bool showLoadingStates = true}) { ... } -``` - -Defining `Widget`-returning functions inside `build` is a pattern the Flutter team discounts โ€” it bypasses element diffing and rebuilds the entire subtree unconditionally. Prefer extracting as a private `StatelessWidget` (screen-scoped, named `_StartChatActionsColumn` per AGENTS.md convention). The `showLoadingStates` flag and the data it needs can be constructor params. - ---- - -### Style: `calloutTitleAndDescription` is also a build-scoped function - -**`lib/screens/start_chat_screen.dart`** - -Same concern as above โ€” `calloutTitleAndDescription()` is a local function that returns a record. It's only called once. Extract it or inline it; the current indirection adds a layer without clarity. - ---- - -### Suggestion: `logDuration` threshold (50ms) is hardcoded and undocumented - -**`lib/utils/logging.dart:3`** - -```dart -void logDuration(Logger logger, String message, int milliseconds) { - if (milliseconds >= 50) { - logger.warning('$message ${milliseconds}ms'); - } -``` - -The 50ms threshold is arbitrary and has no comment explaining why. Either name it as a constant (`_slowThresholdMs`) or add a brief comment. Also: the function doesn't accept an optional threshold, so callers can't tune it for fast vs slow operations. Not blocking, but the magic number will prompt questions later. - ---- - -### Suggestion: `use_start_dm.dart` โ€” stopwatch isn't reset before `createGroup`, total and create times overlap - -**`lib/hooks/use_start_dm.dart:51`** - -```dart -final createGroupStopWatch = Stopwatch()..start(); -final group = await groups_api.createGroup(...); -logDuration(_logger, 'createGroup took', createGroupStopWatch.elapsedMilliseconds); -logDuration(_logger, 'Total DM creation', totalStopWatch.elapsedMilliseconds); -``` - -This is fine โ€” two stopwatches, one measures createGroup only, one is the end-to-end total. The naming (`totalStopWatch` vs `createGroupStopWatch`) is clear. Minor note: `createGroupStopWatch` is never stopped before reading; Dart stopwatch semantics make this correct, but it's worth a comment to signal intent. - ---- - -### Suggestion: `use_user_search.dart` โ€” `follows` variable can be removed - -**`lib/hooks/use_user_search.dart:87โ€“105`** - -```dart -() async { - final stopWatch = Stopwatch()..start(); - try { - final follows = await accounts_api.accountFollows(pubkey: accountPubkey); - return follows; - } finally { - logDuration(...); - } -}, -``` - -`follows` can be returned directly: `return accounts_api.accountFollows(...)`. The intermediate variable serves no purpose. - ---- - -### Design: Duplicate level-filter logic in screen vs provider - -**`lib/screens/app_logs_screen.dart:48โ€“74`** and **`lib/providers/app_log_filter_provider.dart`** - -`AppLogsScreen` re-implements level filtering in its local `applyFilter` function, which mirrors the logic already in `filteredAppLogProvider`. The screen already watches `filteredAppLogProvider` indirectly through `filter` โ€” but it applies the filter again on the raw entries for the paused state. This duplication means any future change to filter logic needs to be applied in two places. Consider whether `frozenRawEntries` could be filtered through the same provider mechanism, or extract the filter logic into a standalone function both can call. - ---- - -## What's Done Well - -- **The loading-skeleton UX approach is clever.** Using `Visibility(maintainSize: true)` to hold space for buttons while showing a spinner prevents layout jank without the complexity of explicit size constraints. The test that validates this (`keeps button layout stable while key package loads`) directly verifies the intent. - -- **`useSupportChat` null-pubkey guard is clean.** Changing `accountPubkey` to `String?` and short-circuiting with `Future.value()` is idiomatic and eliminates a crash path. The test covers it. - -- **`logDuration` utility is a good abstraction.** A small, single-responsibility utility tested with clear threshold boundaries (`>=50ms` โ†’ warning, `<50ms` โ†’ info). No over-engineering. - -- **Removing `initialMetadata` from `StartChatScreen`** simplifies the component contract. The old pattern of passing stale router `extra` metadata then overwriting it with fresh data was a source of bugs (noted in the PR description). Removing it and always fetching fresh is the right call. - -- **Test updates are thorough.** Tests for the `use_chat_profile` blocking-metadata fallback, the `use_support_chat` null-pubkey case, and the level-toggle provider behaviour all represent genuine regression coverage, not just padding. - -- **Moving "Chat with support" to Settings** is a good UX decision. The search screen is for finding people; putting a support shortcut there was conceptually out of place.