From c4385c738f12986702cf94ff6330ae2768bdae7b Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 24 Nov 2017 08:30:53 -0800 Subject: [PATCH] Temporarily fix flakiness of testPlayEmptyTimeline. Fixed by explicitly waiting for the timeline update. This shouldn't be necessary and will be removed as soon as the correct order of events can be guaranteed (timeline change -> state change -> onSeekProcessed). The waiting for the timeline update is implemented by introducing the feature that the test runner also waits until the action schedule has finished before stopping the test. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=176848540 --- .../android/exoplayer2/ExoPlayerTest.java | 9 ++- .../exoplayer2/testutil/ActionSchedule.java | 52 +++++++++++++++++- .../exoplayer2/testutil/ExoHostedTest.java | 4 +- .../testutil/ExoPlayerTestRunner.java | 55 ++++++++++++++++--- 4 files changed, 106 insertions(+), 14 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java index 5f41e57a6a..8213e6133d 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -56,8 +56,15 @@ public final class ExoPlayerTest extends TestCase { public void testPlayEmptyTimeline() throws Exception { Timeline timeline = Timeline.EMPTY; FakeRenderer renderer = new FakeRenderer(); + // TODO(b/69665207): Without waiting for the timeline update, this test is flaky as the timeline + // update happens after the transition to STATE_ENDED and the test runner may already have been + // stopped. Remove action schedule as soon as state changes are part of the masking and the + // correct order of events is restored. + ActionSchedule actionSchedule = new ActionSchedule.Builder("testPlayEmptyTimeline") + .waitForTimelineChanged(timeline) + .build(); ExoPlayerTestRunner testRunner = new ExoPlayerTestRunner.Builder() - .setTimeline(timeline).setRenderers(renderer) + .setTimeline(timeline).setRenderers(renderer).setActionSchedule(actionSchedule) .build().start().blockUntilEnded(TIMEOUT_MS); testRunner.assertNoPositionDiscontinuities(); testRunner.assertTimelinesEqual(timeline); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java index 2dbb4e18d2..5e3d6bcb9a 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.testutil; import android.os.Handler; import android.os.Looper; +import android.support.annotation.Nullable; import android.view.Surface; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.PlaybackParameters; @@ -47,13 +48,28 @@ import com.google.android.exoplayer2.util.Clock; */ public final class ActionSchedule { + /** + * Callback to notify listener that the action schedule has finished. + */ + public interface Callback { + + /** + * Called when action schedule finished executing all its actions. + */ + void onActionScheduleFinished(); + + } + private final ActionNode rootNode; + private final CallbackAction callbackAction; /** * @param rootNode The first node in the sequence. + * @param callbackAction The final action which can be used to trigger a callback. */ - private ActionSchedule(ActionNode rootNode) { + private ActionSchedule(ActionNode rootNode, CallbackAction callbackAction) { this.rootNode = rootNode; + this.callbackAction = callbackAction; } /** @@ -63,9 +79,12 @@ public final class ActionSchedule { * @param trackSelector The track selector to which actions should be applied. * @param surface The surface to use when applying actions. * @param mainHandler A handler associated with the main thread of the host activity. + * @param callback A {@link Callback} to notify when the action schedule finishes, or null if no + * notification is needed. */ /* package */ void start(SimpleExoPlayer player, MappingTrackSelector trackSelector, - Surface surface, Handler mainHandler) { + Surface surface, Handler mainHandler, @Nullable Callback callback) { + callbackAction.setCallback(callback); rootNode.schedule(player, trackSelector, surface, mainHandler); } @@ -304,7 +323,9 @@ public final class ActionSchedule { } public ActionSchedule build() { - return new ActionSchedule(rootNode); + CallbackAction callbackAction = new CallbackAction(tag); + apply(callbackAction); + return new ActionSchedule(rootNode, callbackAction); } private Builder appendActionNode(ActionNode actionNode) { @@ -420,4 +441,29 @@ public final class ActionSchedule { } + /** + * An action calling a specified {@link ActionSchedule.Callback}. + */ + private static final class CallbackAction extends Action { + + private @Nullable Callback callback; + + public CallbackAction(String tag) { + super(tag, "FinishedCallback"); + } + + public void setCallback(@Nullable Callback callback) { + this.callback = callback; + } + + @Override + protected void doActionImpl(SimpleExoPlayer player, MappingTrackSelector trackSelector, + Surface surface) { + if (callback != null) { + callback.onActionScheduleFinished(); + } + } + + } + } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoHostedTest.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoHostedTest.java index ee4018ba0e..ab31238983 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoHostedTest.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoHostedTest.java @@ -126,7 +126,7 @@ public abstract class ExoHostedTest extends Player.DefaultEventListener implemen if (player == null) { pendingSchedule = schedule; } else { - schedule.start(player, trackSelector, surface, actionHandler); + schedule.start(player, trackSelector, surface, actionHandler, /* callback= */ null); } } @@ -162,7 +162,7 @@ public abstract class ExoHostedTest extends Player.DefaultEventListener implemen actionHandler = new Handler(); // Schedule any pending actions. if (pendingSchedule != null) { - pendingSchedule.start(player, trackSelector, surface, actionHandler); + pendingSchedule.start(player, trackSelector, surface, actionHandler, /* callback= */ null); pendingSchedule = null; } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java index 759af41039..7b3292db89 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java @@ -17,6 +17,8 @@ package com.google.android.exoplayer2.testutil; import android.os.Handler; import android.os.HandlerThread; +import android.os.SystemClock; +import android.support.annotation.Nullable; import com.google.android.exoplayer2.DefaultLoadControl; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.ExoPlayerFactory; @@ -48,7 +50,8 @@ import junit.framework.Assert; /** * Helper class to run an ExoPlayer test. */ -public final class ExoPlayerTestRunner extends Player.DefaultEventListener { +public final class ExoPlayerTestRunner extends Player.DefaultEventListener + implements ActionSchedule.Callback { /** * Builder to set-up a {@link ExoPlayerTestRunner}. Default fake implementations will be used for @@ -327,12 +330,13 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { private final RenderersFactory renderersFactory; private final MappingTrackSelector trackSelector; private final LoadControl loadControl; - private final ActionSchedule actionSchedule; - private final Player.EventListener eventListener; + private final @Nullable ActionSchedule actionSchedule; + private final @Nullable Player.EventListener eventListener; private final HandlerThread playerThread; private final Handler handler; private final CountDownLatch endedCountDownLatch; + private final CountDownLatch actionScheduleFinishedCountDownLatch; private final ArrayList timelines; private final ArrayList manifests; private final ArrayList timelineChangeReasons; @@ -346,8 +350,8 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { private ExoPlayerTestRunner(PlayerFactory playerFactory, MediaSource mediaSource, RenderersFactory renderersFactory, MappingTrackSelector trackSelector, - LoadControl loadControl, ActionSchedule actionSchedule, Player.EventListener eventListener, - int expectedPlayerEndedCount) { + LoadControl loadControl, @Nullable ActionSchedule actionSchedule, + @Nullable Player.EventListener eventListener, int expectedPlayerEndedCount) { this.playerFactory = playerFactory; this.mediaSource = mediaSource; this.renderersFactory = renderersFactory; @@ -361,6 +365,7 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { this.periodIndices = new ArrayList<>(); this.discontinuityReasons = new ArrayList<>(); this.endedCountDownLatch = new CountDownLatch(expectedPlayerEndedCount); + this.actionScheduleFinishedCountDownLatch = new CountDownLatch(actionSchedule != null ? 1 : 0); this.playerThread = new HandlerThread("ExoPlayerTest thread"); playerThread.start(); this.handler = new Handler(playerThread.getLooper()); @@ -387,7 +392,7 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { } player.setPlayWhenReady(true); if (actionSchedule != null) { - actionSchedule.start(player, trackSelector, null, handler); + actionSchedule.start(player, trackSelector, null, handler, ExoPlayerTestRunner.this); } player.prepare(mediaSource); } catch (Exception e) { @@ -400,8 +405,9 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { /** * Blocks the current thread until the test runner finishes. A test is deemed to be finished when - * the playback state transitions to {@link Player#STATE_ENDED} or {@link Player#STATE_IDLE}, or - * when am {@link ExoPlaybackException} is thrown. + * the action schedule finished and the playback state transitioned to {@link Player#STATE_ENDED} + * or {@link Player#STATE_IDLE} for the specified number of times. The test also finishes when an + * {@link ExoPlaybackException} is thrown. * * @param timeoutMs The maximum time to wait for the test runner to finish. If this time elapsed * the method will throw a {@link TimeoutException}. @@ -409,6 +415,13 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { * @throws Exception If any exception occurred during playback, release, or due to a timeout. */ public ExoPlayerTestRunner blockUntilEnded(long timeoutMs) throws Exception { + long deadlineMs = SystemClock.elapsedRealtime() + timeoutMs; + try { + blockUntilActionScheduleFinished(timeoutMs); + } catch (TimeoutException error) { + exception = error; + } + timeoutMs = Math.max(0, deadlineMs - SystemClock.elapsedRealtime()); if (!endedCountDownLatch.await(timeoutMs, TimeUnit.MILLISECONDS)) { exception = new TimeoutException("Test playback timed out waiting for playback to end."); } @@ -420,6 +433,24 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { return this; } + /** + * Blocks the current thread until the action schedule finished. Also returns when an + * {@link ExoPlaybackException} is thrown. This does not release the test runner and the test must + * still call {@link #blockUntilEnded(long)}. + * + * @param timeoutMs The maximum time to wait for the action schedule to finish. + * @return This test runner. + * @throws TimeoutException If the action schedule did not finish within the specified timeout. + * @throws InterruptedException If the test thread gets interrupted while waiting. + */ + public ExoPlayerTestRunner blockUntilActionScheduleFinished(long timeoutMs) + throws TimeoutException, InterruptedException { + if (!actionScheduleFinishedCountDownLatch.await(timeoutMs, TimeUnit.MILLISECONDS)) { + throw new TimeoutException("Test playback timed out waiting for action schedule to finish."); + } + return this; + } + // Assertions called on the test thread after test finished. /** @@ -536,6 +567,7 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { while (endedCountDownLatch.getCount() > 0) { endedCountDownLatch.countDown(); } + actionScheduleFinishedCountDownLatch.countDown(); } // Player.EventListener @@ -582,4 +614,11 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener { } } + // ActionSchedule.Callback + + @Override + public void onActionScheduleFinished() { + actionScheduleFinishedCountDownLatch.countDown(); + } + }