From 16d64aea2ef5c0b4dd72e3f7498c84e3e5f2daca Mon Sep 17 00:00:00 2001 From: Michael Mikhail Date: Wed, 4 Jan 2023 18:43:11 +0000 Subject: [PATCH] Fix MediaCarouselController Tests Enable tests and use real configuration object instead of a mock. Move the check for the size media players and media content inside reorderAllPlayers function because we only need to check when a new view is actually added to media carousel. Bug: 253229241 Test: atest MediaCarouselControllerTest Change-Id: Ia8a7af53d02aa01b30941ada004a7da03603c821 --- .../controls/ui/MediaCarouselController.kt | 22 ++++++------- .../ui/MediaCarouselControllerTest.kt | 31 +++---------------- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt index b72923a5d22cd..4e21476a0fbdc 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt @@ -590,6 +590,17 @@ constructor( ?: mediaCarouselScrollHandler.scrollToPlayer(destIndex = mediaIndex) } } + // Check postcondition: mediaContent should have the same number of children as there + // are + // elements in mediaPlayers. + if (MediaPlayerData.players().size != mediaContent.childCount) { + Log.e( + TAG, + "Size of players list and number of views in carousel are out of sync. " + + "Players size is ${MediaPlayerData.players().size}. " + + "View count is ${mediaContent.childCount}." + ) + } } // Returns true if new player is added @@ -665,17 +676,6 @@ constructor( updatePageIndicator() mediaCarouselScrollHandler.onPlayersChanged() mediaFrame.requiresRemeasuring = true - // Check postcondition: mediaContent should have the same number of children as there - // are - // elements in mediaPlayers. - if (MediaPlayerData.players().size != mediaContent.childCount) { - Log.e( - TAG, - "Size of players list and number of views in carousel are out of sync. " + - "Players size is ${MediaPlayerData.players().size}. " + - "View count is ${mediaContent.childCount}." - ) - } return existingPlayer == null } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt index 997198e116c08..5ebcca926abdd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt @@ -61,7 +61,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Before -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor @@ -107,7 +106,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { @Captor lateinit var listener: ArgumentCaptor @Captor lateinit var configListener: ArgumentCaptor - @Captor lateinit var newConfig: ArgumentCaptor @Captor lateinit var visualStabilityCallback: ArgumentCaptor @Captor lateinit var keyguardCallback: ArgumentCaptor @@ -150,7 +148,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { MediaPlayerData.clear() } - @Ignore("b/253229241") @Test fun testPlayerOrdering() { // Test values: key, data, last active time @@ -327,7 +324,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { } } - @Ignore("b/253229241") @Test fun testOrderWithSmartspace_prioritized() { testPlayerOrdering() @@ -335,7 +331,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { // If smartspace is prioritized MediaPlayerData.addMediaRecommendation( SMARTSPACE_KEY, - EMPTY_SMARTSPACE_MEDIA_DATA, + EMPTY_SMARTSPACE_MEDIA_DATA.copy(isActive = true), panel, true, clock @@ -345,7 +341,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertTrue(MediaPlayerData.playerKeys().elementAt(2).isSsMediaRec) } - @Ignore("b/253229241") @Test fun testOrderWithSmartspace_prioritized_updatingVisibleMediaPlayers() { testPlayerOrdering() @@ -362,7 +357,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertTrue(MediaPlayerData.visiblePlayerKeys().elementAt(2).isSsMediaRec) } - @Ignore("b/253229241") @Test fun testOrderWithSmartspace_notPrioritized() { testPlayerOrdering() @@ -370,7 +364,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { // If smartspace is not prioritized MediaPlayerData.addMediaRecommendation( SMARTSPACE_KEY, - EMPTY_SMARTSPACE_MEDIA_DATA, + EMPTY_SMARTSPACE_MEDIA_DATA.copy(isActive = true), panel, false, clock @@ -381,7 +375,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertTrue(MediaPlayerData.playerKeys().elementAt(idx).isSsMediaRec) } - @Ignore("b/253229241") @Test fun testPlayingExistingMediaPlayerFromCarousel_visibleMediaPlayersNotUpdated() { testPlayerOrdering() @@ -419,7 +412,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { ) } - @Ignore("b/253229241") @Test fun testSwipeDismiss_logged() { mediaCarouselController.mediaCarouselScrollHandler.dismissCallback.invoke() @@ -427,7 +419,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logSwipeDismiss() } - @Ignore("b/253229241") @Test fun testSettingsButton_logged() { mediaCarouselController.settingsButton.callOnClick() @@ -435,18 +426,16 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logCarouselSettings() } - @Ignore("b/253229241") @Test fun testLocationChangeQs_logged() { mediaCarouselController.onDesiredLocationChanged( - MediaHierarchyManager.LOCATION_QS, + LOCATION_QS, mediaHostState, animate = false ) - verify(logger).logCarouselPosition(MediaHierarchyManager.LOCATION_QS) + verify(logger).logCarouselPosition(LOCATION_QS) } - @Ignore("b/253229241") @Test fun testLocationChangeQqs_logged() { mediaCarouselController.onDesiredLocationChanged( @@ -457,7 +446,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logCarouselPosition(MediaHierarchyManager.LOCATION_QQS) } - @Ignore("b/253229241") @Test fun testLocationChangeLockscreen_logged() { mediaCarouselController.onDesiredLocationChanged( @@ -468,7 +456,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logCarouselPosition(MediaHierarchyManager.LOCATION_LOCKSCREEN) } - @Ignore("b/253229241") @Test fun testLocationChangeDream_logged() { mediaCarouselController.onDesiredLocationChanged( @@ -479,7 +466,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logCarouselPosition(MediaHierarchyManager.LOCATION_DREAM_OVERLAY) } - @Ignore("b/253229241") @Test fun testRecommendationRemoved_logged() { val packageName = "smartspace package" @@ -493,7 +479,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(logger).logRecommendationRemoved(eq(packageName), eq(instanceId!!)) } - @Ignore("b/253229241") @Test fun testMediaLoaded_ScrollToActivePlayer() { listener.value.onMediaDataLoaded( @@ -551,7 +536,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { ) } - @Ignore("b/253229241") @Test fun testMediaLoadedFromRecommendationCard_ScrollToActivePlayer() { listener.value.onSmartspaceMediaDataLoaded( @@ -595,7 +579,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertEquals(playerIndex, 0) } - @Ignore("b/253229241") @Test fun testRecommendationRemovedWhileNotVisible_updateHostVisibility() { var result = false @@ -607,7 +590,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertEquals(true, result) } - @Ignore("b/253229241") @Test fun testRecommendationRemovedWhileVisible_thenReorders_updateHostVisibility() { var result = false @@ -621,7 +603,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertEquals(true, result) } - @Ignore("b/253229241") @Test fun testGetCurrentVisibleMediaContentIntent() { val clickIntent1 = mock(PendingIntent::class.java) @@ -668,7 +649,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { assertEquals(mediaCarouselController.getCurrentVisibleMediaContentIntent(), clickIntent2) } - @Ignore("b/253229241") @Test fun testSetCurrentState_UpdatePageIndicatorAlphaWhenSquish() { val delta = 0.0001F @@ -690,7 +670,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(pageIndicator).alpha = floatThat { abs(it - 1.0F) < delta } } - @Ignore("b/253229241") @Test fun testOnConfigChanged_playersAreAddedBack() { listener.value.onMediaDataLoaded( @@ -716,7 +695,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { val playersSize = MediaPlayerData.players().size - configListener.value.onConfigChanged(capture(newConfig)) + configListener.value.onConfigChanged(Configuration()) assertEquals(playersSize, MediaPlayerData.players().size) assertEquals(