diff --git a/test/example_data.dart b/test/example_data.dart index fc32781a1e..06d6d130c0 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -17,6 +17,7 @@ import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/settings.dart'; import 'package:zulip/model/store.dart'; +import 'model/binding.dart'; import 'model/test_store.dart'; import 'stdlib_checks.dart'; @@ -130,7 +131,14 @@ GetServerSettingsResult serverSettings({ ); } -ServerEmojiData serverEmojiDataPopular = ServerEmojiData(codeToNames: { +ServerEmojiData _immutableServerEmojiData({ + required Map> codeToNames}) { + return ServerEmojiData( + codeToNames: Map.unmodifiable(codeToNames.map( + (k, v) => MapEntry(k, List.unmodifiable(v))))); +} + +final ServerEmojiData serverEmojiDataPopular = _immutableServerEmojiData(codeToNames: { '1f44d': ['+1', 'thumbs_up', 'like'], '1f389': ['tada'], '1f642': ['slight_smile'], @@ -157,7 +165,7 @@ ServerEmojiData serverEmojiDataPopularPlus(ServerEmojiData data) { /// /// zulip/zulip@9feba0f16f is a Server 11 commit. // TODO(server-11) can drop this -ServerEmojiData serverEmojiDataPopularLegacy = ServerEmojiData(codeToNames: { +final ServerEmojiData serverEmojiDataPopularLegacy = _immutableServerEmojiData(codeToNames: { '1f44d': ['+1', 'thumbs_up', 'like'], '1f389': ['tada'], '1f642': ['smile'], @@ -289,24 +297,77 @@ Account account({ ); } -final User selfUser = user(fullName: 'Self User'); +/// A [User] which throws on attempting to mutate any of its fields. +/// +/// We use this to prevent any tests from leaking state through having a +/// [PerAccountStore] (which will be discarded when [TestZulipBinding.reset] +/// is called at the end of the test case) mutate a [User] in its [UserStore] +/// which happens to a value in this file like [selfUser] (which will not be +/// discarded by [TestZulipBinding.reset]). That was the cause of issue #1712. +class _ImmutableUser extends User { + _ImmutableUser.copyUser(User user) : super( + // When adding a field here, be sure to add the corresponding setter below. + userId: user.userId, + deliveryEmail: user.deliveryEmail, + email: user.email, + fullName: user.fullName, + dateJoined: user.dateJoined, + isActive: user.isActive, + isBillingAdmin: user.isBillingAdmin, + isBot: user.isBot, + botType: user.botType, + botOwnerId: user.botOwnerId, + role: user.role, + timezone: user.timezone, + avatarUrl: user.avatarUrl, + avatarVersion: user.avatarVersion, + profileData: user.profileData == null ? null : Map.unmodifiable(user.profileData!), + isSystemBot: user.isSystemBot, + // When adding a field here, be sure to add the corresponding setter below. + ); + + static final Error _error = UnsupportedError( + 'Cannot mutate immutable User.\n' + 'When a test needs to have the store handle an event which will\n' + 'modify a user, use `eg.user()` to make a fresh User object\n' + 'instead of using a shared User object like `eg.selfUser`.'); + + // userId already immutable + @override set deliveryEmail(_) => throw _error; + @override set email(_) => throw _error; + @override set fullName(_) => throw _error; + // dateJoined already immutable + @override set isActive(_) => throw _error; + @override set isBillingAdmin(_) => throw _error; + // isBot already immutable + // botType already immutable + @override set botOwnerId(_) => throw _error; + @override set role(_) => throw _error; + @override set timezone(_) => throw _error; + @override set avatarUrl(_) => throw _error; + @override set avatarVersion(_) => throw _error; + @override set profileData(_) => throw _error; + // isSystemBot already immutable +} + +final User selfUser = _ImmutableUser.copyUser(user(fullName: 'Self User')); +final User otherUser = _ImmutableUser.copyUser(user(fullName: 'Other User')); +final User thirdUser = _ImmutableUser.copyUser(user(fullName: 'Third User')); +final User fourthUser = _ImmutableUser.copyUser(user(fullName: 'Fourth User')); + +// There's no need for an [Account] analogue of [_ImmutableUser], +// because [Account] (which is generated by Drift) is already immutable. final Account selfAccount = account( id: 1001, user: selfUser, apiKey: 'dQcEJWTq3LczosDkJnRTwf31zniGvMrO', // A Zulip API key is 32 digits of base64. ); - -final User otherUser = user(fullName: 'Other User'); final Account otherAccount = account( id: 1002, user: otherUser, apiKey: '6dxT4b73BYpCTU+i4BB9LAKC5h/CufqY', // A Zulip API key is 32 digits of base64. ); -final User thirdUser = user(fullName: 'Third User'); - -final User fourthUser = user(fullName: 'Fourth User'); - //////////////////////////////////////////////////////////////// // Data attached to the self-account on the realm // @@ -448,21 +509,21 @@ UserTopicItem userTopicItem( // Messages, and pieces of messages. // -Reaction unicodeEmojiReaction = Reaction( +final Reaction unicodeEmojiReaction = Reaction( emojiName: 'thumbs_up', emojiCode: '1f44d', reactionType: ReactionType.unicodeEmoji, userId: selfUser.userId, ); -Reaction realmEmojiReaction = Reaction( +final Reaction realmEmojiReaction = Reaction( emojiName: 'twocents', emojiCode: '181', reactionType: ReactionType.realmEmoji, userId: selfUser.userId, ); -Reaction zulipExtraEmojiReaction = Reaction( +final Reaction zulipExtraEmojiReaction = Reaction( emojiName: 'zulip', emojiCode: 'zulip', reactionType: ReactionType.zulipExtraEmoji, diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 177aa43503..bc9b53711f 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -55,6 +55,7 @@ late FakeApiConnection connection; Future setupToMessageActionSheet(WidgetTester tester, { required Message message, required Narrow narrow, + User? selfUser, User? sender, List? mutedUserIds, bool? realmAllowMessageEditing, @@ -67,15 +68,17 @@ Future setupToMessageActionSheet(WidgetTester tester, { // TODO(#1667) will be null in a search narrow; remove `!`. assert(narrow.containsMessage(message)!); + selfUser ??= eg.selfUser; + final selfAccount = eg.account(user: selfUser); await testBinding.globalStore.add( - eg.selfAccount, + selfAccount, eg.initialSnapshot( realmAllowMessageEditing: realmAllowMessageEditing, realmMessageContentEditLimitSeconds: realmMessageContentEditLimitSeconds, )); - store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store = await testBinding.globalStore.perAccount(selfAccount.id); await store.addUsers([ - eg.selfUser, + selfUser, sender ?? eg.user(userId: message.senderId), if (narrow is DmNarrow) ...narrow.otherRecipientIds.map((id) => eg.user(userId: id)), @@ -97,7 +100,7 @@ Future setupToMessageActionSheet(WidgetTester tester, { connection.prepare(json: eg.newestGetMessagesResult( foundOldest: true, messages: [message]).toJson()); - await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id, child: MessageListPage(initNarrow: narrow))); // global store, per-account store, and message list get loaded @@ -1204,11 +1207,13 @@ void main() { }); testWidgets('no error if user lost posting permission after action sheet opened', (tester) async { + final selfUser = eg.user(role: UserRole.member); final stream = eg.stream(); final message = eg.streamMessage(stream: stream); - await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); + await setupToMessageActionSheet(tester, selfUser: selfUser, + message: message, narrow: TopicNarrow.ofMessage(message)); - await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.selfUser.userId, + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: selfUser.userId, role: UserRole.guest)); await store.handleEvent(eg.channelUpdateEvent(stream, property: ChannelPropertyName.channelPostPolicy, @@ -1240,7 +1245,8 @@ void main() { }); testWidgets('no error if recipient was deactivated while raw-content request in progress', (tester) async { - final message = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]); + final otherUser = eg.user(); + final message = eg.dmMessage(from: eg.selfUser, to: [otherUser]); await setupToMessageActionSheet(tester, message: message, narrow: DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); @@ -1253,7 +1259,7 @@ void main() { await tapQuoteAndReplyButton(tester); await tester.pump(const Duration(seconds: 1)); // message not yet fetched - await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.otherUser.userId, + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: otherUser.userId, isActive: false)); await tester.pump(); // no error diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 3eb49f2ca8..e898218e6e 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -27,16 +27,18 @@ import 'test_app.dart'; Future setupPage(WidgetTester tester, { required List dmMessages, required List users, + User? selfUser, List? mutedUserIds, NavigatorObserver? navigatorObserver, - String? newNameForSelfUser, }) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + selfUser ??= eg.selfUser; + final selfAccount = eg.account(user: selfUser); + await testBinding.globalStore.add(selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(selfAccount.id); - await store.addUser(eg.selfUser); + await store.addUser(selfUser); for (final user in users) { await store.addUser(user); } @@ -46,13 +48,8 @@ Future setupPage(WidgetTester tester, { await store.addMessages(dmMessages); - if (newNameForSelfUser != null) { - await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.selfUser.userId, - fullName: newNameForSelfUser)); - } - await tester.pumpWidget(TestZulipApp( - accountId: eg.selfAccount.id, + accountId: selfAccount.id, navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [], child: const HomePage())); @@ -187,7 +184,8 @@ void main() { } Future markMessageAsRead(WidgetTester tester, Message message) async { - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final store = await testBinding.globalStore.perAccount( + testBinding.globalStore.accounts.single.id); await store.handleEvent(UpdateMessageFlagsAddEvent( id: 1, flag: MessageFlag.read, all: false, messages: [message.id])); await tester.pump(); @@ -216,18 +214,18 @@ void main() { }); testWidgets('short name takes one line', (tester) async { - final message = eg.dmMessage(from: eg.selfUser, to: []); const name = 'Short name'; - await setupPage(tester, users: [], dmMessages: [message], - newNameForSelfUser: name); + final selfUser = eg.user(fullName: name); + await setupPage(tester, selfUser: selfUser, users: [], + dmMessages: [eg.dmMessage(from: selfUser, to: [])]); checkTitle(tester, name, 1); }); testWidgets('very long name takes two lines (must be ellipsized)', (tester) async { - final message = eg.dmMessage(from: eg.selfUser, to: []); const name = 'Long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name'; - await setupPage(tester, users: [], dmMessages: [message], - newNameForSelfUser: name); + final selfUser = eg.user(fullName: name); + await setupPage(tester, selfUser: selfUser, users: [], + dmMessages: [eg.dmMessage(from: selfUser, to: [])]); checkTitle(tester, name, 2); });