From cef497a935f04c8c812b349000832ab31962dd25 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 14:51:39 -0700 Subject: [PATCH 01/15] msglist test [nfc]: Sort imports as usual --- test/model/message_list_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index beebc09c86..89ba7e7356 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -9,9 +9,9 @@ import 'package:zulip/model/store.dart'; import '../api/fake_api.dart'; import '../api/model/model_checks.dart'; +import '../example_data.dart' as eg; import '../model/binding.dart'; import '../model/test_store.dart'; -import '../example_data.dart' as eg; const int userId = 1; From 0a2569a1c667107e237cfdeec0c62ee59b61c13c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 14:53:20 -0700 Subject: [PATCH 02/15] msglist test: Cut redundant length check in setup; tighten If the fetch somehow didn't get the right number of messages, most of the subsequent tests couldn't pass anyway. --- test/model/message_list_test.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 89ba7e7356..6658bf46e8 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -31,7 +31,6 @@ Future messageListViewWithMessages(List messages, PerA final messageList = MessageListView.init(store: store, narrow: narrow); final connection = store.connection as FakeApiConnection; - connection.prepare(json: GetMessagesResult( anchor: messages.first.id, foundNewest: true, @@ -40,11 +39,8 @@ Future messageListViewWithMessages(List messages, PerA historyLimited: false, messages: messages, ).toJson()); - await messageList.fetch(); - check(messageList.messages.length).equals(messages.length); - return messageList; } From 2ee3ead86d45e4180852cb6431020dfe53512ba1 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 14:56:36 -0700 Subject: [PATCH 03/15] msglist test: Tighten findMessageWithId test --- test/model/message_list_test.dart | 42 +++++++++++++------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 6658bf46e8..d0c30350fb 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -50,32 +50,24 @@ void main() async { final stream = eg.stream(); final narrow = StreamNarrow(stream.streamId); - group('update message tests', () { - - test('find message in message list returns index of message', () async { - final store = await setupStore(stream); - - final m1 = eg.streamMessage(id: 2, stream: stream); - final m2 = eg.streamMessage(id: 4, stream: stream); - final m3 = eg.streamMessage(id: 6, stream: stream); - - final messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow); - // The implementation of this uses a binary search, so let's test it - // a bit more exhaustively. - - check(messageList.findMessageWithId(1)).equals(-1); - check(messageList.findMessageWithId(2)).equals(0); - check(messageList.findMessageWithId(3)).equals(-1); - check(messageList.findMessageWithId(4)).equals(1); - check(messageList.findMessageWithId(5)).equals(-1); - check(messageList.findMessageWithId(6)).equals(2); - check(messageList.findMessageWithId(7)).equals(-1); - - // Invalid IDs - check(messageList.findMessageWithId(-8409)).equals(-1); - check(messageList.findMessageWithId(0)).equals(-1); - }); + test('findMessageWithId', () async { + final store = await setupStore(stream); + final m1 = eg.streamMessage(id: 2, stream: stream); + final m2 = eg.streamMessage(id: 4, stream: stream); + final m3 = eg.streamMessage(id: 6, stream: stream); + final messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow); + + // Exercise the binary search before, at, and after each element of the list. + check(messageList.findMessageWithId(1)).equals(-1); + check(messageList.findMessageWithId(2)).equals(0); + check(messageList.findMessageWithId(3)).equals(-1); + check(messageList.findMessageWithId(4)).equals(1); + check(messageList.findMessageWithId(5)).equals(-1); + check(messageList.findMessageWithId(6)).equals(2); + check(messageList.findMessageWithId(7)).equals(-1); + }); + group('update message tests', () { test('update events are correctly applied to message when it is in the stream', () async { final store = await setupStore(stream); From 6d31091b0ac2a309e746648f3c168474497bf1d6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 14:58:52 -0700 Subject: [PATCH 04/15] msglist test [nfc]: Tighten test names; use method's name --- test/model/message_list_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index d0c30350fb..ca76543be8 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -67,8 +67,8 @@ void main() async { check(messageList.findMessageWithId(7)).equals(-1); }); - group('update message tests', () { - test('update events are correctly applied to message when it is in the stream', () async { + group('maybeUpdateMessage', () { + test('update a message', () async { final store = await setupStore(stream); const oldContent = "

Hello, world

"; @@ -114,7 +114,7 @@ void main() async { ..isMeMessage.isTrue(); }); - test('update event is ignored when message is not in the message list', () async { + test('ignore when message not present', () async { final store = await setupStore(stream); const oldContent = "

Hello, world

"; From 82fa36f327b91ef1c8dfa648badaa88d58e61ec3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:02:08 -0700 Subject: [PATCH 05/15] msglist test [nfc]: Tighten listenersNotified slightly; use type The variable and the listener that updates it are naturally closely related, so group them together. Then group the subsequent maybeUpdateMessage call together with the checks that inspect that call's behavior. --- test/model/message_list_test.dart | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index ca76543be8..fb2bbe3380 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -99,13 +99,11 @@ void main() async { ..flags.deepEquals(oldFlags) ..isMeMessage.isFalse(); - var listenersNotified = false; - + bool listenersNotified = false; messageList.addListener(() { listenersNotified = true; }); - messageList.maybeUpdateMessage(updateEvent); + messageList.maybeUpdateMessage(updateEvent); check(listenersNotified).isTrue(); - check(message) ..identicalTo(messageList.messages.single) ..content.equals(newContent) @@ -138,11 +136,10 @@ void main() async { final message = messageList.messages.single; check(message).content.equals(oldContent); - var listenersNotified = false; - + bool listenersNotified = false; messageList.addListener(() { listenersNotified = true; }); - messageList.maybeUpdateMessage(updateEvent); + messageList.maybeUpdateMessage(updateEvent); check(listenersNotified).isFalse(); check(message).content.equals(oldContent); }); From c30d5f9b384c3537f89631fe12dc05f77b253666 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:05:54 -0700 Subject: [PATCH 06/15] msglist test [nfc]: Put identicalTo check into logical order The subject, or "actual", we're inspecting is the Message that one finds in the data structure. The thing we're checking about it is that it is the same object as the Message we found there before. --- test/model/message_list_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index fb2bbe3380..5f7c753ed2 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -104,8 +104,8 @@ void main() async { messageList.maybeUpdateMessage(updateEvent); check(listenersNotified).isTrue(); - check(message) - ..identicalTo(messageList.messages.single) + check(messageList.messages.single) + ..identicalTo(message) ..content.equals(newContent) ..lastEditTimestamp.equals(newTimestamp) ..flags.equals(newFlags) From 8c4e952b087ed6a6e02fff31cefb3d89da96a2cd Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:14:11 -0700 Subject: [PATCH 07/15] msglist test [nfc]: Make "before" checks be more to the point This test is for `maybeUpdateMessage`, not `fetch`. So it's not its job to check that `fetch` successfully got the content and flags and isMeMessage that we expected. What is useful for a "before" check here, on the other hand, is to confirm that our "after" checks aren't going to be vacuous, by checking that the message doesn't already have the values we'll be looking for there. So check that instead. Include one for lastEditTimestamp, which was previously left out of the "before". A low-tech way to express this would be to keep the checks like `.equals(oldContent)` and add assertions that oldContent differed from newContent, and so on. But we can do it a bit more neatly using the `not` check. --- test/model/message_list_test.dart | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 5f7c753ed2..30d4e25c92 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -71,23 +71,22 @@ void main() async { test('update a message', () async { final store = await setupStore(stream); - const oldContent = "

Hello, world

"; - const newContent = "

Hello, edited

"; - const newTimestamp = 99999; - - List oldFlags = []; - List newFlags = ["starred"]; - - final originalMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent, flags: oldFlags); + final originalMessage = eg.streamMessage(id: 243, stream: stream, + content: "

Hello, world

", + flags: [], + ); final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + final newFlags = ["starred"]; + const newContent = "

Hello, edited

"; + const editTimestamp = 99999; final updateEvent = UpdateMessageEvent( id: 1, messageId: originalMessage.id, messageIds: [originalMessage.id], flags: newFlags, renderedContent: newContent, - editTimestamp: newTimestamp, + editTimestamp: editTimestamp, isMeMessage: true, userId: userId, renderingOnly: false, @@ -95,8 +94,9 @@ void main() async { final message = messageList.messages.single; check(message) - ..content.equals(oldContent) - ..flags.deepEquals(oldFlags) + ..content.not(it()..equals(newContent)) + ..lastEditTimestamp.isNull() + ..flags.not(it()..deepEquals(newFlags)) ..isMeMessage.isFalse(); bool listenersNotified = false; @@ -107,7 +107,7 @@ void main() async { check(messageList.messages.single) ..identicalTo(message) ..content.equals(newContent) - ..lastEditTimestamp.equals(newTimestamp) + ..lastEditTimestamp.equals(editTimestamp) ..flags.equals(newFlags) ..isMeMessage.isTrue(); }); From fc03c596a24695ef6baf8c861f895038fde0b6aa Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:26:16 -0700 Subject: [PATCH 08/15] msglist test [nfc]: Make both "before" and "after" checks directly on point The thing we're checking in "after" is that the message has been updated to reflect the values in the update event. So let's express that directly. This also lets us do away entirely with auxiliary names for various pieces of the update event's data. --- test/model/message_list_test.dart | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 30d4e25c92..925ee97183 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -77,16 +77,13 @@ void main() async { ); final messageList = await messageListViewWithMessages([originalMessage], store, narrow); - final newFlags = ["starred"]; - const newContent = "

Hello, edited

"; - const editTimestamp = 99999; final updateEvent = UpdateMessageEvent( id: 1, messageId: originalMessage.id, messageIds: [originalMessage.id], - flags: newFlags, - renderedContent: newContent, - editTimestamp: editTimestamp, + flags: ["starred"], + renderedContent: "

Hello, edited

", + editTimestamp: 99999, isMeMessage: true, userId: userId, renderingOnly: false, @@ -94,10 +91,10 @@ void main() async { final message = messageList.messages.single; check(message) - ..content.not(it()..equals(newContent)) + ..content.not(it()..equals(updateEvent.renderedContent!)) ..lastEditTimestamp.isNull() - ..flags.not(it()..deepEquals(newFlags)) - ..isMeMessage.isFalse(); + ..flags.not(it()..deepEquals(updateEvent.flags)) + ..isMeMessage.not(it()..equals(updateEvent.isMeMessage!)); bool listenersNotified = false; messageList.addListener(() { listenersNotified = true; }); @@ -106,10 +103,10 @@ void main() async { check(listenersNotified).isTrue(); check(messageList.messages.single) ..identicalTo(message) - ..content.equals(newContent) - ..lastEditTimestamp.equals(editTimestamp) - ..flags.equals(newFlags) - ..isMeMessage.isTrue(); + ..content.equals(updateEvent.renderedContent!) + ..lastEditTimestamp.equals(updateEvent.editTimestamp) + ..flags.equals(updateEvent.flags) + ..isMeMessage.equals(updateEvent.isMeMessage!); }); test('ignore when message not present', () async { From 1813c37fc1ddb65d989d085008a56d7a365e4c98 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:43:54 -0700 Subject: [PATCH 09/15] msglist test: Similarly tighten message-not-present test --- test/model/message_list_test.dart | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 925ee97183..f4ed882345 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -112,11 +112,8 @@ void main() async { test('ignore when message not present', () async { final store = await setupStore(stream); - const oldContent = "

Hello, world

"; - const newContent = "

Hello, edited

"; - const newTimestamp = 99999; - - final originalMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent); + final originalMessage = eg.streamMessage(id: 243, stream: stream, + content: "

Hello, world

"); final messageList = await messageListViewWithMessages([originalMessage], store, narrow); final updateEvent = UpdateMessageEvent( @@ -124,21 +121,20 @@ void main() async { messageId: originalMessage.id + 1, messageIds: [originalMessage.id + 1], flags: originalMessage.flags, - renderedContent: newContent, - editTimestamp: newTimestamp, + renderedContent: "

Hello, edited

", + editTimestamp: 99999, userId: userId, renderingOnly: false, ); - final message = messageList.messages.single; - check(message).content.equals(oldContent); - bool listenersNotified = false; messageList.addListener(() { listenersNotified = true; }); messageList.maybeUpdateMessage(updateEvent); check(listenersNotified).isFalse(); - check(message).content.equals(oldContent); + check(messageList.messages.single) + ..content.equals(originalMessage.content) + ..content.not(it()..equals(updateEvent.renderedContent!)); }); test('rendering-only update does not change timestamp', () async { From 1e972e229e9a4c7607f07631e99c0c5c2b93f661 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:47:49 -0700 Subject: [PATCH 10/15] msglist test [nfc]: Unify legacy vs. current rendering-only cases These two cases differed only in their name, the `renderingOnly` value in the event, and that one of them used `[0]` instead of `.single`. Fix the latter, parameterize, and dedupe. --- test/model/message_list_test.dart | 41 +++++++------------------------ 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index f4ed882345..5feb544b5c 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -137,7 +137,8 @@ void main() async { ..content.not(it()..equals(updateEvent.renderedContent!)); }); - test('rendering-only update does not change timestamp', () async { + // TODO(server-5): Cut legacy case for rendering-only message update + Future checkRenderingOnly({required bool legacy}) async { final store = await setupStore(stream); const oldContent = "

Hello, world

"; @@ -157,47 +158,23 @@ void main() async { flags: originalMessage.flags, renderedContent: newContent, editTimestamp: newTimestamp, - renderingOnly: true, + renderingOnly: legacy ? null : true, userId: null, ); - final message = messageList.messages[0]; + final message = messageList.messages.single; messageList.maybeUpdateMessage(updateEvent); check(message) ..content.equals(newContent) ..lastEditTimestamp.equals(oldTimestamp); + } + + test('rendering-only update does not change timestamp', () async { + await checkRenderingOnly(legacy: false); }); - // TODO(server-5): Cut this test; rely on renderingOnly from FL 114 test('rendering-only update does not change timestamp (for old server versions)', () async { - final store = await setupStore(stream); - - const oldContent = "

Hello, world

"; - const oldTimestamp = 78492; - const newContent = "

Hello, world

Some link preview
"; - const newTimestamp = 99999; - - final originalMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent); - originalMessage.lastEditTimestamp = oldTimestamp; - - final messageList = await messageListViewWithMessages([originalMessage], store, narrow); - - final updateEvent = UpdateMessageEvent( - id: 1, - messageId: originalMessage.id, - messageIds: [originalMessage.id], - flags: originalMessage.flags, - renderedContent: newContent, - editTimestamp: newTimestamp, - renderingOnly: null, - userId: null, - ); - - final message = messageList.messages.single; - messageList.maybeUpdateMessage(updateEvent); - check(message) - ..content.equals(newContent) - ..lastEditTimestamp.equals(oldTimestamp); + await checkRenderingOnly(legacy: true); }); }); } From d7c64cb38212f56fd52721f032390b1d0fcb0a11 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:52:40 -0700 Subject: [PATCH 11/15] msglist test: Similarly tighten rendering-only test cases --- test/model/message_list_test.dart | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 5feb544b5c..ff82e7b2a3 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -141,13 +141,9 @@ void main() async { Future checkRenderingOnly({required bool legacy}) async { final store = await setupStore(stream); - const oldContent = "

Hello, world

"; - const oldTimestamp = 78492; - const newContent = "

Hello, world

Some link preview
"; - const newTimestamp = 99999; - - final originalMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent); - originalMessage.lastEditTimestamp = oldTimestamp; + final originalMessage = eg.streamMessage(id: 972, stream: stream, + content: "

Hello, world

"); + originalMessage.lastEditTimestamp = 78492; final messageList = await messageListViewWithMessages([originalMessage], store, narrow); @@ -156,17 +152,21 @@ void main() async { messageId: originalMessage.id, messageIds: [originalMessage.id], flags: originalMessage.flags, - renderedContent: newContent, - editTimestamp: newTimestamp, + renderedContent: "

Hello, world

Some link preview
", + editTimestamp: 99999, renderingOnly: legacy ? null : true, userId: null, ); final message = messageList.messages.single; messageList.maybeUpdateMessage(updateEvent); - check(message) - ..content.equals(newContent) - ..lastEditTimestamp.equals(oldTimestamp); + check(messageList.messages.single) + ..identicalTo(message) + // Content is updated... + ..content.equals(updateEvent.renderedContent!) + // ... edit timestamp is not. + ..lastEditTimestamp.equals(originalMessage.lastEditTimestamp) + ..lastEditTimestamp.not(it()..equals(updateEvent.editTimestamp)); } test('rendering-only update does not change timestamp', () async { From 50cb61b1fe41b509841d4aefb7c8e3d8572e2bf7 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 15:55:03 -0700 Subject: [PATCH 12/15] test [nfc]: Accept lastEditTimestamp on eg.streamMessage and eg.dmMessage --- test/example_data.dart | 5 ++++- test/model/message_list_test.dart | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index e237b2abc3..2e39806cd4 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -94,7 +94,6 @@ ZulipStream stream({ final _messagePropertiesBase = { 'is_me_message': false, - 'last_edit_timestamp': null, 'recipient_id': 32, // obsolescent in API, and ignored }; @@ -132,6 +131,7 @@ StreamMessage streamMessage({ String? topic, String? content, String? contentMarkdown, + int? lastEditTimestamp, List? flags, }) { final effectiveStream = stream ?? _stream(); @@ -148,6 +148,7 @@ StreamMessage streamMessage({ 'stream_id': effectiveStream.streamId, 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs + 'last_edit_timestamp': lastEditTimestamp, 'subject': topic ?? 'example topic', 'timestamp': 1678139636, 'type': 'stream', @@ -164,6 +165,7 @@ DmMessage dmMessage({ required List to, String? content, String? contentMarkdown, + int? lastEditTimestamp, List? flags, }) { assert(!to.any((user) => user.userId == from.userId)); @@ -177,6 +179,7 @@ DmMessage dmMessage({ 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs + 'last_edit_timestamp': lastEditTimestamp, 'subject': '', 'timestamp': 1678139636, 'type': 'private', diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index ff82e7b2a3..8507dd209e 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -142,9 +142,8 @@ void main() async { final store = await setupStore(stream); final originalMessage = eg.streamMessage(id: 972, stream: stream, + lastEditTimestamp: 78492, content: "

Hello, world

"); - originalMessage.lastEditTimestamp = 78492; - final messageList = await messageListViewWithMessages([originalMessage], store, narrow); final updateEvent = UpdateMessageEvent( From f661e66c33e7980bb35c9219ac38784030060ac3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 16:02:09 -0700 Subject: [PATCH 13/15] msglist test [nfc]: Unify setup functions Each of the test cases calls setupStore and then uses the resulting store for just one thing, namely passing it to messageListViewWithMessages. So just subsume the former into the latter. --- test/model/message_list_test.dart | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 8507dd209e..22c2ce7ed9 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -5,7 +5,6 @@ import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/message_list.dart'; import 'package:zulip/model/narrow.dart'; -import 'package:zulip/model/store.dart'; import '../api/fake_api.dart'; import '../api/model/model_checks.dart'; @@ -15,7 +14,7 @@ import '../model/test_store.dart'; const int userId = 1; -Future setupStore(ZulipStream stream) async { +Future messageListViewWithMessages(List messages, ZulipStream stream, Narrow narrow) async { addTearDown(TestZulipBinding.instance.reset); await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot()); @@ -24,10 +23,6 @@ Future setupStore(ZulipStream stream) async { store.addUser(eg.user(userId: userId)); store.addStream(stream); - return store; -} - -Future messageListViewWithMessages(List messages, PerAccountStore store, Narrow narrow) async { final messageList = MessageListView.init(store: store, narrow: narrow); final connection = store.connection as FakeApiConnection; @@ -51,11 +46,10 @@ void main() async { final narrow = StreamNarrow(stream.streamId); test('findMessageWithId', () async { - final store = await setupStore(stream); final m1 = eg.streamMessage(id: 2, stream: stream); final m2 = eg.streamMessage(id: 4, stream: stream); final m3 = eg.streamMessage(id: 6, stream: stream); - final messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow); + final messageList = await messageListViewWithMessages([m1, m2, m3], stream, narrow); // Exercise the binary search before, at, and after each element of the list. check(messageList.findMessageWithId(1)).equals(-1); @@ -69,13 +63,11 @@ void main() async { group('maybeUpdateMessage', () { test('update a message', () async { - final store = await setupStore(stream); - final originalMessage = eg.streamMessage(id: 243, stream: stream, content: "

Hello, world

", flags: [], ); - final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); final updateEvent = UpdateMessageEvent( id: 1, @@ -110,11 +102,9 @@ void main() async { }); test('ignore when message not present', () async { - final store = await setupStore(stream); - final originalMessage = eg.streamMessage(id: 243, stream: stream, content: "

Hello, world

"); - final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); final updateEvent = UpdateMessageEvent( id: 1, @@ -139,12 +129,10 @@ void main() async { // TODO(server-5): Cut legacy case for rendering-only message update Future checkRenderingOnly({required bool legacy}) async { - final store = await setupStore(stream); - final originalMessage = eg.streamMessage(id: 972, stream: stream, lastEditTimestamp: 78492, content: "

Hello, world

"); - final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); final updateEvent = UpdateMessageEvent( id: 1, From bd22cd25df14c7555238ef655af59e31c7dcb9ac Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 16:08:16 -0700 Subject: [PATCH 14/15] msglist test [nfc]: Reorder to set up listener next to message list itself --- test/model/message_list_test.dart | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 22c2ce7ed9..59ac83a9d6 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -64,11 +64,7 @@ void main() async { group('maybeUpdateMessage', () { test('update a message', () async { final originalMessage = eg.streamMessage(id: 243, stream: stream, - content: "

Hello, world

", - flags: [], - ); - final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); - + content: "

Hello, world

"); final updateEvent = UpdateMessageEvent( id: 1, messageId: originalMessage.id, @@ -81,6 +77,10 @@ void main() async { renderingOnly: false, ); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); + final message = messageList.messages.single; check(message) ..content.not(it()..equals(updateEvent.renderedContent!)) @@ -88,9 +88,6 @@ void main() async { ..flags.not(it()..deepEquals(updateEvent.flags)) ..isMeMessage.not(it()..equals(updateEvent.isMeMessage!)); - bool listenersNotified = false; - messageList.addListener(() { listenersNotified = true; }); - messageList.maybeUpdateMessage(updateEvent); check(listenersNotified).isTrue(); check(messageList.messages.single) @@ -104,8 +101,6 @@ void main() async { test('ignore when message not present', () async { final originalMessage = eg.streamMessage(id: 243, stream: stream, content: "

Hello, world

"); - final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); - final updateEvent = UpdateMessageEvent( id: 1, messageId: originalMessage.id + 1, @@ -117,6 +112,7 @@ void main() async { renderingOnly: false, ); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); bool listenersNotified = false; messageList.addListener(() { listenersNotified = true; }); @@ -132,8 +128,6 @@ void main() async { final originalMessage = eg.streamMessage(id: 972, stream: stream, lastEditTimestamp: 78492, content: "

Hello, world

"); - final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); - final updateEvent = UpdateMessageEvent( id: 1, messageId: originalMessage.id, @@ -145,6 +139,8 @@ void main() async { userId: null, ); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); + final message = messageList.messages.single; messageList.maybeUpdateMessage(updateEvent); check(messageList.messages.single) From edb88208c06ff040ad803217f04a3f180bb66966 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Aug 2023 16:09:26 -0700 Subject: [PATCH 15/15] msglist test: Consistently check whether listeners notified --- test/model/message_list_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 59ac83a9d6..6893938a23 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -140,9 +140,12 @@ void main() async { ); final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); final message = messageList.messages.single; messageList.maybeUpdateMessage(updateEvent); + check(listenersNotified).isTrue(); check(messageList.messages.single) ..identicalTo(message) // Content is updated...