Skip to content

Commit f6e90e0

Browse files
gnpricechrisbobbe
authored andcommitted
push_device: Test auto-register-token end to end, dropping ad-hoc Future
Now these tests invoke this code the same way it gets invoked in production. Instead of directly invoking a registerToken method, they use a debug hook that can only pause and resume a control path which has already been started.
1 parent 5d1358b commit f6e90e0

File tree

4 files changed

+58
-31
lines changed

4 files changed

+58
-31
lines changed

lib/model/push_device.dart

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:async';
2+
13
import '../notifications/receive.dart';
24
import 'store.dart';
35

@@ -6,10 +8,7 @@ import 'store.dart';
68
// TODO(#1764) do that tracking of responses
79
class PushDeviceManager extends PerAccountStoreBase {
810
PushDeviceManager({required super.core}) {
9-
if (!debugAutoRegisterToken) {
10-
return;
11-
}
12-
registerToken();
11+
_registerTokenAndSubscribe();
1312
}
1413

1514
bool _disposed = false;
@@ -25,15 +24,18 @@ class PushDeviceManager extends PerAccountStoreBase {
2524
}
2625

2726
/// Send this client's notification token to the server, now and if it changes.
28-
///
29-
/// TODO The returned future isn't especially meaningful (it may or may not
30-
/// mean we actually sent the token). Make it just `void` once we fix the
31-
/// one test that relies on the future.
3227
// TODO(#322) save acked token, to dedupe updating it on the server
3328
// TODO(#323) track the addFcmToken/etc request, warn if not succeeding
34-
Future<void> registerToken() async {
29+
void _registerTokenAndSubscribe() async {
30+
_debugMaybePause();
31+
if (_debugRegisterTokenProceed != null) {
32+
await _debugRegisterTokenProceed!.future;
33+
}
34+
3535
NotificationService.instance.token.addListener(_registerToken);
3636
await _registerToken();
37+
38+
_debugRegisterTokenCompleted?.complete();
3739
}
3840

3941
Future<void> _registerToken() async {
@@ -42,22 +44,49 @@ class PushDeviceManager extends PerAccountStoreBase {
4244
await NotificationService.instance.registerToken(connection);
4345
}
4446

45-
/// In debug mode, controls whether [registerToken] should be called
46-
/// immediately in the constructor.
47+
Completer<void>? _debugRegisterTokenProceed;
48+
Completer<void>? _debugRegisterTokenCompleted;
49+
50+
void _debugMaybePause() {
51+
assert(() {
52+
if (debugAutoPause) {
53+
_debugRegisterTokenProceed = Completer();
54+
_debugRegisterTokenCompleted = Completer();
55+
}
56+
return true;
57+
}());
58+
}
59+
60+
/// Unpause registering the token (after [debugAutoPause]),
61+
/// returning a future that completes when any immediate request is completed.
62+
///
63+
/// This has no effect if [debugAutoPause] was false
64+
/// when this instance was constructed,
65+
/// and therefore no effect outside of debug mode.
66+
Future<void> debugUnpauseRegisterToken() async {
67+
_debugRegisterTokenProceed!.complete();
68+
await _debugRegisterTokenCompleted!.future;
69+
}
70+
71+
/// In debug mode, controls whether new instances should pause
72+
/// before registering the token with the server.
73+
///
74+
/// When paused, token registration can be unpaused
75+
/// with [debugUnpauseRegisterToken].
4776
///
48-
/// Outside of debug mode, this is always true and the setter has no effect.
49-
static bool get debugAutoRegisterToken {
50-
bool result = true;
77+
/// Outside of debug mode, this is always false and the setter has no effect.
78+
static bool get debugAutoPause {
79+
bool result = false;
5180
assert(() {
52-
result = _debugAutoRegisterToken;
81+
result = _debugAutoPause;
5382
return true;
5483
}());
5584
return result;
5685
}
57-
static bool _debugAutoRegisterToken = true;
58-
static set debugAutoRegisterToken(bool value) {
86+
static bool _debugAutoPause = false;
87+
static set debugAutoPause(bool value) {
5988
assert(() {
60-
_debugAutoRegisterToken = value;
89+
_debugAutoPause = value;
6190
return true;
6291
}());
6392
}

test/model/actions_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ void main() {
148148
}));
149149

150150
test('notifications are removed after logout', () => awaitFakeAsync((async) async {
151-
PushDeviceManager.debugAutoRegisterToken = false;
152-
addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
151+
PushDeviceManager.debugAutoPause = true;
152+
addTearDown(() => PushDeviceManager.debugAutoPause = false);
153153
await prepare();
154154
testBinding.firebaseMessagingInitialToken = '123';
155155
addTearDown(NotificationService.debugReset);

test/model/push_device_test.dart

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,13 @@ import '../stdlib_checks.dart';
1515
void main() {
1616
TestZulipBinding.ensureInitialized();
1717

18-
group('registerToken', () {
19-
// TODO test registerToken gets called on constructing an instance
20-
18+
group('register token', () {
2119
late PushDeviceManager model;
2220
late FakeApiConnection connection;
2321

2422
void prepareStore() {
25-
PushDeviceManager.debugAutoRegisterToken = false;
26-
addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
23+
PushDeviceManager.debugAutoPause = true;
24+
addTearDown(() => PushDeviceManager.debugAutoPause = false);
2725
final store = eg.store();
2826
model = store.pushDevices;
2927
connection = store.connection as FakeApiConnection;
@@ -56,7 +54,7 @@ void main() {
5654
// On store startup, send the token.
5755
prepareStore();
5856
connection.prepare(json: {});
59-
await model.registerToken();
57+
await model.debugUnpauseRegisterToken();
6058
if (defaultTargetPlatform == TargetPlatform.android) {
6159
checkLastRequestFcm(token: '012abc');
6260
} else {
@@ -84,14 +82,14 @@ void main() {
8482
// TODO this test is a bit brittle in its interaction with asynchrony;
8583
// to fix, probably extend TestZulipBinding to control when getToken finishes.
8684
//
87-
// The aim here is to first wait for `model.registerToken`
85+
// The aim here is to first wait for `model.debugUnpauseRegisterToken`
8886
// to complete whatever it's going to do; then check no request was made;
8987
// and only after that wait for `NotificationService.start` to finish,
9088
// including its `getToken` call.
9189

9290
// On store startup, send nothing (because we have nothing to send).
9391
prepareStore();
94-
await model.registerToken();
92+
await model.debugUnpauseRegisterToken();
9593
check(connection.lastRequest).isNull();
9694

9795
// When the token later appears, send it.
@@ -125,7 +123,7 @@ void main() {
125123

126124
prepareStore();
127125
connection.prepare(json: {});
128-
await model.registerToken();
126+
await model.debugUnpauseRegisterToken();
129127
checkLastRequestApns(token: '012abc', appid: 'com.example.test');
130128
}));
131129
});

test/notifications/open_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ void main() {
8181
testBinding.firebaseMessagingInitialToken = '012abc';
8282
addTearDown(NotificationService.debugReset);
8383
NotificationService.debugBackgroundIsolateIsLive = false;
84-
PushDeviceManager.debugAutoRegisterToken = false;
85-
addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
84+
PushDeviceManager.debugAutoPause = true;
85+
addTearDown(() => PushDeviceManager.debugAutoPause = false);
8686
await NotificationService.instance.start();
8787
}
8888

0 commit comments

Comments
 (0)