From 2e81186a332e1dc7c046b0916b60181457354798 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 5 May 2020 13:54:00 +0100 Subject: [PATCH] Add timeout to TestExoPlayer runUntil methods. If the condition isn't fulfilled, they currently block until the test runner times out the test. Our usual approach is to timeout in the test itself so that the error message is clearly showing the blocked condition. Also clean-up some documentation. PiperOrigin-RevId: 309930198 --- .../android/exoplayer2/ExoPlayerTest.java | 5 +- .../exoplayer2/testutil/TestExoPlayer.java | 155 ++++++++++++------ .../testutil/TestExoPlayerTest.java | 75 +++++++++ 3 files changed, 187 insertions(+), 48 deletions(-) create mode 100644 testutils/src/test/java/com/google/android/exoplayer2/testutil/TestExoPlayerTest.java diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index 85c537ad653..770416bb4c6 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -3641,7 +3641,8 @@ public boolean shouldStartPlayback( @Test public void - nextLoadPositionExceedingLoadControlMaxBuffer_whileCurrentLoadInProgress_doesNotThrowException() { + nextLoadPositionExceedingLoadControlMaxBuffer_whileCurrentLoadInProgress_doesNotThrowException() + throws Exception { long maxBufferUs = 2 * C.MICROS_PER_SECOND; LoadControl loadControlWithMaxBufferUs = new DefaultLoadControl() { @@ -3706,7 +3707,7 @@ public boolean isReady() { // Wait until the MediaSource is prepared, i.e. returned its timeline, and at least one // iteration of doSomeWork after this was run. - TestExoPlayer.runUntilTimelineChanged(player, /* expectedTimeline= */ null); + TestExoPlayer.runUntilTimelineChanged(player); TestExoPlayer.runUntilPendingCommandsAreFullyHandled(player); assertThat(player.getPlayerError()).isNull(); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestExoPlayer.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestExoPlayer.java index 9d431b7f676..a8672b703f0 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestExoPlayer.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestExoPlayer.java @@ -41,6 +41,7 @@ import com.google.android.exoplayer2.video.VideoListener; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -51,6 +52,12 @@ */ public class TestExoPlayer { + /** + * The default timeout applied when calling one of the {@code runUntil} methods. This timeout + * should be sufficient for any condition using a Robolectric test. + */ + public static final long DEFAULT_TIMEOUT_MS = 10_000; + /** Reflectively call Robolectric ShadowLooper#runOneTask. */ private static final Object shadowLooper; @@ -305,15 +312,19 @@ public SimpleExoPlayer build() { private TestExoPlayer() {} /** - * Run tasks of the main {@link Looper} until the {@code player}'s state reaches the {@code - * expectedState}. + * Runs tasks of the main {@link Looper} until {@link Player#getPlaybackState()} matches the + * expected state. + * + * @param player The {@link Player}. + * @param expectedState The expected {@link Player.State}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static void runUntilPlaybackState(Player player, @Player.State int expectedState) { + public static void runUntilPlaybackState(Player player, @Player.State int expectedState) + throws TimeoutException { verifyMainTestThread(player); if (player.getPlaybackState() == expectedState) { return; } - AtomicBoolean receivedExpectedState = new AtomicBoolean(false); Player.EventListener listener = new Player.EventListener() { @@ -325,21 +336,24 @@ public void onPlaybackStateChanged(int state) { } }; player.addListener(listener); - runUntil(() -> receivedExpectedState.get()); + runUntil(receivedExpectedState::get); player.removeListener(listener); } /** - * Run tasks of the main {@link Looper} until the {@code player} calls the {@link - * Player.EventListener#onPlaybackSpeedChanged} callback with that matches {@code - * expectedPlayWhenReady}. + * Runs tasks of the main {@link Looper} until {@link Player#getPlayWhenReady()} matches the + * expected value. + * + * @param player The {@link Player}. + * @param expectedPlayWhenReady The expected value for {@link Player#getPlayWhenReady()}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static void runUntilPlayWhenReady(Player player, boolean expectedPlayWhenReady) { + public static void runUntilPlayWhenReady(Player player, boolean expectedPlayWhenReady) + throws TimeoutException { verifyMainTestThread(player); if (player.getPlayWhenReady() == expectedPlayWhenReady) { return; } - AtomicBoolean receivedExpectedPlayWhenReady = new AtomicBoolean(false); Player.EventListener listener = new Player.EventListener() { @@ -352,34 +366,53 @@ public void onPlayWhenReadyChanged(boolean playWhenReady, int reason) { } }; player.addListener(listener); - runUntil(() -> receivedExpectedPlayWhenReady.get()); + runUntil(receivedExpectedPlayWhenReady::get); } /** - * Run tasks of the main {@link Looper} until the {@code player} calls the {@link - * Player.EventListener#onTimelineChanged} callback. + * Runs tasks of the main {@link Looper} until {@link Player#getCurrentTimeline()} matches the + * expected timeline. * * @param player The {@link Player}. - * @param expectedTimeline A specific {@link Timeline} to wait for, or null if any timeline is - * accepted. - * @return The received {@link Timeline}. + * @param expectedTimeline The expected {@link Timeline}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static Timeline runUntilTimelineChanged( - Player player, @Nullable Timeline expectedTimeline) { + public static void runUntilTimelineChanged(Player player, Timeline expectedTimeline) + throws TimeoutException { verifyMainTestThread(player); - - if (expectedTimeline != null && expectedTimeline.equals(player.getCurrentTimeline())) { - return expectedTimeline; + if (expectedTimeline.equals(player.getCurrentTimeline())) { + return; } + AtomicBoolean receivedExpectedTimeline = new AtomicBoolean(false); + Player.EventListener listener = + new Player.EventListener() { + @Override + public void onTimelineChanged(Timeline timeline, int reason) { + if (expectedTimeline.equals(timeline)) { + receivedExpectedTimeline.set(true); + } + player.removeListener(this); + } + }; + player.addListener(listener); + runUntil(receivedExpectedTimeline::get); + } + /** + * Runs tasks of the main {@link Looper} until a timeline change occurred. + * + * @param player The {@link Player}. + * @return The new {@link Timeline}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. + */ + public static Timeline runUntilTimelineChanged(Player player) throws TimeoutException { + verifyMainTestThread(player); AtomicReference receivedTimeline = new AtomicReference<>(); Player.EventListener listener = new Player.EventListener() { @Override public void onTimelineChanged(Timeline timeline, int reason) { - if (expectedTimeline == null || expectedTimeline.equals(timeline)) { - receivedTimeline.set(timeline); - } + receivedTimeline.set(timeline); player.removeListener(this); } }; @@ -389,12 +422,16 @@ public void onTimelineChanged(Timeline timeline, int reason) { } /** - * Run tasks of the main {@link Looper} until the {@code player} calls the {@link + * Runs tasks of the main {@link Looper} until a {@link * Player.EventListener#onPositionDiscontinuity} callback with the specified {@link - * Player.DiscontinuityReason}. + * Player.DiscontinuityReason} occurred. + * + * @param player The {@link Player}. + * @param expectedReason The expected {@link Player.DiscontinuityReason}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ public static void runUntilPositionDiscontinuity( - Player player, @Player.DiscontinuityReason int expectedReason) { + Player player, @Player.DiscontinuityReason int expectedReason) throws TimeoutException { AtomicBoolean receivedCallback = new AtomicBoolean(false); Player.EventListener listener = new Player.EventListener() { @@ -407,17 +444,17 @@ public void onPositionDiscontinuity(int reason) { } }; player.addListener(listener); - runUntil(() -> receivedCallback.get()); + runUntil(receivedCallback::get); } /** - * Run tasks of the main {@link Looper} until the {@code player} calls the {@link - * Player.EventListener#onPlayerError} callback. + * Runs tasks of the main {@link Looper} until a player error occurred. * * @param player The {@link Player}. - * @return The raised error. + * @return The raised {@link ExoPlaybackException}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static ExoPlaybackException runUntilError(Player player) { + public static ExoPlaybackException runUntilError(Player player) throws TimeoutException { verifyMainTestThread(player); AtomicReference receivedError = new AtomicReference<>(); Player.EventListener listener = @@ -434,10 +471,13 @@ public void onPlayerError(ExoPlaybackException error) { } /** - * Run tasks of the main {@link Looper} until the {@code player} calls the {@link - * com.google.android.exoplayer2.video.VideoRendererEventListener#onRenderedFirstFrame} callback. + * Runs tasks of the main {@link Looper} until the {@link VideoListener#onRenderedFirstFrame} + * callback has been called. + * + * @param player The {@link Player}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static void runUntilRenderedFirstFrame(SimpleExoPlayer player) { + public static void runUntilRenderedFirstFrame(SimpleExoPlayer player) throws TimeoutException { verifyMainTestThread(player); AtomicBoolean receivedCallback = new AtomicBoolean(false); VideoListener listener = @@ -449,14 +489,18 @@ public void onRenderedFirstFrame() { } }; player.addVideoListener(listener); - runUntil(() -> receivedCallback.get()); + runUntil(receivedCallback::get); } /** - * Runs tasks of the main {@link Looper} until the {@code player} handled all previously issued - * commands completely on the internal playback thread. + * Runs tasks of the main {@link Looper} until the player completely handled all previously issued + * commands on the internal playback thread. + * + * @param player The {@link Player}. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS default timeout} is exceeded. */ - public static void runUntilPendingCommandsAreFullyHandled(ExoPlayer player) { + public static void runUntilPendingCommandsAreFullyHandled(ExoPlayer player) + throws TimeoutException { verifyMainTestThread(player); // Send message to player that will arrive after all other pending commands. Thus, the message // execution on the app thread will also happen after all other pending command @@ -466,20 +510,39 @@ public static void runUntilPendingCommandsAreFullyHandled(ExoPlayer player) { .createMessage((type, data) -> receivedMessageCallback.set(true)) .setHandler(Util.createHandler()) .send(); - runUntil(() -> receivedMessageCallback.get()); + runUntil(receivedMessageCallback::get); } - /** Run tasks of the main {@link Looper} until the {@code condition} returns {@code true}. */ - public static void runUntil(Supplier condition) { - verifyMainTestThread(); + /** + * Runs tasks of the main {@link Looper} until the {@code condition} returns {@code true}. + * + * @param condition The condition. + * @throws TimeoutException If the {@link #DEFAULT_TIMEOUT_MS} is exceeded. + */ + public static void runUntil(Supplier condition) throws TimeoutException { + runUntil(condition, DEFAULT_TIMEOUT_MS, Clock.DEFAULT); + } + /** + * Runs tasks of the main {@link Looper} until the {@code condition} returns {@code true}. + * + * @param condition The condition. + * @param timeoutMs The timeout in milliseconds. + * @param clock The {@link Clock} to measure the timeout. + * @throws TimeoutException If the {@code timeoutMs timeout} is exceeded. + */ + public static void runUntil(Supplier condition, long timeoutMs, Clock clock) + throws TimeoutException { + verifyMainTestThread(); try { + long timeoutTimeMs = clock.currentTimeMillis() + timeoutMs; while (!condition.get()) { + if (clock.currentTimeMillis() >= timeoutTimeMs) { + throw new TimeoutException(); + } runOneTaskMethod.invoke(shadowLooper); } - } catch (IllegalAccessException e) { - throw new IllegalStateException(e); - } catch (InvocationTargetException e) { + } catch (IllegalAccessException | InvocationTargetException e) { throw new IllegalStateException(e); } } diff --git a/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestExoPlayerTest.java b/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestExoPlayerTest.java new file mode 100644 index 00000000000..3e182225623 --- /dev/null +++ b/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestExoPlayerTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2.testutil; + +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.atMost; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.util.Clock; +import com.google.android.exoplayer2.util.Supplier; +import java.util.concurrent.TimeoutException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.LooperMode; + +/** Unit test for {@link TestExoPlayer}. */ +@RunWith(AndroidJUnit4.class) +@LooperMode(LooperMode.Mode.PAUSED) +public final class TestExoPlayerTest { + + @Test + public void runUntil_withConditionAlreadyTrue_returnsImmediately() throws Exception { + Clock mockClock = mock(Clock.class); + + TestExoPlayer.runUntil(() -> true, /* timeoutMs= */ 0, mockClock); + + verify(mockClock, atMost(1)).currentTimeMillis(); + } + + @Test + public void runUntil_withConditionThatNeverBecomesTrue_timesOut() { + Clock mockClock = mock(Clock.class); + when(mockClock.currentTimeMillis()).thenReturn(0L, 41L, 42L); + + assertThrows( + TimeoutException.class, + () -> TestExoPlayer.runUntil(() -> false, /* timeoutMs= */ 42, mockClock)); + + verify(mockClock, times(3)).currentTimeMillis(); + } + + @SuppressWarnings("unchecked") + @Test + public void runUntil_whenConditionBecomesTrueAfterDelay_returnsWhenConditionBecomesTrue() + throws Exception { + Supplier mockCondition = mock(Supplier.class); + when(mockCondition.get()) + .thenReturn(false) + .thenReturn(false) + .thenReturn(false) + .thenReturn(false) + .thenReturn(true); + + TestExoPlayer.runUntil(mockCondition, /* timeoutMs= */ 5674, mock(Clock.class)); + + verify(mockCondition, times(5)).get(); + } +}