From a496bbd77789b9556cf246e9a555ee8dc5b962c2 Mon Sep 17 00:00:00 2001 From: ibaker Date: Fri, 22 Dec 2023 03:39:56 -0800 Subject: [PATCH] Add parentheses to fix `UnexpectedLoaderException` message logic These were missing in https://github.com/androidx/media/commit/5211ff0dc1a80d948d220521c8f0327067a78467 Without these, the `": null"` is still logged (but the `"Unexpected IllegalStateException"` bit is not), because the **whole** string concatenation is compared to null as the boolean condition of the ternary, and this condition is always false (the result of a string concatentation is never `null`) i.e. (with excess parentheses for clarity): ``` (("Unexpected " + cause.getClass().getSimpleName() + cause.getMessage()) != null) ? ": " + cause.getMessage() : "" ``` Also add a test because obviously this isn't as simple as I'd thought. PiperOrigin-RevId: 593079975 --- .../media3/exoplayer/upstream/Loader.java | 6 +-- .../UnexpectedLoaderExceptionTest.java | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/UnexpectedLoaderExceptionTest.java diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/Loader.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/Loader.java index a8147fecb4..46923e540b 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/Loader.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/Loader.java @@ -48,9 +48,9 @@ public final class Loader implements LoaderErrorThrower { public UnexpectedLoaderException(Throwable cause) { super( - "Unexpected " + cause.getClass().getSimpleName() + cause.getMessage() != null - ? ": " + cause.getMessage() - : "", + "Unexpected " + + cause.getClass().getSimpleName() + + (cause.getMessage() != null ? ": " + cause.getMessage() : ""), cause); } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/UnexpectedLoaderExceptionTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/UnexpectedLoaderExceptionTest.java new file mode 100644 index 0000000000..7f13a8210d --- /dev/null +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/UnexpectedLoaderExceptionTest.java @@ -0,0 +1,48 @@ +/* + * Copyright 2023 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 + * + * https://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 androidx.media3.exoplayer.upstream; + +import static com.google.common.truth.Truth.assertThat; + +import androidx.media3.exoplayer.upstream.Loader.UnexpectedLoaderException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link UnexpectedLoaderException}. */ +@RunWith(JUnit4.class) +public class UnexpectedLoaderExceptionTest { + + @Test + public void causeWithMessage_messageAppended() { + UnexpectedLoaderException unexpectedLoaderException = + new UnexpectedLoaderException(new IllegalStateException("test message")); + + assertThat(unexpectedLoaderException) + .hasMessageThat() + .isEqualTo("Unexpected IllegalStateException: test message"); + } + + @Test + public void causeWithoutMessage_noMessageAppended() { + UnexpectedLoaderException unexpectedLoaderException = + new UnexpectedLoaderException(new IllegalStateException()); + + assertThat(unexpectedLoaderException) + .hasMessageThat() + .isEqualTo("Unexpected IllegalStateException"); + } +}