Skip to content

Commit 313f8a9

Browse files
committed
unreads test [nfc]: Accept that a whole PerAccountStore is used for tests
We've had a couple of approaches we've experimented with for how to write the tests for a given substore within our data model: * Operate on the given substore's individual API. * Operate on the API of the overall PerAccountStore, though focusing on the particular portion of that API provided by the given substore. In general I think the outcome of these experiments is that we want to be writing our tests against the API of the whole PerAccountStore. Fundamentally that's for the reasons described here: https://zulip.readthedocs.io/en/latest/testing/philosophy.html#integration-testing-or-unit-testing because the overall PerAccountStore is the layer whose API corresponds naturally to the Zulip server API, which is an interface that's external to the app and so is the real interface we're committed to supporting. This particular test file is one which we originally wrote on the other side of the experiment, operating on the individual `Unreads` substore. But it already had some tension with that approach, ever since we realized that that substore needed access to another substore, the ChannelStore, in order to properly handle muting (ffa2d32, 0d03c8e). To handle that, these tests ended up making a whole PerAccountStore after all, and had a TODO comment for trying to fully make the substore-only approach work. With the advent of CorePerAccountStore, there's no longer a realistic prospect that we'd want to end up carrying that through. That's because the Unreads now gets its value of `selfUserId` from a CorePerAccountStore, and the only reasonable way to construct one of those is as part of a whole PerAccountStore. So resolve the experiment, at least as to this file: remove the TODO comment, and rename the variable to just `store` accordingly.
1 parent 706af43 commit 313f8a9

File tree

1 file changed

+21
-23
lines changed

1 file changed

+21
-23
lines changed

Diff for: test/model/unreads_test.dart

