From 3acc85c2dfdaa9cfc257b1b1f00575bfad0814de Mon Sep 17 00:00:00 2001
From: olly <olly@google.com>
Date: Fri, 13 Mar 2020 11:44:15 +0000
Subject: [PATCH] Re-split ExoHostedTest.onTestFinished into logMetrics and
 assertPassed

This is a partial revert of <unknown commit>. The split and the order is important. For
example, as things are currently, if playback fails for a test that makes additional
assertions in onTestFinished, the visible reason the test failed is quite likely to
be one of the additional assertions, rather than the error that actually caused the
playback to fail. I think we probably also don't want to log metrics if playback
fails.

PiperOrigin-RevId: 300733109
---
 .../exoplayer2/playbacktests/gts/DashTestRunner.java |  5 ++++-
 .../android/exoplayer2/testutil/ExoHostedTest.java   | 12 +++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/playbacktests/src/androidTest/java/com/google/android/exoplayer2/playbacktests/gts/DashTestRunner.java b/playbacktests/src/androidTest/java/com/google/android/exoplayer2/playbacktests/gts/DashTestRunner.java
index 6bfef1d1b4..b699fc0469 100644
--- a/playbacktests/src/androidTest/java/com/google/android/exoplayer2/playbacktests/gts/DashTestRunner.java
+++ b/playbacktests/src/androidTest/java/com/google/android/exoplayer2/playbacktests/gts/DashTestRunner.java
@@ -312,7 +312,7 @@ import java.util.List;
     }
 
     @Override
-    protected void onTestFinished(DecoderCounters audioCounters, DecoderCounters videoCounters) {
+    protected void logMetrics(DecoderCounters audioCounters, DecoderCounters videoCounters) {
       metricsLogger.logMetric(MetricsLogger.KEY_TEST_NAME, streamName);
       metricsLogger.logMetric(MetricsLogger.KEY_IS_CDD_LIMITED_RETRY, isCddLimitedRetry);
       metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_DROPPED_COUNT,
@@ -324,7 +324,10 @@ import java.util.List;
       metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_RENDERED_COUNT,
           videoCounters.renderedOutputBufferCount);
       metricsLogger.close();
+    }
 
+    @Override
+    protected void assertPassed(DecoderCounters audioCounters, DecoderCounters videoCounters) {
       if (fullPlaybackNoSeeking) {
         // We shouldn't have skipped any output buffers.
         DecoderCountersUtil
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 a0a9f0c0c7..f95d13ba33 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
@@ -159,10 +159,10 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
 
   @Override
   public final void onFinished() {
-    onTestFinished(audioDecoderCounters, videoDecoderCounters);
     if (failOnPlayerError && playerError != null) {
       throw new Error(playerError);
     }
+    logMetrics(audioDecoderCounters, videoDecoderCounters);
     if (expectedPlayingTimeMs != EXPECTED_PLAYING_TIME_UNSET) {
       long playingTimeToAssertMs = expectedPlayingTimeMs == EXPECTED_PLAYING_TIME_MEDIA_DURATION_MS
           ? sourceDurationMs : expectedPlayingTimeMs;
@@ -176,6 +176,8 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
                   && totalPlayingTimeMs <= maxAllowedActualPlayingTimeMs)
           .isTrue();
     }
+    // Make any additional assertions.
+    assertPassed(audioDecoderCounters, videoDecoderCounters);
   }
 
   // AnalyticsListener
@@ -259,8 +261,12 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
     // Do nothing. Interested subclasses may override.
   }
 
-  protected void onTestFinished(DecoderCounters audioCounters, DecoderCounters videoCounters) {
-    // Do nothing. Subclasses may override to add clean-up and assertions.
+  protected void logMetrics(DecoderCounters audioCounters, DecoderCounters videoCounters) {
+    // Do nothing. Subclasses may override to log metrics.
+  }
+
+  protected void assertPassed(DecoderCounters audioCounters, DecoderCounters videoCounters) {
+    // Do nothing. Subclasses may override to add additional assertions.
   }
 
   @EnsuresNonNullIf(