Skip to content

test: Prevent mutating shared example users or other data #1714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 22, 2025
Merged

test: Prevent mutating shared example users or other data #1714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
427e541
action_sheet test: Avoid mutating shared example users
gnprice Jul 18, 2025
d1eaacb
recent-dms test: Avoid mutating eg.selfUser
gnprice Jul 18, 2025
19439b3
test: Prevent mutation of shared example users
gnprice Jul 18, 2025
d8d19c8
test [nfc]: Make all example-data globals final
gnprice Jul 18, 2025
edf1eee
test: Prevent mutating shared example ServerEmojiData values
gnprice Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 73 additions & 12 deletions test/example_data.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -130,7 +131,14 @@ GetServerSettingsResult serverSettings({
);
}

ServerEmojiData serverEmojiDataPopular = ServerEmojiData(codeToNames: {
ServerEmojiData _immutableServerEmojiData({
required Map<String, List<String>> codeToNames}) {
return ServerEmojiData(
codeToNames: Map.unmodifiable(codeToNames.map(
(k, v) => MapEntry(k, List<String>.unmodifiable(v)))));
}

final ServerEmojiData serverEmojiDataPopular = _immutableServerEmojiData(codeToNames: {
'1f44d': ['+1', 'thumbs_up', 'like'],
'1f389': ['tada'],
'1f642': ['slight_smile'],
Expand All @@ -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'],
Expand Down Expand Up @@ -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
//
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 14 additions & 8 deletions test/widgets/action_sheet_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ late FakeApiConnection connection;
Future<void> setupToMessageActionSheet(WidgetTester tester, {
required Message message,
required Narrow narrow,
User? selfUser,
User? sender,
List<int>? mutedUserIds,
bool? realmAllowMessageEditing,
Expand All @@ -67,15 +68,17 @@ Future<void> 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)),
Expand All @@ -97,7 +100,7 @@ Future<void> 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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand All @@ -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
Expand Down
32 changes: 15 additions & 17 deletions test/widgets/recent_dm_conversations_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,18 @@ import 'test_app.dart';
Future<void> setupPage(WidgetTester tester, {
required List<DmMessage> dmMessages,
required List<User> users,
User? selfUser,
List<int>? 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);
}
Expand All @@ -46,13 +48,8 @@ Future<void> 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()));

Expand Down Expand Up @@ -187,7 +184,8 @@ void main() {
}

Future<void> 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();
Expand Down Expand Up @@ -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);
});

Expand Down