From 5a1545058bb8e00cd714edaaeea6af401f17988d Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 18 May 2022 11:20:21 +0100 Subject: [PATCH 1/3] Instead of using a magic number, explicitly test for the events we expect. This permits a clear error when the events are missing / extra and while not making the test invulnerable to future changes in events, should be explicit on what's changed. --- .../timeline/TimelineForwardPaginationTest.kt | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelineForwardPaginationTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelineForwardPaginationTest.kt index d5b4a07fc0..e407c1b42d 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelineForwardPaginationTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelineForwardPaginationTest.kt @@ -140,9 +140,24 @@ class TimelineForwardPaginationTest : InstrumentedTest { aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() assertEquals(EventType.STATE_ROOM_CREATE, snapshot.lastOrNull()?.root?.getClearType()) - // 6 for room creation item (backward pagination), 1 for the context, and 50 for the forward pagination - // 6 + 1 + 50 - assertEquals(57, snapshot.size) + + // We explicitly test all the types we expect here, as we expect 51 messages and "some" state events + // But state events can change over time. So this acts as a kinda documentation of what we expect and + // provides a good error message if it doesn't match + + val snapshotTypes = mutableMapOf() + snapshot.groupingBy { it -> it.root.type }.eachCountTo(snapshotTypes) + // Some state events on room creation + assertEquals("m.room.name", 1, snapshotTypes.remove("m.room.name")) + assertEquals("m.room.guest_access", 1, snapshotTypes.remove("m.room.guest_access")) + assertEquals("m.room.history_visibility", 1, snapshotTypes.remove("m.room.history_visibility")) + assertEquals("m.room.join_rules", 1, snapshotTypes.remove("m.room.join_rules")) + assertEquals("m.room.power_levels", 1, snapshotTypes.remove("m.room.power_levels")) + assertEquals("m.room.create", 1, snapshotTypes.remove("m.room.create")) + assertEquals("m.room.member", 1, snapshotTypes.remove("m.room.member")) + // 50 from pagination + 1 context + assertEquals("m.room.message", 51, snapshotTypes.remove("m.room.message")) + assertEquals("Additional events found in timeline", setOf(), snapshotTypes.keys) } // Alice paginates once again FORWARD for 50 events @@ -152,8 +167,8 @@ class TimelineForwardPaginationTest : InstrumentedTest { val snapshot = runBlocking { aliceTimeline.awaitPaginate(Timeline.Direction.FORWARDS, 50) } - // 6 for room creation item (backward pagination),and numberOfMessagesToSend (all the message of the room) - snapshot.size == 6 + numberOfMessagesToSend && + // 7 for room creation item (backward pagination),and numberOfMessagesToSend (all the message of the room) + snapshot.size == 7 + numberOfMessagesToSend && snapshot.checkSendOrder(message, numberOfMessagesToSend, 0) // The timeline is fully loaded From 261dadb9860a640e62956d0596ac5140254cda4f Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 18 May 2022 12:37:07 +0100 Subject: [PATCH 2/3] Additionally increment for TimelinePreviousLastForwardTest --- .../room/timeline/TimelinePreviousLastForwardTest.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt index 6e5fed8df9..ff8823a39c 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt @@ -74,8 +74,12 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { Timber.w(" event ${it.root}") } - // Ok, we have the 8 first messages of the initial sync (room creation and bob invite and join events) - snapshot.size == 8 + // Ok, we have the 9 first messages of the initial sync (room creation and bob invite and join events) + // create + // join alice + // power_levels, join_rules, history_visibility, guest_access, name + // invite, join bob + snapshot.size == 9 } bobTimeline.addListener(eventsListener) @@ -192,7 +196,7 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { Timber.w(" event ${it.root}") } - snapshot.size == 44 // 8 + 1 + 35 + snapshot.size == 45 // 9 + 1 + 35 } bobTimeline.addListener(eventsListener) @@ -221,7 +225,7 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Bob can see the first event of the room (so Back pagination has worked) snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE && // 8 for room creation item 60 message from Alice - snapshot.size == 68 && // 8 + 60 + snapshot.size == 69 && // 8 + 60 snapshot.checkSendOrder(secondMessage, 30, 0) && snapshot.checkSendOrder(firstMessage, 30, 30) } From 94411ed60e912210f104e21d9dae29d5db42fe7a Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 18 May 2022 16:34:39 +0100 Subject: [PATCH 3/3] Update TimelinePreviousLastForwardTest.kt Update comments in line with code changes. --- .../session/room/timeline/TimelinePreviousLastForwardTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt index ff8823a39c..1a36adec44 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/room/timeline/TimelinePreviousLastForwardTest.kt @@ -224,8 +224,8 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Bob can see the first event of the room (so Back pagination has worked) snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE && - // 8 for room creation item 60 message from Alice - snapshot.size == 69 && // 8 + 60 + // 9 for room creation item 60 message from Alice + snapshot.size == 69 && // 9 + 60U snapshot.checkSendOrder(secondMessage, 30, 0) && snapshot.checkSendOrder(firstMessage, 30, 30) }