From ced62779fb5f48d36f81eea4b016d83b0eb44799 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Mon, 26 Apr 2021 14:01:16 -0400 Subject: [PATCH] Fix timing issue for flaky tests Inject SystemClock so that we can mock it in tests and ensure the clock will always advance. Also switched to using elapsedRealtime, which is guaranteed to be monotonic Fixes: 182813365 Test: atest MediaDataManagerTest Change-Id: I133fb48e8242dc7f8e75bfa5c585c8b6142963aa --- .../android/systemui/media/MediaDataFilter.kt | 9 ++++--- .../systemui/media/MediaDataManager.kt | 13 ++++++---- .../systemui/media/MediaDataFilterTest.kt | 14 ++++++---- .../systemui/media/MediaDataManagerTest.kt | 26 +++++++++++-------- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt index 9163044c2d739..e5c15107be371 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt @@ -25,6 +25,7 @@ import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.settings.CurrentUserTracker import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.util.time.SystemClock import java.util.concurrent.Executor import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -36,7 +37,8 @@ private const val DEBUG = true * Maximum age of a media control to re-activate on smartspace signal. If there is no media control * available within this time window, smartspace recommendations will be shown instead. */ -private val SMARTSPACE_MAX_AGE = SystemProperties +@VisibleForTesting +internal val SMARTSPACE_MAX_AGE = SystemProperties .getLong("debug.sysui.smartspace_max_age", TimeUnit.HOURS.toMillis(3)) /** @@ -51,7 +53,8 @@ class MediaDataFilter @Inject constructor( private val broadcastDispatcher: BroadcastDispatcher, private val mediaResumeListener: MediaResumeListener, private val lockscreenUserManager: NotificationLockscreenUserManager, - @Main private val executor: Executor + @Main private val executor: Executor, + private val systemClock: SystemClock ) : MediaDataManager.Listener { private val userTracker: CurrentUserTracker private val _listeners: MutableSet = mutableSetOf() @@ -100,7 +103,7 @@ class MediaDataFilter @Inject constructor( hasSmartspace = true // Before forwarding the smartspace target, first check if we have recently inactive media - val now = System.currentTimeMillis() + val now = systemClock.elapsedRealtime() val sorted = userEntries.toSortedMap(compareBy { userEntries.get(it)?.lastActive ?: -1 }) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index 138c4222831f5..a45bd33c346a3 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -56,6 +56,7 @@ import com.android.systemui.statusbar.notification.row.HybridGroupManager import com.android.systemui.util.Assert import com.android.systemui.util.Utils import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.time.SystemClock import java.io.FileDescriptor import java.io.IOException import java.io.PrintWriter @@ -108,7 +109,8 @@ class MediaDataManager( private val activityStarter: ActivityStarter, private val smartspaceMediaDataProvider: SmartspaceMediaDataProvider, private var useMediaResumption: Boolean, - private val useQsMediaPlayer: Boolean + private val useQsMediaPlayer: Boolean, + private val systemClock: SystemClock ) : Dumpable, BcSmartspaceDataPlugin.SmartspaceTargetListener { companion object { @@ -164,12 +166,13 @@ class MediaDataManager( mediaDataCombineLatest: MediaDataCombineLatest, mediaDataFilter: MediaDataFilter, activityStarter: ActivityStarter, - smartspaceMediaDataProvider: SmartspaceMediaDataProvider + smartspaceMediaDataProvider: SmartspaceMediaDataProvider, + clock: SystemClock ) : this(context, backgroundExecutor, foregroundExecutor, mediaControllerFactory, broadcastDispatcher, dumpManager, mediaTimeoutListener, mediaResumeListener, mediaSessionBasedFilter, mediaDeviceManager, mediaDataCombineLatest, mediaDataFilter, activityStarter, smartspaceMediaDataProvider, Utils.useMediaResumption(context), - Utils.useQsMediaPlayer(context)) + Utils.useQsMediaPlayer(context), clock) private val appChangeReceiver = object : BroadcastReceiver() { override fun onReceive(context: Context, intent: Intent) { @@ -474,7 +477,7 @@ class MediaDataManager( } val mediaAction = getResumeMediaAction(resumeAction) - val lastActive = System.currentTimeMillis() + val lastActive = systemClock.elapsedRealtime() foregroundExecutor.execute { onMediaDataLoaded(packageName, null, MediaData(userId, true, bgColor, appName, null, desc.subtitle, desc.title, artworkIcon, listOf(mediaAction), listOf(0), @@ -597,7 +600,7 @@ class MediaDataManager( val isLocalSession = mediaController.playbackInfo?.playbackType == MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL val isPlaying = mediaController.playbackState?.let { isPlayingState(it.state) } ?: null - val lastActive = System.currentTimeMillis() + val lastActive = systemClock.elapsedRealtime() foregroundExecutor.execute { val resumeAction: Runnable? = mediaEntries[key]?.resumeAction val hasCheckedForResume = mediaEntries[key]?.hasCheckedForResume == true diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt index ac24cdeb74bc2..bfd60b969287a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt @@ -25,6 +25,7 @@ import android.testing.TestableLooper import com.android.systemui.SysuiTestCase import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import org.junit.Before import org.junit.Test @@ -80,12 +81,13 @@ class MediaDataFilterTest : SysuiTestCase() { private lateinit var dataMain: MediaData private lateinit var dataGuest: MediaData private val device = MediaDeviceData(true, null, DEVICE_NAME) + private val clock = FakeSystemClock() @Before fun setup() { MockitoAnnotations.initMocks(this) mediaDataFilter = MediaDataFilter(broadcastDispatcher, mediaResumeListener, - lockscreenUserManager, executor) + lockscreenUserManager, executor, clock) mediaDataFilter.mediaDataManager = mediaDataManager mediaDataFilter.addListener(listener) @@ -246,8 +248,9 @@ class MediaDataFilterTest : SysuiTestCase() { @Test fun testOnSmartspaceMediaDataLoaded_noRecentMedia_nonEmptyRecommendation_usesSmartspace() { - val dataOld = dataMain.copy(active = false, lastActive = 0L) + val dataOld = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataOld) + clock.advanceTime(SMARTSPACE_MAX_AGE + 100) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) verify(listener).onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData)) @@ -258,8 +261,9 @@ class MediaDataFilterTest : SysuiTestCase() { fun testOnSmartspaceMediaDataLoaded_noRecentMedia_emptyRecommendation_showsNothing() { `when`(smartspaceData.iconGrid).thenReturn(listOf()) - val dataOld = dataMain.copy(active = false, lastActive = 0L) + val dataOld = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataOld) + clock.advanceTime(SMARTSPACE_MAX_AGE + 100) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) verify(listener, never()) @@ -270,7 +274,7 @@ class MediaDataFilterTest : SysuiTestCase() { @Test fun testOnSmartspaceMediaDataLoaded_hasRecentMedia_usesMedia() { // WHEN we have media that was recently played, but not currently active - val dataCurrent = dataMain.copy(active = false, lastActive = System.currentTimeMillis()) + val dataCurrent = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataCurrent) verify(listener).onMediaDataLoaded(eq(KEY), eq(null), eq(dataCurrent)) @@ -294,7 +298,7 @@ class MediaDataFilterTest : SysuiTestCase() { @Test fun testOnSmartspaceMediaDataRemoved_usedMedia_clearsMedia() { - val dataCurrent = dataMain.copy(active = false, lastActive = System.currentTimeMillis()) + val dataCurrent = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataCurrent) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt index daa8b4bbb6d05..acfc513eb27a0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt @@ -81,11 +81,12 @@ class MediaDataManagerTest : SysuiTestCase() { lateinit var mediaDataManager: MediaDataManager lateinit var mediaNotification: StatusBarNotification @Captor lateinit var mediaDataCaptor: ArgumentCaptor + private val clock = FakeSystemClock() @Before fun setup() { - foregroundExecutor = FakeExecutor(FakeSystemClock()) - backgroundExecutor = FakeExecutor(FakeSystemClock()) + foregroundExecutor = FakeExecutor(clock) + backgroundExecutor = FakeExecutor(clock) smartspaceMediaDataProvider = SmartspaceMediaDataProvider() mediaDataManager = MediaDataManager( context = context, @@ -103,7 +104,8 @@ class MediaDataManagerTest : SysuiTestCase() { activityStarter = activityStarter, smartspaceMediaDataProvider = smartspaceMediaDataProvider, useMediaResumption = true, - useQsMediaPlayer = true + useQsMediaPlayer = true, + systemClock = clock ) session = MediaSession(context, "MediaDataManagerTestSession") mediaNotification = SbnBuilder().run { @@ -310,7 +312,7 @@ class MediaDataManagerTest : SysuiTestCase() { setTitle(SESSION_TITLE) build() } - val currentTimeMillis = System.currentTimeMillis() + val currentTime = clock.elapsedRealtime() mediaDataManager.addResumptionControls(USER_ID, desc, Runnable {}, session.sessionToken, APP_NAME, pendingIntent, PACKAGE_NAME) assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) @@ -322,7 +324,7 @@ class MediaDataManagerTest : SysuiTestCase() { assertThat(data.song).isEqualTo(SESSION_TITLE) assertThat(data.app).isEqualTo(APP_NAME) assertThat(data.actions).hasSize(1) - assertThat(data.lastActive).isAtLeast(currentTimeMillis) + assertThat(data.lastActive).isAtLeast(currentTime) } @Test @@ -380,12 +382,12 @@ class MediaDataManagerTest : SysuiTestCase() { @Test fun testOnMediaDataChanged_updatesLastActiveTime() { - val currentTimeMillis = System.currentTimeMillis() + val currentTime = clock.elapsedRealtime() mediaDataManager.onNotificationAdded(KEY, mediaNotification) assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor)) - assertThat(mediaDataCaptor.value!!.lastActive).isAtLeast(currentTimeMillis) + assertThat(mediaDataCaptor.value!!.lastActive).isAtLeast(currentTime) } @Test @@ -396,12 +398,13 @@ class MediaDataManagerTest : SysuiTestCase() { assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) // WHEN the notification times out - val currentTimeMillis = System.currentTimeMillis() + clock.advanceTime(100) + val currentTime = clock.elapsedRealtime() mediaDataManager.setTimedOut(KEY, true, true) // THEN the last active time is not changed verify(listener).onMediaDataLoaded(eq(KEY), eq(KEY), capture(mediaDataCaptor)) - assertThat(mediaDataCaptor.value.lastActive).isLessThan(currentTimeMillis) + assertThat(mediaDataCaptor.value.lastActive).isLessThan(currentTime) } @Test @@ -417,13 +420,14 @@ class MediaDataManagerTest : SysuiTestCase() { mediaDataManager.onMediaDataLoaded(KEY, null, data.copy(resumeAction = Runnable {})) // WHEN the notification is removed - val currentTimeMillis = System.currentTimeMillis() + clock.advanceTime(100) + val currentTime = clock.elapsedRealtime() mediaDataManager.onNotificationRemoved(KEY) // THEN the last active time is not changed verify(listener).onMediaDataLoaded(eq(PACKAGE_NAME), eq(KEY), capture(mediaDataCaptor)) assertThat(mediaDataCaptor.value.resumption).isTrue() - assertThat(mediaDataCaptor.value.lastActive).isLessThan(currentTimeMillis) + assertThat(mediaDataCaptor.value.lastActive).isLessThan(currentTime) } @Test