Skip to content

test: make current time testable #1363

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import '../log.dart';
import '../notifications/receive.dart';
import 'actions.dart';
import 'autocomplete.dart';
import 'binding.dart';
import 'database.dart';
import 'emoji.dart';
import 'localizations.dart';
Expand Down Expand Up @@ -678,7 +679,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
///
/// To determine if a user is a full member, callers must also check that the
/// user's role is at least [UserRole.member].
bool hasPassedWaitingPeriod(User user, {required DateTime byDate}) {
bool hasPassedWaitingPeriod(User user) {
// [User.dateJoined] is in UTC. For logged-in users, the format is:
// YYYY-MM-DDTHH:mm+00:00, which includes the timezone offset for UTC.
// For logged-out spectators, the format is: YYYY-MM-DD, which doesn't
Expand All @@ -690,7 +691,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
// See the related discussion:
// https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/provide.20an.20explicit.20format.20for.20.60realm_user.2Edate_joined.60/near/1980194
final dateJoined = DateTime.parse(user.dateJoined);
return byDate.difference(dateJoined).inDays >= realmWaitingPeriodThreshold;
final now = ZulipBinding.instance.utcNow();
return now.difference(dateJoined).inDays >= realmWaitingPeriodThreshold;
}

/// The given user's real email address, if known, for displaying in the UI.
Expand Down Expand Up @@ -741,7 +743,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
bool hasPostingPermission({
required ZulipStream inChannel,
required User user,
required DateTime byDate,
}) {
final role = user.role;
// We let the users with [unknown] role to send the message, then the server
Expand All @@ -753,7 +754,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
case ChannelPostPolicy.fullMembers: {
if (!role.isAtLeast(UserRole.member)) return false;
return role == UserRole.member
? hasPassedWaitingPeriod(user, byDate: byDate)
? hasPassedWaitingPeriod(user)
: true;
}
case ChannelPostPolicy.moderators: return role.isAtLeast(UserRole.moderator);
Expand Down
3 changes: 1 addition & 2 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2064,8 +2064,7 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
case ChannelNarrow(:final streamId):
case TopicNarrow(:final streamId):
final channel = store.streams[streamId];
if (channel == null || !store.hasPostingPermission(inChannel: channel,
user: store.selfUser, byDate: DateTime.now())) {
if (channel == null || !store.hasPostingPermission(inChannel: channel, user: store.selfUser)) {
return _ErrorBanner(getLabel: (zulipLocalizations) =>
zulipLocalizations.errorBannerCannotPostInChannelLabel);
}
Expand Down
15 changes: 8 additions & 7 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import '../api/model/model.dart';
import '../generated/l10n/zulip_localizations.dart';
import '../model/database.dart';
import '../model/message.dart';
import '../model/binding.dart';
import '../model/message_list.dart';
import '../model/narrow.dart';
import '../model/store.dart';
Expand Down Expand Up @@ -1631,19 +1632,19 @@ class DateText extends StatelessWidget {
),
formatHeaderDate(
zulipLocalizations,
DateTime.fromMillisecondsSinceEpoch(timestamp * 1000),
now: DateTime.now()));
DateTime.fromMillisecondsSinceEpoch(timestamp * 1000)));
}
}

