From a00bef9e517f1f7da3c6c4523e4a8a0ae50cd284 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 22 Aug 2023 13:33:03 +0100 Subject: [PATCH] Fix MediaControllerSurfaceSizeChangeTest There are multiple problems in the test: 1. The controller must run on the main thread due to the Player interface requirement to add surfaces on the thread their callbacks will be called on. 2. This test made assertions about an expected callback that aren't true. Setting an anonymous surface will not result in a size change callback because the size stays unknown. But clearing a surface (even if never set) ensures the returned size is guaranteed to be zero. PiperOrigin-RevId: 559080905 --- .../MediaControllerSurfaceSizeChangeTest.java | 109 ++++++++---------- 1 file changed, 46 insertions(+), 63 deletions(-) diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerSurfaceSizeChangeTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerSurfaceSizeChangeTest.java index caa9297508..1da6f39524 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerSurfaceSizeChangeTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerSurfaceSizeChangeTest.java @@ -22,6 +22,7 @@ import static androidx.media3.test.session.common.TestUtils.TIMEOUT_MS; import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import android.os.Looper; import android.os.RemoteException; import android.view.Surface; import android.view.SurfaceHolder; @@ -32,14 +33,14 @@ import android.view.ViewGroup.LayoutParams; import androidx.annotation.Nullable; import androidx.media3.common.C; import androidx.media3.common.Player; -import androidx.media3.test.session.common.HandlerThreadTestRule; +import androidx.media3.test.session.common.MainLooperTestRule; import androidx.media3.test.session.common.PollingCheck; import androidx.media3.test.session.common.SurfaceActivity; import androidx.media3.test.session.common.TestUtils; import androidx.test.core.app.ApplicationProvider; import androidx.test.filters.LargeTest; import androidx.test.rule.ActivityTestRule; -import java.util.Arrays; +import com.google.common.collect.ImmutableList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; @@ -48,8 +49,6 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.RuleChain; -import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -61,8 +60,6 @@ import org.junit.runners.Parameterized; @LargeTest public class MediaControllerSurfaceSizeChangeTest { - private static final String TAG = "MCSurfaceSizeChangeTest"; - private static final String SET_VIDEO_SURFACE = "setVideoSurface"; private static final String SET_VIDEO_SURFACE_HOLDER = "setVideoSurfaceHolder"; private static final String SET_VIDEO_SURFACE_VIEW = "setVideoSurfaceView"; @@ -81,11 +78,11 @@ public class MediaControllerSurfaceSizeChangeTest { @Parameterized.Parameters( name = "action:{0}, precondition:{1}, sizeCondition:{2}, callbackShouldBeCalled:{3}") public static List params() { - return Arrays.asList( + return ImmutableList.of( // Major use cases new Object[] {SET_VIDEO_SURFACE, SET_VIDEO_SURFACE, SIZE_SAME, false}, new Object[] {SET_VIDEO_SURFACE, SET_VIDEO_SURFACE, SIZE_DIFFERENT, false}, - new Object[] {SET_VIDEO_SURFACE, NO_PRECONDITION, SIZE_NOT_APPLICABLE, true}, + new Object[] {SET_VIDEO_SURFACE, NO_PRECONDITION, SIZE_NOT_APPLICABLE, false}, new Object[] {SET_VIDEO_SURFACE_HOLDER, SET_VIDEO_SURFACE_HOLDER, SIZE_SAME, false}, new Object[] {SET_VIDEO_SURFACE_HOLDER, SET_VIDEO_SURFACE_HOLDER, SIZE_DIFFERENT, true}, new Object[] {SET_VIDEO_SURFACE_HOLDER, NO_PRECONDITION, SIZE_NOT_APPLICABLE, true}, @@ -125,15 +122,10 @@ public class MediaControllerSurfaceSizeChangeTest { new Object[] {SET_VIDEO_TEXTURE_VIEW, SET_VIDEO_SURFACE_HOLDER, SIZE_DIFFERENT, true}, new Object[] {SET_VIDEO_TEXTURE_VIEW, SET_VIDEO_SURFACE_VIEW, SIZE_SAME, false}, new Object[] {SET_VIDEO_TEXTURE_VIEW, SET_VIDEO_SURFACE_VIEW, SIZE_DIFFERENT, true}, - new Object[] {CLEAR_VIDEO_SURFACE, NO_PRECONDITION, SIZE_NOT_APPLICABLE, false}); + new Object[] {CLEAR_VIDEO_SURFACE, NO_PRECONDITION, SIZE_NOT_APPLICABLE, true}); } - private final HandlerThreadTestRule threadTestRule = new HandlerThreadTestRule(TAG); - private final MediaControllerTestRule controllerTestRule = - new MediaControllerTestRule(threadTestRule); - - @Rule - public final TestRule chain = RuleChain.outerRule(threadTestRule).around(controllerTestRule); + @Rule public final MainLooperTestRule mainLooperTestRule = new MainLooperTestRule(); private final String action; private final String precondition; @@ -150,7 +142,6 @@ public class MediaControllerSurfaceSizeChangeTest { private SurfaceActivity activity; private RemoteMediaSession remoteSession; - private MediaController controller; @Nullable private View viewForPrecondition; @Nullable private View viewForAction; @@ -163,7 +154,6 @@ public class MediaControllerSurfaceSizeChangeTest { remoteSession = new RemoteMediaSession( DEFAULT_TEST_NAME, ApplicationProvider.getApplicationContext(), null); - controller = controllerTestRule.createController(remoteSession.getToken()); } @After @@ -189,49 +179,49 @@ public class MediaControllerSurfaceSizeChangeTest { @Test public void test() throws Throwable { - setPrecondition(); + MediaController controller = + new MediaController.Builder(activity, remoteSession.getToken()) + .setApplicationLooper(Looper.getMainLooper()) + .buildAsync() + .get(); + setPrecondition(controller); setViewForAction(); matchSizeOfViewForAction(); setExpectedWidthAndHeightFromCallback(); - doAction(); + doAction(controller); waitCallbackAndAssert(); } - private void setPrecondition() throws Exception { + private void setPrecondition(MediaController controller) throws Exception { switch (precondition) { case SET_VIDEO_SURFACE: { Surface testSurface = activity.getFirstSurfaceHolder().getSurface(); - threadTestRule.getHandler().postAndSync(() -> controller.setVideoSurface(testSurface)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoSurface(testSurface)); viewForPrecondition = activity.getFirstSurfaceView(); break; } case SET_VIDEO_SURFACE_HOLDER: { SurfaceHolder testSurfaceHolder = activity.getFirstSurfaceHolder(); - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoSurfaceHolder(testSurfaceHolder)); + MainLooperTestRule.runOnMainSync( + () -> controller.setVideoSurfaceHolder(testSurfaceHolder)); viewForPrecondition = activity.getFirstSurfaceView(); break; } case SET_VIDEO_SURFACE_VIEW: { SurfaceView testSurfaceView = activity.getFirstSurfaceView(); - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoSurfaceView(testSurfaceView)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoSurfaceView(testSurfaceView)); viewForPrecondition = activity.getFirstSurfaceView(); break; } case SET_VIDEO_TEXTURE_VIEW: { TextureView testTextureView = activity.getFirstTextureView(); - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoTextureView(testTextureView)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoTextureView(testTextureView)); viewForPrecondition = activity.getFirstTextureView(); break; } @@ -241,28 +231,26 @@ public class MediaControllerSurfaceSizeChangeTest { default: throw new AssertionError(precondition + " is not an allowed precondition."); } - threadTestRule - .getHandler() - .postAndSync( - () -> - controller.addListener( - new Player.Listener() { - @Override - public void onSurfaceSizeChanged(int width, int height) { - if (countDownLatch.getCount() == 0) { - return; - } - newSurfaceWidthRef.set(width); - newSurfaceHeightRef.set(height); - countDownLatch.countDown(); - } + MainLooperTestRule.runOnMainSync( + () -> + controller.addListener( + new Player.Listener() { + @Override + public void onSurfaceSizeChanged(int width, int height) { + if (countDownLatch.getCount() == 0) { + return; + } + newSurfaceWidthRef.set(width); + newSurfaceHeightRef.set(height); + countDownLatch.countDown(); + } - @Override - public void onEvents(Player player, Player.Events events) { - eventsRef.set(events); - countDownLatch.countDown(); - } - })); + @Override + public void onEvents(Player player, Player.Events events) { + eventsRef.set(events); + countDownLatch.countDown(); + } + })); } private void setViewForAction() { @@ -321,43 +309,38 @@ public class MediaControllerSurfaceSizeChangeTest { } } - private void doAction() throws Exception { + private void doAction(MediaController controller) throws Exception { switch (action) { case SET_VIDEO_SURFACE: { SurfaceView testSurfaceView = (SurfaceView) viewForAction; Surface surface = testSurfaceView.getHolder().getSurface(); - threadTestRule.getHandler().postAndSync(() -> controller.setVideoSurface(surface)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoSurface(surface)); break; } case SET_VIDEO_SURFACE_HOLDER: { SurfaceView testSurfaceView = (SurfaceView) viewForAction; SurfaceHolder testSurfaceHolder = testSurfaceView.getHolder(); - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoSurfaceHolder(testSurfaceHolder)); + MainLooperTestRule.runOnMainSync( + () -> controller.setVideoSurfaceHolder(testSurfaceHolder)); break; } case SET_VIDEO_SURFACE_VIEW: { SurfaceView testSurfaceView = (SurfaceView) viewForAction; - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoSurfaceView(testSurfaceView)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoSurfaceView(testSurfaceView)); break; } case SET_VIDEO_TEXTURE_VIEW: { TextureView testTextureView = (TextureView) viewForAction; - threadTestRule - .getHandler() - .postAndSync(() -> controller.setVideoTextureView(testTextureView)); + MainLooperTestRule.runOnMainSync(() -> controller.setVideoTextureView(testTextureView)); break; } case CLEAR_VIDEO_SURFACE: { - threadTestRule.getHandler().postAndSync(() -> controller.clearVideoSurface()); + MainLooperTestRule.runOnMainSync(controller::clearVideoSurface); break; } default: