From bf43cca302153e1920198fd3f4986af81f5a072f Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 22 Aug 2018 03:55:24 -0700 Subject: [PATCH] Fix flaky ExoPlayerTest tests. Some tests were flaky because of the PlayUntilPosition action which called player.setPlayWhenReady from the wrong thread. Also fixed some other misc flakiness. ExoPlayerTest seems to be non-flaky now (tested with 10000 runs). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=209743076 --- .../android/exoplayer2/ExoPlayerTest.java | 31 +++++++------------ .../android/exoplayer2/testutil/Action.java | 21 +++++++++++-- 2 files changed, 30 insertions(+), 22 deletions(-) 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 6409c1b604..8846e31917 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 @@ -29,7 +29,6 @@ import com.google.android.exoplayer2.source.ConcatenatingMediaSource; import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.source.MediaSourceEventListener.EventDispatcher; -import com.google.android.exoplayer2.source.ShuffleOrder; import com.google.android.exoplayer2.source.TrackGroup; import com.google.android.exoplayer2.source.TrackGroupArray; import com.google.android.exoplayer2.source.ads.AdPlaybackState; @@ -1290,12 +1289,12 @@ public final class ExoPlayerTest { @Test public void testPlaybackErrorDuringSourceInfoRefreshWithShuffleModeEnabledUsesCorrectFirstPeriod() throws Exception { - Timeline timeline = new FakeTimeline(/* windowCount= */ 1); - FakeMediaSource mediaSource = new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); + FakeMediaSource mediaSource = + new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), /* manifest= */ null); ConcatenatingMediaSource concatenatingMediaSource = new ConcatenatingMediaSource( /* isAtomic= */ false, new FakeShuffleOrder(0), mediaSource, mediaSource); - AtomicInteger windowIndexAfterReprepare = new AtomicInteger(); + AtomicInteger windowIndexAfterError = new AtomicInteger(); ActionSchedule actionSchedule = new ActionSchedule.Builder("testPlaybackErrorDuringSourceInfoRefreshUsesCorrectFirstPeriod") .setShuffleModeEnabled(true) @@ -1304,17 +1303,12 @@ public final class ExoPlayerTest { // is still being prepared. The error will be thrown while the player handles the new // source info. .seek(/* windowIndex= */ 100, /* positionMs= */ 0) - .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null)) .waitForPlaybackState(Player.STATE_IDLE) - // Re-prepare to play the source in its default shuffled order. - .prepareSource( - concatenatingMediaSource, /* resetPosition= */ false, /* resetState= */ false) - .waitForTimelineChanged(null) .executeRunnable( new PlayerRunnable() { @Override public void run(SimpleExoPlayer player) { - windowIndexAfterReprepare.set(player.getCurrentWindowIndex()); + windowIndexAfterError.set(player.getCurrentWindowIndex()); } }) .build(); @@ -1330,7 +1324,7 @@ public final class ExoPlayerTest { // Expected exception. assertThat(e.getUnexpectedException()).isInstanceOf(IllegalSeekPositionException.class); } - assertThat(windowIndexAfterReprepare.get()).isEqualTo(1); + assertThat(windowIndexAfterError.get()).isEqualTo(1); } @Test @@ -2253,21 +2247,20 @@ public final class ExoPlayerTest { @Test public void testUpdateTrackSelectorThenSeekToUnpreparedPeriod_returnsEmptyTrackGroups() throws Exception { - Timeline fakeTimeline = new FakeTimeline(/* windowCount= */ 1); + // Use unset duration to prevent pre-loading of the second window. + Timeline fakeTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ C.TIME_UNSET)); MediaSource[] fakeMediaSources = { new FakeMediaSource(fakeTimeline, null, Builder.VIDEO_FORMAT), new FakeMediaSource(fakeTimeline, null, Builder.AUDIO_FORMAT) }; - MediaSource mediaSource = - new ConcatenatingMediaSource( - /* isAtomic= */ false, - /* useLazyPreparation= */ true, - new ShuffleOrder.DefaultShuffleOrder(0), - fakeMediaSources); + MediaSource mediaSource = new ConcatenatingMediaSource(fakeMediaSources); FakeRenderer renderer = new FakeRenderer(Builder.VIDEO_FORMAT); DefaultTrackSelector trackSelector = new DefaultTrackSelector(); ActionSchedule actionSchedule = - new ActionSchedule.Builder("testSendMessages") + new ActionSchedule.Builder("testUpdateTrackSelectorThenSeekToUnpreparedPeriod") .pause() .waitForPlaybackState(Player.STATE_READY) .seek(/* windowIndex= */ 1, /* positionMs= */ 0) diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java index 9e7d583894..f06fcc3add 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java @@ -34,6 +34,7 @@ import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerRunnable; import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerTarget; import com.google.android.exoplayer2.trackselection.DefaultTrackSelector; import com.google.android.exoplayer2.trackselection.DefaultTrackSelector.Parameters; +import com.google.android.exoplayer2.util.ConditionVariable; import com.google.android.exoplayer2.util.HandlerWrapper; /** @@ -503,10 +504,24 @@ public abstract class Action { final Surface surface, final HandlerWrapper handler, final ActionNode nextAction) { - // Schedule one message on the playback thread to pause the player immediately. + Handler testThreadHandler = new Handler(); + // Schedule a message on the playback thread to ensure the player is paused immediately. player .createMessage( - (messageType, payload) -> player.setPlayWhenReady(/* playWhenReady= */ false)) + (messageType, payload) -> { + // Block playback thread until pause command has been sent from test thread. + ConditionVariable blockPlaybackThreadCondition = new ConditionVariable(); + testThreadHandler.post( + () -> { + player.setPlayWhenReady(/* playWhenReady= */ false); + blockPlaybackThreadCondition.open(); + }); + try { + blockPlaybackThreadCondition.block(); + } catch (InterruptedException e) { + // Ignore. + } + }) .setPosition(windowIndex, positionMs) .send(); // Schedule another message on this test thread to continue action schedule. @@ -515,7 +530,7 @@ public abstract class Action { (messageType, payload) -> nextAction.schedule(player, trackSelector, surface, handler)) .setPosition(windowIndex, positionMs) - .setHandler(new Handler()) + .setHandler(testThreadHandler) .send(); player.setPlayWhenReady(true); }