mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
Fix flaky test in TimelinePresenterTest (#7702)
The problem seemed to be sometimes the background work to switch from live timeline to a focused event timeline wasn't finished in time and an assertion failed from time to time. By providing the coroutine to run this on we can pass a `TestScope` and force it to reliably run immediately instead in unit tests.
This commit is contained in:
committed by
GitHub
parent
8cd3750cde
commit
3d198d8065
+8
-1
@@ -51,6 +51,7 @@ import io.element.android.libraries.androidutils.system.toast
|
||||
import io.element.android.libraries.architecture.NodeInputs
|
||||
import io.element.android.libraries.architecture.callback
|
||||
import io.element.android.libraries.architecture.inputs
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.designsystem.utils.OnLifecycleEvent
|
||||
import io.element.android.libraries.di.RoomScope
|
||||
import io.element.android.libraries.di.annotations.ApplicationContext
|
||||
@@ -103,6 +104,7 @@ class MessagesNode(
|
||||
private val roomMemberModerationRenderer: RoomMemberModerationRenderer,
|
||||
private val eventContentValidationCache: EventContentValidationCache,
|
||||
private val emojiPickerRenderer: EmojiPickerRenderer,
|
||||
private val dispatchers: CoroutineDispatchers,
|
||||
) : Node(buildContext, plugins = plugins), MessagesNavigator {
|
||||
data class Inputs(
|
||||
val focusedEventId: EventId?,
|
||||
@@ -111,7 +113,12 @@ class MessagesNode(
|
||||
private val inputs = inputs<Inputs>()
|
||||
private val callback: Callback = callback()
|
||||
|
||||
private val timelineController = TimelineController(room, room.liveTimeline)
|
||||
private val timelineController = TimelineController(
|
||||
room = room,
|
||||
liveTimeline = room.liveTimeline,
|
||||
roomCoroutineScope = room.roomCoroutineScope,
|
||||
dispatchers = dispatchers,
|
||||
)
|
||||
private val presenter = presenterFactory.create(
|
||||
navigator = this,
|
||||
composerPresenter = messageComposerPresenterFactory.create(timelineController, this, threadRoot = null),
|
||||
|
||||
+8
-1
@@ -54,6 +54,7 @@ import io.element.android.libraries.architecture.NodeInputs
|
||||
import io.element.android.libraries.architecture.Presenter
|
||||
import io.element.android.libraries.architecture.callback
|
||||
import io.element.android.libraries.architecture.inputs
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.designsystem.utils.OnLifecycleEvent
|
||||
import io.element.android.libraries.di.RoomScope
|
||||
import io.element.android.libraries.emoji.api.picker.EmojiPickerRenderer
|
||||
@@ -95,6 +96,7 @@ class ThreadedMessagesNode(
|
||||
private val appNavigationStateService: AppNavigationStateService,
|
||||
private val roomMemberModerationRenderer: RoomMemberModerationRenderer,
|
||||
private val emojiPickerRenderer: EmojiPickerRenderer,
|
||||
private val dispatchers: CoroutineDispatchers,
|
||||
) : Node(buildContext, plugins = plugins), MessagesNavigator {
|
||||
data class Inputs(
|
||||
val threadRootEventId: ThreadId,
|
||||
@@ -113,7 +115,12 @@ class ThreadedMessagesNode(
|
||||
*/
|
||||
private suspend fun createPresenter(): Presenter<MessagesState> {
|
||||
val threadedTimeline = room.createTimeline(CreateTimelineParams.Threaded(threadRootEventId = inputs.threadRootEventId)).getOrThrow()
|
||||
val timelineController = TimelineController(room, threadedTimeline)
|
||||
val timelineController = TimelineController(
|
||||
room,
|
||||
liveTimeline = threadedTimeline,
|
||||
roomCoroutineScope = room.roomCoroutineScope,
|
||||
dispatchers = dispatchers,
|
||||
)
|
||||
this.timelineController = timelineController
|
||||
return presenterFactory.create(
|
||||
navigator = this,
|
||||
|
||||
+6
-2
@@ -12,7 +12,10 @@ import dev.zacsweers.metro.ContributesBinding
|
||||
import dev.zacsweers.metro.SingleIn
|
||||
import dev.zacsweers.metro.binding
|
||||
import io.element.android.features.messages.impl.timeline.di.LiveTimeline
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.core.coroutine.childScope
|
||||
import io.element.android.libraries.di.RoomScope
|
||||
import io.element.android.libraries.di.annotations.RoomCoroutineScope
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.ThreadId
|
||||
import io.element.android.libraries.matrix.api.room.CreateTimelineParams
|
||||
@@ -23,7 +26,6 @@ import io.element.android.libraries.matrix.api.timeline.TimelineProvider
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.SupervisorJob
|
||||
import kotlinx.coroutines.cancel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
@@ -47,8 +49,10 @@ import java.util.Optional
|
||||
class TimelineController(
|
||||
private val room: JoinedRoom,
|
||||
@LiveTimeline private val liveTimeline: Timeline,
|
||||
@RoomCoroutineScope private val roomCoroutineScope: CoroutineScope,
|
||||
dispatchers: CoroutineDispatchers,
|
||||
) : Closeable, TimelineProvider {
|
||||
private val coroutineScope = CoroutineScope(SupervisorJob())
|
||||
private val coroutineScope = roomCoroutineScope.childScope(dispatchers.computation, "TimelineController")
|
||||
|
||||
private val liveTimelineFlow = flowOf(liveTimeline)
|
||||
private val detachedTimelineFlow = MutableStateFlow<Optional<Timeline>>(Optional.empty())
|
||||
|
||||
+6
-1
@@ -1484,7 +1484,12 @@ class MessagesPresenterTest {
|
||||
clipboardHelper = clipboardHelper,
|
||||
htmlConverterProvider = FakeHtmlConverterProvider(),
|
||||
buildMeta = aBuildMeta(),
|
||||
timelineController = TimelineController(joinedRoom, timeline),
|
||||
timelineController = TimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = timeline,
|
||||
roomCoroutineScope = backgroundScope,
|
||||
dispatchers = coroutineDispatchers,
|
||||
),
|
||||
permalinkParser = permalinkParser,
|
||||
analyticsService = analyticsService,
|
||||
encryptionService = encryptionService,
|
||||
|
||||
+7
-1
@@ -65,6 +65,7 @@ import io.element.android.tests.testutils.WarmUpRule
|
||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.lambda.value
|
||||
import io.element.android.tests.testutils.test
|
||||
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.TestScope
|
||||
@@ -307,7 +308,12 @@ class MessageComposerPresenterSlashCommandTest {
|
||||
permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionPresenter),
|
||||
permalinkParser = permalinkParser,
|
||||
permalinkBuilder = permalinkBuilder,
|
||||
timelineController = TimelineController(room, timeline),
|
||||
timelineController = TimelineController(
|
||||
room = room,
|
||||
liveTimeline = timeline,
|
||||
roomCoroutineScope = backgroundScope,
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
),
|
||||
draftService = draftService,
|
||||
mentionSpanProvider = mentionSpanProvider,
|
||||
pillificationHelper = textPillificationHelper,
|
||||
|
||||
+7
-1
@@ -113,6 +113,7 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.lambda.value
|
||||
import io.element.android.tests.testutils.robolectric.RobolectricTest
|
||||
import io.element.android.tests.testutils.test
|
||||
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
import io.element.android.tests.testutils.waitForPredicate
|
||||
import io.mockk.mockk
|
||||
import kotlinx.collections.immutable.ImmutableList
|
||||
@@ -1707,7 +1708,12 @@ class MessageComposerPresenterTest : RobolectricTest() {
|
||||
permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionPresenter),
|
||||
permalinkParser = permalinkParser,
|
||||
permalinkBuilder = permalinkBuilder,
|
||||
timelineController = TimelineController(room, timeline),
|
||||
timelineController = TimelineController(
|
||||
room = room,
|
||||
liveTimeline = timeline,
|
||||
roomCoroutineScope = backgroundScope,
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
),
|
||||
draftService = draftService,
|
||||
mentionSpanProvider = mentionSpanProvider,
|
||||
pillificationHelper = textPillificationHelper,
|
||||
|
||||
+34
-9
@@ -21,8 +21,10 @@ import io.element.android.libraries.matrix.test.timeline.FakeTimeline
|
||||
import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem
|
||||
import io.element.android.tests.testutils.lambda.lambdaError
|
||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.test.TestScope
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Test
|
||||
|
||||
@@ -35,7 +37,10 @@ class TimelineControllerTest {
|
||||
liveTimeline = liveTimeline,
|
||||
createTimelineResult = { Result.success(detachedTimeline) }
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
|
||||
sut.activeTimelineFlow().test {
|
||||
awaitItem().also { state ->
|
||||
@@ -74,7 +79,10 @@ class TimelineControllerTest {
|
||||
}
|
||||
}
|
||||
)
|
||||
val sut = TimelineController(joinedRoom, liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
|
||||
sut.activeTimelineFlow().test {
|
||||
awaitItem().also { state ->
|
||||
@@ -102,7 +110,10 @@ class TimelineControllerTest {
|
||||
val joinedRoom = FakeJoinedRoom(
|
||||
liveTimeline = liveTimeline
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
sut.activeTimelineFlow().test {
|
||||
awaitItem().also { state ->
|
||||
assertThat(state).isEqualTo(liveTimeline)
|
||||
@@ -121,7 +132,10 @@ class TimelineControllerTest {
|
||||
liveTimeline = liveTimeline,
|
||||
createTimelineResult = { Result.success(detachedTimeline) }
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
sut.activeTimelineFlow().test {
|
||||
awaitItem().also { state ->
|
||||
assertThat(state).isEqualTo(liveTimeline)
|
||||
@@ -149,7 +163,10 @@ class TimelineControllerTest {
|
||||
val joinedRoom = FakeJoinedRoom(
|
||||
liveTimeline = liveTimeline
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
assertThat(sut.timelineItems().first()).hasSize(1)
|
||||
}
|
||||
|
||||
@@ -171,7 +188,10 @@ class TimelineControllerTest {
|
||||
liveTimeline = liveTimeline,
|
||||
createTimelineResult = { Result.success(detachedTimeline) }
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
sut.activeTimelineFlow().test {
|
||||
sut.focusOnEvent(AN_EVENT_ID, null)
|
||||
awaitItem().also { state ->
|
||||
@@ -196,7 +216,10 @@ class TimelineControllerTest {
|
||||
liveTimeline = liveTimeline,
|
||||
createTimelineResult = { Result.success(detachedTimeline) }
|
||||
)
|
||||
val sut = TimelineController(room = joinedRoom, liveTimeline = liveTimeline)
|
||||
val sut = createTimelineController(
|
||||
room = joinedRoom,
|
||||
liveTimeline = liveTimeline,
|
||||
)
|
||||
|
||||
sut.activeTimelineFlow().test {
|
||||
awaitItem().also { state ->
|
||||
@@ -220,12 +243,14 @@ class TimelineControllerTest {
|
||||
}
|
||||
}
|
||||
|
||||
internal fun createTimelineController(
|
||||
internal fun TestScope.createTimelineController(
|
||||
room: FakeJoinedRoom = FakeJoinedRoom(liveTimeline = FakeTimeline()),
|
||||
liveTimeline: Timeline = FakeTimeline(name = "live"),
|
||||
): TimelineController {
|
||||
return TimelineController(
|
||||
room = room,
|
||||
liveTimeline = liveTimeline
|
||||
liveTimeline = liveTimeline,
|
||||
roomCoroutineScope = backgroundScope,
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
)
|
||||
}
|
||||
|
||||
+11
-2
@@ -1079,7 +1079,11 @@ class TimelinePresenterTest {
|
||||
val focusedState = consumeItemsUntilPredicate { it.focusRequestState == FocusRequestState.Success(AN_EVENT_ID) }.last()
|
||||
assertThat(focusedState.isLive).isFalse()
|
||||
initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS))
|
||||
advanceUntilIdle()
|
||||
|
||||
// Give some time for any potential pagination to be called, but it should not be called because the focused event is not rendered yet.
|
||||
// Also, given enough time, a new state should be emitted with isLive = false, because we switched to a focused event timeline.
|
||||
runCurrent()
|
||||
|
||||
assert(paginateLambda).isNeverCalled()
|
||||
assertThat(expectMostRecentItem().isLive).isFalse()
|
||||
}
|
||||
@@ -1906,7 +1910,12 @@ class TimelinePresenterTest {
|
||||
sendPollResponseAction = sendPollResponseAction,
|
||||
sessionPreferencesStore = sessionPreferencesStore,
|
||||
timelineItemIndexer = timelineItemIndexer,
|
||||
timelineController = TimelineController(room, timeline),
|
||||
timelineController = TimelineController(
|
||||
room = room,
|
||||
liveTimeline = timeline,
|
||||
roomCoroutineScope = backgroundScope,
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
),
|
||||
resolveVerifiedUserSendFailurePresenter = resolveVerifiedUserSendFailurePresenter,
|
||||
typingNotificationPresenter = { aTypingNotificationState() },
|
||||
roomCallStatePresenter = { aStandByCallState() },
|
||||
|
||||
Reference in New Issue
Block a user