@visibleForTesting
String formatHeaderDate(
ZulipLocalizations zulipLocalizations,
DateTime dateTime, {
required DateTime now,
}) {
assert(!dateTime.isUtc && !now.isUtc,
'`dateTime` and `now` need to be in local time.');
DateTime dateTime,
) {
assert(!dateTime.isUtc,
'`dateTime` need to be in local time.');

final now = ZulipBinding.instance.utcNow().toLocal();

if (dateTime.year == now.year &&
dateTime.month == now.month &&
Expand Down
42 changes: 23 additions & 19 deletions test/model/store_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'dart:async';
import 'dart:io';

import 'package:checks/checks.dart';
import 'package:clock/clock.dart';
import 'package:fake_async/fake_async.dart';
import 'package:flutter/foundation.dart';
import 'package:http/http.dart' as http;
Expand Down Expand Up @@ -480,9 +481,11 @@ void main() {
for (final (String dateJoined, DateTime currentDate, bool hasPassedWaitingPeriod) in testCases) {
test('user joined at $dateJoined ${hasPassedWaitingPeriod ? 'has' : "hasn't"} '
'passed waiting period by $currentDate', () {
final user = eg.user(dateJoined: dateJoined);
check(store.hasPassedWaitingPeriod(user, byDate: currentDate))
.equals(hasPassedWaitingPeriod);
withClock(Clock.fixed(currentDate), () {
final user = eg.user(dateJoined: dateJoined);
check(store.hasPassedWaitingPeriod(user))
.equals(hasPassedWaitingPeriod);
});
});
}
});
Expand Down Expand Up @@ -526,11 +529,10 @@ void main() {
test('"${role.name}" user ${canPost ? 'can' : "can't"} post in channel '
'with "${policy.name}" policy', () {
final store = eg.store();
// we don't use `withClock` because current time is not actually relevant for
// these test cases; for the ones which it is, they're practiced below.
final actual = store.hasPostingPermission(
inChannel: eg.stream(channelPostPolicy: policy), user: eg.user(role: role),
// [byDate] is not actually relevant for these test cases; for the
// ones which it is, they're practiced below.
byDate: DateTime.now());
inChannel: eg.stream(channelPostPolicy: policy), user: eg.user(role: role));
check(actual).equals(canPost);
});
}
Expand All @@ -544,21 +546,23 @@ void main() {
role: UserRole.member, dateJoined: dateJoined);

test('a "full" member -> can post in the channel', () {
final store = localStore(realmWaitingPeriodThreshold: 3);
final hasPermission = store.hasPostingPermission(
inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers),
user: memberUser(dateJoined: '2024-11-25T10:00+00:00'),
byDate: DateTime.utc(2024, 11, 28, 10, 00));
check(hasPermission).isTrue();
withClock(Clock.fixed(DateTime.utc(2024, 11, 28, 10, 00)), () {
final store = localStore(realmWaitingPeriodThreshold: 3);
final hasPermission = store.hasPostingPermission(
inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers),
user: memberUser(dateJoined: '2024-11-25T10:00+00:00'));
check(hasPermission).isTrue();
});
});

test('not a "full" member -> cannot post in the channel', () {
final store = localStore(realmWaitingPeriodThreshold: 3);
final actual = store.hasPostingPermission(
inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers),
user: memberUser(dateJoined: '2024-11-25T10:00+00:00'),
byDate: DateTime.utc(2024, 11, 28, 09, 59));
check(actual).isFalse();
withClock(Clock.fixed(DateTime.utc(2024, 11, 28, 09, 59)), () {
final store = localStore(realmWaitingPeriodThreshold: 3);
final actual = store.hasPostingPermission(
inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers),
user: memberUser(dateJoined: '2024-11-25T10:00+00:00'));
check(actual).isFalse();
});
});
});
});
Expand Down
8 changes: 5 additions & 3 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'dart:convert';
import 'dart:io';

import 'package:checks/checks.dart';
import 'package:clock/clock.dart';
import 'package:collection/collection.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
Expand Down Expand Up @@ -1498,7 +1499,6 @@ void main() {

group('formatHeaderDate', () {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
final now = DateTime.parse("2023-01-10 12:00");
final testCases = [
("2023-01-10 12:00", zulipLocalizations.today),
("2023-01-10 00:00", zulipLocalizations.today),
Expand All @@ -1513,8 +1513,10 @@ void main() {
];
for (final (dateTime, expected) in testCases) {
test('$dateTime returns $expected', () {
check(formatHeaderDate(zulipLocalizations, DateTime.parse(dateTime), now: now))
.equals(expected);
withClock(Clock.fixed(DateTime.parse("2023-01-10 12:00")), () {
check(formatHeaderDate(zulipLocalizations, DateTime.parse(dateTime)))
.equals(expected);
});
});
}
});
Expand Down