From 2a93eff25ac42458a3eecdd0d716c4ca50bb36a1 Mon Sep 17 00:00:00 2001 From: ibaker Date: Thu, 10 Dec 2020 12:36:06 +0000 Subject: [PATCH] Attach resource info to exceptions thrown in DataSourceContractTest Before this we added the resource info to Truth assertions, but the same info is missing if an exception bubbles out of the SUT. PiperOrigin-RevId: 346757960 --- .../testutil/AdditionalFailureInfo.java | 111 +++++++++++ .../testutil/DataSourceContractTest.java | 16 +- .../testutil/AdditionalFailureInfoTest.java | 174 ++++++++++++++++++ 3 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 testutils/src/main/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfo.java create mode 100644 testutils/src/test/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfoTest.java diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfo.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfo.java new file mode 100644 index 0000000000..7818255075 --- /dev/null +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfo.java @@ -0,0 +1,111 @@ +/* + * 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 androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; +import com.google.android.exoplayer2.util.Util; +import com.google.common.truth.Truth; +import java.util.concurrent.atomic.AtomicReference; +import org.checkerframework.checker.nullness.compatqual.NullableType; +import org.junit.Rule; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * A JUnit {@link Rule} that attaches additional info to any errors/exceptions thrown by the test. + * + *

This is useful for tests where the line-number from a stacktrace doesn't provide enough detail + * about the failure, for example when an assertion fails inside a loop. + * + *

This can be preferable to many calls to {@link Truth#assertWithMessage(String)} because it + * will also add info to errors/exceptions that bubble out from the system-under-test. + * + *

Includes special handling for {@link AssertionError} to ensure that test failures are + * correctly distinguished from test errors (all other errors/exceptions). + */ +@RequiresApi(19) +public final class AdditionalFailureInfo implements TestRule { + + private final AtomicReference<@NullableType String> info; + + public AdditionalFailureInfo() { + info = new AtomicReference<>(); + } + + /** + * Set the additional info to be added to any test failures. Pass {@code null} to skip adding any + * additional info. + * + *

Can be called from any thread. + */ + public void setInfo(@Nullable String info) { + this.info.set(info); + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + try { + base.evaluate(); + } catch (Throwable e) { + @Nullable String resourceInfo = AdditionalFailureInfo.this.info.get(); + if (resourceInfo == null) { + throw e; + } else if (e instanceof AssertionError) { + // Deliberately prune the AssertionError from the causal chain and "replace" it with + // this new one by copying the stacktrace from old to new (so it looks like the new + // one was thrown from where the old one was thrown). + DiscreteAssertionError assertionError = + new DiscreteAssertionError(resourceInfo + "\n" + e.getMessage(), e.getCause()); + assertionError.setStackTrace(e.getStackTrace()); + throw assertionError; + } else { + Exception exception = new Exception(resourceInfo, e); + StackTraceElement[] stackTrace = exception.getStackTrace(); + // Only include the top line of the stack trace (this method) - the rest will be + // uninteresting JUnit framework calls that obscure the true test failure. + if (stackTrace.length > 0) { + exception.setStackTrace( + Util.nullSafeArrayCopyOfRange(stackTrace, /* from= */ 0, /* to= */ 1)); + } + throw exception; + } + } + } + }; + } + + /** An {@link AssertionError} that doesn't print its class name in stack traces. */ + @SuppressWarnings("OverrideThrowableToString") + private static class DiscreteAssertionError extends AssertionError { + + public DiscreteAssertionError(@Nullable String message, @Nullable Throwable cause) { + super(message, cause); + } + + // To avoid printing the class name before the message. Inspired by Truth: + // https://github.com/google/truth/blob/152f3936/core/src/main/java/com/google/common/truth/Platform.java#L186-L192 + @Override + public String toString() { + @Nullable String message = getLocalizedMessage(); + return message != null ? message : super.toString(); + } + } +} diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java index b0491af06a..ac11f11724 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java @@ -16,11 +16,12 @@ package com.google.android.exoplayer2.testutil; import static com.google.android.exoplayer2.util.Assertions.checkNotNull; -import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import android.net.Uri; import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DataSpec; @@ -31,6 +32,7 @@ import java.io.IOException; import java.util.List; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; /** @@ -47,8 +49,11 @@ import org.junit.Test; * in the implementation. The test should be overridden in the subclass and annotated {@link * Ignore}, with a link to an issue to track fixing the implementation and un-ignoring the test. */ +@RequiresApi(19) public abstract class DataSourceContractTest { + @Rule public final AdditionalFailureInfo additionalFailureInfo = new AdditionalFailureInfo(); + /** Creates and returns an instance of the {@link DataSource}. */ protected abstract DataSource createDataSource(); @@ -74,19 +79,20 @@ public abstract class DataSourceContractTest { public void unboundedDataSpec_readEverything() throws Exception { ImmutableList resources = getTestResources(); Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource."); + for (int i = 0; i < resources.size(); i++) { + additionalFailureInfo.setInfo(getFailureLabel(resources, i)); TestResource resource = resources.get(i); DataSource dataSource = createDataSource(); try { long length = dataSource.open(new DataSpec(resource.getUri())); byte[] data = Util.readToEnd(dataSource); - - String failureLabel = getFailureLabel(resources, i); - assertWithMessage(failureLabel).that(length).isEqualTo(resource.getExpectedLength()); - assertWithMessage(failureLabel).that(data).isEqualTo(resource.getExpectedBytes()); + assertThat(length).isEqualTo(resource.getExpectedLength()); + assertThat(data).isEqualTo(resource.getExpectedBytes()); } finally { dataSource.close(); } + additionalFailureInfo.setInfo(null); } } diff --git a/testutils/src/test/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfoTest.java b/testutils/src/test/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfoTest.java new file mode 100644 index 0000000000..2087b1ad0e --- /dev/null +++ b/testutils/src/test/java/com/google/android/exoplayer2/testutil/AdditionalFailureInfoTest.java @@ -0,0 +1,174 @@ +/* + * 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 com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import androidx.annotation.Nullable; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.util.Consumer; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runner.RunWith; +import org.junit.runners.model.Statement; + +/** Tests for {@link AdditionalFailureInfo}. */ +@RunWith(AndroidJUnit4.class) +public final class AdditionalFailureInfoTest { + + private final ExpectedException expectedException = new ExpectedException(); + private final AdditionalFailureInfo additionalFailureInfo = new AdditionalFailureInfo(); + + @Rule + public RuleChain ruleChain = RuleChain.outerRule(expectedException).around(additionalFailureInfo); + + @Test + public void extraInfoSet_addsInfoToAssertionErrorAndSuppressesClassName() { + // Generate an AssertionError using Truth for realism - we'll rethrow this below to test that + // the rule catches it and rethrows a new, augmented AssertionError (with the original stack + // trace). + AssertionError originalAssertionError = + assertThrows(AssertionError.class, () -> assertThat(true).isFalse()); + + additionalFailureInfo.setInfo("extra info"); + + // Check that the original AssertionError is removed from the chain, and its (augmented) message + // and stack trace is set on the new one. + expectedException.setAssertions( + throwable -> { + assertThat(throwable).isInstanceOf(AssertionError.class); + assertThat(throwable) + .hasMessageThat() + .isEqualTo("extra info\n" + originalAssertionError.getMessage()); + assertThat(throwable).hasCauseThat().isNull(); + assertThat(throwable.getStackTrace()).isEqualTo(originalAssertionError.getStackTrace()); + assertThat(throwable.toString()).startsWith("extra info"); + assertThat(throwable.toString()).doesNotContain("AssertionError"); + }); + + throw originalAssertionError; + } + + @Test + public void extraInfoSet_transformsEverythingElseToExceptionAddsInfoAndTruncatesStackTrace() { + IllegalArgumentException originalException = new IllegalArgumentException(); + + additionalFailureInfo.setInfo("extra info"); + + expectedException.setAssertions( + throwable -> { + assertThat(throwable).hasMessageThat().isEqualTo("extra info"); + // Note: Deliberately not using instanceof since we don't want to allow subclasses. + assertThat(throwable.getClass()).isEqualTo(Exception.class); + assertThat(throwable).hasCauseThat().isSameInstanceAs(originalException); + assertThat(throwable.getStackTrace()).hasLength(1); + // The exception is thrown from an anonymous inner class, so just check the prefix. + assertThat(throwable.getStackTrace()[0].getClassName()) + .startsWith(AdditionalFailureInfo.class.getCanonicalName()); + }); + + throw originalException; + } + + @Test + public void noExtraInfoSet_assertionErrorPropagatedDirectly() { + // Generate an AssertionError using Truth for realism. + AssertionError originalAssertionError = + assertThrows(AssertionError.class, () -> assertThat(true).isFalse()); + + expectedException.setAssertions( + throwable -> assertThat(throwable).isSameInstanceAs(originalAssertionError)); + + throw originalAssertionError; + } + + @Test + public void noExtraInfoSet_otherExceptionPropagatedDirectly() { + IllegalArgumentException originalException = new IllegalArgumentException(); + + expectedException.setAssertions( + throwable -> assertThat(throwable).isSameInstanceAs(originalException)); + + throw originalException; + } + + @Test + public void extraInfoSetAndCleared_assertionErrorPropagatedDirectly() { + // Generate an AssertionError using Truth for realism + AssertionError originalAssertionError = + assertThrows(AssertionError.class, () -> assertThat(true).isFalse()); + + additionalFailureInfo.setInfo("extra info"); + additionalFailureInfo.setInfo(null); + + expectedException.setAssertions( + throwable -> assertThat(throwable).isSameInstanceAs(originalAssertionError)); + + throw originalAssertionError; + } + + @Test + public void extraInfoSetAndCleared_otherExceptionPropagatedDirectly() { + IllegalArgumentException originalException = new IllegalArgumentException(); + + additionalFailureInfo.setInfo("extra info"); + additionalFailureInfo.setInfo(null); + + expectedException.setAssertions( + throwable -> assertThat(throwable).isSameInstanceAs(originalException)); + throw originalException; + } + + /** + * A similar rule to JUnit's existing {@link org.junit.rules.ExpectedException}, but without + * relying on Hamcrest matchers. + * + *

JUnit's one is deprecated in favour of using {@link Assert#assertThrows}, but we need a + * {@link Rule} because we need to assert on the exception after it's been transformed by + * the {@link AdditionalFailureInfo} rule. + */ + private static class ExpectedException implements TestRule { + + @Nullable private Consumer throwableAssertions; + + /** + * Provide a callback of assertions to execute on the caught exception. + * + *

A failure in this {@link Consumer} will cause the test to fail. + */ + public void setAssertions(Consumer throwableAssertions) { + this.throwableAssertions = throwableAssertions; + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() { + Throwable expected = assertThrows(Throwable.class, base::evaluate); + if (throwableAssertions != null) { + throwableAssertions.accept(expected); + } + } + }; + } + } +}