+21-23
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ void main() {
1616
// These variables are the common state operated on by each test.
1717
// Each test case calls [prepare] to initialize them.
1818
late Unreads model;
19-
late PerAccountStore channelStore; // TODO reduce this to ChannelStore
19+
late PerAccountStore store;
2020
late int notifiedCount;
2121

2222
void checkNotified({required int count}) {
@@ -37,9 +37,7 @@ void main() {
3737
oldUnreadsMissing: false,
3838
),
3939
}) {
40-
final store = eg.store(
41-
initialSnapshot: eg.initialSnapshot(unreadMsgs: initial));
42-
channelStore = store;
40+
store = eg.store(initialSnapshot: eg.initialSnapshot(unreadMsgs: initial));
4341
notifiedCount = 0;
4442
model = store.unreads
4543
..addListener(() {
@@ -158,11 +156,11 @@ void main() {
158156
final stream2 = eg.stream();
159157
final stream3 = eg.stream();
160158
prepare();
161-
await channelStore.addStreams([stream1, stream2, stream3]);
162-
await channelStore.addSubscription(eg.subscription(stream1));
163-
await channelStore.addSubscription(eg.subscription(stream2));
164-
await channelStore.addSubscription(eg.subscription(stream3, isMuted: true));
165-
await channelStore.addUserTopic(stream1, 'a', UserTopicVisibilityPolicy.muted);
159+
await store.addStreams([stream1, stream2, stream3]);
160+
await store.addSubscription(eg.subscription(stream1));
161+
await store.addSubscription(eg.subscription(stream2));
162+
await store.addSubscription(eg.subscription(stream3, isMuted: true));
163+
await store.addUserTopic(stream1, 'a', UserTopicVisibilityPolicy.muted);
166164
fillWithMessages([
167165
eg.streamMessage(stream: stream1, topic: 'a', flags: []),
168166
eg.streamMessage(stream: stream1, topic: 'b', flags: []),
@@ -178,10 +176,10 @@ void main() {
178176
test('countInChannel/Narrow', () async {
179177
final stream = eg.stream();
180178
prepare();
181-
await channelStore.addStream(stream);
182-
await channelStore.addSubscription(eg.subscription(stream));
183-
await channelStore.addUserTopic(stream, 'a', UserTopicVisibilityPolicy.unmuted);
184-
await channelStore.addUserTopic(stream, 'c', UserTopicVisibilityPolicy.muted);
179+
await store.addStream(stream);
180+
await store.addSubscription(eg.subscription(stream));
181+
await store.addUserTopic(stream, 'a', UserTopicVisibilityPolicy.unmuted);
182+
await store.addUserTopic(stream, 'c', UserTopicVisibilityPolicy.muted);
185183
fillWithMessages([
186184
eg.streamMessage(stream: stream, topic: 'a', flags: []),
187185
eg.streamMessage(stream: stream, topic: 'a', flags: []),
@@ -193,7 +191,7 @@ void main() {
193191
check(model.countInChannel (stream.streamId)).equals(5);
194192
check(model.countInChannelNarrow(stream.streamId)).equals(5);
195193

196-
await channelStore.handleEvent(SubscriptionUpdateEvent(id: 1,
194+
await store.handleEvent(SubscriptionUpdateEvent(id: 1,
197195
streamId: stream.streamId,
198196
property: SubscriptionProperty.isMuted, value: true));
199197
check(model.countInChannel (stream.streamId)).equals(2);
@@ -220,7 +218,7 @@ void main() {
220218
test('countInMentionsNarrow', () async {
221219
final stream = eg.stream();
222220
prepare();
223-
await channelStore.addStream(stream);
221+
await store.addStream(stream);
224222
fillWithMessages([
225223
eg.streamMessage(stream: stream, flags: []),
226224
eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]),
@@ -232,7 +230,7 @@ void main() {
232230
test('countInStarredMessagesNarrow', () async {
233231
final stream = eg.stream();
234232
prepare();
235-
await channelStore.addStream(stream);
233+
await store.addStream(stream);
236234
fillWithMessages([
237235
eg.streamMessage(stream: stream, flags: []),
238236
eg.streamMessage(stream: stream, flags: [MessageFlag.starred]),
@@ -481,8 +479,8 @@ void main() {
481479

482480
Future<void> prepareStore() async {
483481
prepare();
484-
await channelStore.addStream(origChannel);
485-
await channelStore.addSubscription(eg.subscription(origChannel));
482+
await store.addStream(origChannel);
483+
await store.addSubscription(eg.subscription(origChannel));
486484
readMessages = List<StreamMessage>.generate(10,
487485
(_) => eg.streamMessage(stream: origChannel, topic: origTopic,
488486
flags: [MessageFlag.read]));
@@ -505,8 +503,8 @@ void main() {
505503
test('moved messages = unread messages', () async {
506504
await prepareStore();
507505
final newChannel = eg.stream();
508-
await channelStore.addStream(newChannel);
509-
await channelStore.addSubscription(eg.subscription(newChannel));
506+
await store.addStream(newChannel);
507+
await store.addSubscription(eg.subscription(newChannel));
510508
fillWithMessages(unreadMessages);
511509
final originalMessageIds =
512510
model.streams[origChannel.streamId]![TopicName(origTopic)]!;
@@ -587,8 +585,8 @@ void main() {
587585
test('moving to unsubscribed channels drops the unreads', () async {
588586
await prepareStore();
589587
final unsubscribedChannel = eg.stream();
590-
await channelStore.addStream(unsubscribedChannel);
591-
assert(!channelStore.subscriptions.containsKey(
588+
await store.addStream(unsubscribedChannel);
589+
assert(!store.subscriptions.containsKey(
592590
unsubscribedChannel.streamId));
593591
fillWithMessages(unreadMessages);
594592

@@ -618,7 +616,7 @@ void main() {
618616
fillWithMessages(unreadMessages);
619617

620618
final unknownChannel = eg.stream();
621-
assert(!channelStore.streams.containsKey(unknownChannel.streamId));
619+
assert(!store.streams.containsKey(unknownChannel.streamId));
622620
final unknownUnreadMessage = eg.streamMessage(
623621
stream: unknownChannel, topic: origTopic);
624622

0 commit comments

Comments
 (0)