Commit 3acc85c2 by olly Committed by Oliver Woodman

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
parent 4a582b23
...@@ -312,7 +312,7 @@ import java.util.List; ...@@ -312,7 +312,7 @@ import java.util.List;
} }
@Override @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_TEST_NAME, streamName);
metricsLogger.logMetric(MetricsLogger.KEY_IS_CDD_LIMITED_RETRY, isCddLimitedRetry); metricsLogger.logMetric(MetricsLogger.KEY_IS_CDD_LIMITED_RETRY, isCddLimitedRetry);
metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_DROPPED_COUNT, metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_DROPPED_COUNT,
...@@ -324,7 +324,10 @@ import java.util.List; ...@@ -324,7 +324,10 @@ import java.util.List;
metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_RENDERED_COUNT, metricsLogger.logMetric(MetricsLogger.KEY_FRAMES_RENDERED_COUNT,
videoCounters.renderedOutputBufferCount); videoCounters.renderedOutputBufferCount);
metricsLogger.close(); metricsLogger.close();
}
@Override
protected void assertPassed(DecoderCounters audioCounters, DecoderCounters videoCounters) {
if (fullPlaybackNoSeeking) { if (fullPlaybackNoSeeking) {
// We shouldn't have skipped any output buffers. // We shouldn't have skipped any output buffers.
DecoderCountersUtil DecoderCountersUtil
......
...@@ -159,10 +159,10 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest { ...@@ -159,10 +159,10 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
@Override @Override
public final void onFinished() { public final void onFinished() {
onTestFinished(audioDecoderCounters, videoDecoderCounters);
if (failOnPlayerError && playerError != null) { if (failOnPlayerError && playerError != null) {
throw new Error(playerError); throw new Error(playerError);
} }
logMetrics(audioDecoderCounters, videoDecoderCounters);
if (expectedPlayingTimeMs != EXPECTED_PLAYING_TIME_UNSET) { if (expectedPlayingTimeMs != EXPECTED_PLAYING_TIME_UNSET) {
long playingTimeToAssertMs = expectedPlayingTimeMs == EXPECTED_PLAYING_TIME_MEDIA_DURATION_MS long playingTimeToAssertMs = expectedPlayingTimeMs == EXPECTED_PLAYING_TIME_MEDIA_DURATION_MS
? sourceDurationMs : expectedPlayingTimeMs; ? sourceDurationMs : expectedPlayingTimeMs;
...@@ -176,6 +176,8 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest { ...@@ -176,6 +176,8 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
&& totalPlayingTimeMs <= maxAllowedActualPlayingTimeMs) && totalPlayingTimeMs <= maxAllowedActualPlayingTimeMs)
.isTrue(); .isTrue();
} }
// Make any additional assertions.
assertPassed(audioDecoderCounters, videoDecoderCounters);
} }
// AnalyticsListener // AnalyticsListener
...@@ -259,8 +261,12 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest { ...@@ -259,8 +261,12 @@ public abstract class ExoHostedTest implements AnalyticsListener, HostedTest {
// Do nothing. Interested subclasses may override. // Do nothing. Interested subclasses may override.
} }
protected void onTestFinished(DecoderCounters audioCounters, DecoderCounters videoCounters) { protected void logMetrics(DecoderCounters audioCounters, DecoderCounters videoCounters) {
// Do nothing. Subclasses may override to add clean-up and assertions. // 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( @EnsuresNonNullIf(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment