Skip to content

Commit 5515540

Browse files
gnpricechrisbobbe
authored andcommitted
test: Prevent mutation of shared example users
This ensures we don't re-introduce the sort of state leak that caused #1712. In three recent commits we fixed every existing example of this sort of state leak; only three test files had them. I also looked through all the other top-level fields in this file `example_data.dart`. Most are immutable atoms like ints and strings. There are a handful of remaining points of mutability which could in principle allow this sort of state leak; so I'll tighten those up too in the next couple of commits.
1 parent 54ac1dd commit 5515540

File tree

1 file changed

+61
-7
lines changed

1 file changed

+61
-7
lines changed

test/example_data.dart

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import 'package:zulip/model/narrow.dart';
1717
import 'package:zulip/model/settings.dart';
1818
import 'package:zulip/model/store.dart';
1919

20+
import 'model/binding.dart';
2021
import 'model/test_store.dart';
2122
import 'stdlib_checks.dart';
2223

@@ -289,24 +290,77 @@ Account account({
289290
);
290291
}
291292

292-
final User selfUser = user(fullName: 'Self User');
293+
/// A [User] which throws on attempting to mutate any of its fields.
294+
///
295+
/// We use this to prevent any tests from leaking state through having a
296+
/// [PerAccountStore] (which will be discarded when [TestZulipBinding.reset]
297+
/// is called at the end of the test case) mutate a [User] in its [UserStore]
298+
/// which happens to a value in this file like [selfUser] (which will not be
299+
/// discarded by [TestZulipBinding.reset]). That was the cause of issue #1712.
300+
class _ImmutableUser extends User {
301+
_ImmutableUser.copyUser(User user) : super(
302+
// When adding a field here, be sure to add the corresponding setter below.
303+
userId: user.userId,
304+
deliveryEmail: user.deliveryEmail,
305+
email: user.email,
306+
fullName: user.fullName,
307+
dateJoined: user.dateJoined,
308+
isActive: user.isActive,
309+
isBillingAdmin: user.isBillingAdmin,
310+
isBot: user.isBot,
311+
botType: user.botType,
312+
botOwnerId: user.botOwnerId,
313+
role: user.role,
314+
timezone: user.timezone,
315+
avatarUrl: user.avatarUrl,
316+
avatarVersion: user.avatarVersion,
317+
profileData: user.profileData == null ? null : Map.unmodifiable(user.profileData!),
318+
isSystemBot: user.isSystemBot,
319+
// When adding a field here, be sure to add the corresponding setter below.
320+
);
321+
322+
static final Error _error = UnsupportedError(
323+
'Cannot mutate immutable User.\n'
324+
'When a test needs to have the store handle an event which will\n'
325+
'modify a user, use `eg.user()` to make a fresh User object\n'
326+
'instead of using a shared User object like `eg.selfUser`.');
327+
328+
// userId already immutable
329+
@override set deliveryEmail(_) => throw _error;
330+
@override set email(_) => throw _error;
331+
@override set fullName(_) => throw _error;
332+
// dateJoined already immutable
333+
@override set isActive(_) => throw _error;
334+
@override set isBillingAdmin(_) => throw _error;
335+
// isBot already immutable
336+
// botType already immutable
337+
@override set botOwnerId(_) => throw _error;
338+
@override set role(_) => throw _error;
339+
@override set timezone(_) => throw _error;
340+
@override set avatarUrl(_) => throw _error;
341+
@override set avatarVersion(_) => throw _error;
342+
@override set profileData(_) => throw _error;
343+
// isSystemBot already immutable
344+
}
345+
346+
final User selfUser = _ImmutableUser.copyUser(user(fullName: 'Self User'));
347+
final User otherUser = _ImmutableUser.copyUser(user(fullName: 'Other User'));
348+
final User thirdUser = _ImmutableUser.copyUser(user(fullName: 'Third User'));
349+
final User fourthUser = _ImmutableUser.copyUser(user(fullName: 'Fourth User'));
350+
351+
// There's no need for an [Account] analogue of [_ImmutableUser],
352+
// because [Account] (which is generated by Drift) is already immutable.
293353
final Account selfAccount = account(
294354
id: 1001,
295355
user: selfUser,
296356
apiKey: 'dQcEJWTq3LczosDkJnRTwf31zniGvMrO', // A Zulip API key is 32 digits of base64.
297357
);
298-
299-
final User otherUser = user(fullName: 'Other User');
300358
final Account otherAccount = account(
301359
id: 1002,
302360
user: otherUser,
303361
apiKey: '6dxT4b73BYpCTU+i4BB9LAKC5h/CufqY', // A Zulip API key is 32 digits of base64.
304362
);
305363

306-
final User thirdUser = user(fullName: 'Third User');
307-
308-
final User fourthUser = user(fullName: 'Fourth User');
309-
310364
////////////////////////////////////////////////////////////////
311365
// Data attached to the self-account on the realm
312366
//

0 commit comments

Comments
 (0)