From 25c927e9f3dde097b99c710401667f575209a1ec Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 2 Dec 2024 09:09:43 -0800 Subject: [PATCH] Standardize `ForwardingXXX` tests Add missing ones for `ForwardingExtractorInput` and `ForwardingSeekMap`. `ForwardingTimeline` is a bit more fiddly, so it's left for a follow-up change. PiperOrigin-RevId: 701988492 --- .../media3/common/ForwardingPlayerTest.java | 40 +++++++++---------- .../exoplayer/ForwardingRendererTest.java | 21 ++++------ .../audio/ForwardingAudioSinkTest.java | 23 +++++------ .../ForwardingExtractorInputTest.java | 38 ++++++++++++++++++ .../extractor/ForwardingSeekMapTest.java | 36 +++++++++++++++++ .../extractor/ForwardingTrackOutputTest.java | 22 ++++------ .../androidx/media3/test/utils/TestUtil.java | 21 +++++++++- 7 files changed, 136 insertions(+), 65 deletions(-) create mode 100644 libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorInputTest.java create mode 100644 libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingSeekMapTest.java diff --git a/libraries/common/src/test/java/androidx/media3/common/ForwardingPlayerTest.java b/libraries/common/src/test/java/androidx/media3/common/ForwardingPlayerTest.java index b3c2ccbe3b..174955e106 100644 --- a/libraries/common/src/test/java/androidx/media3/common/ForwardingPlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/ForwardingPlayerTest.java @@ -18,17 +18,17 @@ package androidx.media3.common; import static androidx.media3.common.Player.EVENT_IS_PLAYING_CHANGED; import static androidx.media3.common.Player.EVENT_MEDIA_ITEM_TRANSITION; import static androidx.media3.common.Player.EVENT_TIMELINE_CHANGED; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethodsExcept; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethods; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import androidx.media3.test.utils.StubPlayer; -import androidx.media3.test.utils.TestUtil; import androidx.test.ext.junit.runners.AndroidJUnit4; -import java.lang.reflect.Method; +import com.google.common.collect.ImmutableSet; import java.util.HashSet; -import java.util.List; import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -99,29 +99,27 @@ public class ForwardingPlayerTest { @Test public void forwardingPlayer_overridesAllPlayerMethods() throws Exception { - // Check with reflection that ForwardingPlayer overrides all Player methods. - List methods = TestUtil.getPublicMethods(Player.class); - for (Method method : methods) { - assertThat( - ForwardingPlayer.class - .getDeclaredMethod(method.getName(), method.getParameterTypes()) - .getDeclaringClass()) - .isEqualTo(ForwardingPlayer.class); - } + assertForwardingClassOverridesAllMethods(Player.class, ForwardingPlayer.class); } @Test + public void forwardingPlayer_forwardsAllPlayerMethods() throws Exception { + // addListener and removeListener don't directly forward their parameters due to wrapping in + // ForwardingListener. They are tested separately in addListener_addsForwardingListener() and + // removeListener_removesForwardingListener(). + assertForwardingClassForwardsAllMethodsExcept( + Player.class, + ForwardingPlayer::new, + /* excludedMethods= */ ImmutableSet.of("addListener", "removeListener")); + } + + @Test + @SuppressWarnings("unchecked") public void forwardingListener_overridesAllListenerMethods() throws Exception { // Check with reflection that ForwardingListener overrides all Listener methods. - Class forwardingListenerClass = getInnerClass("ForwardingListener"); - List methods = TestUtil.getPublicMethods(Player.Listener.class); - for (Method method : methods) { - assertThat( - forwardingListenerClass - .getMethod(method.getName(), method.getParameterTypes()) - .getDeclaringClass()) - .isEqualTo(forwardingListenerClass); - } + Class forwardingListenerClass = + (Class) getInnerClass("ForwardingListener"); + assertForwardingClassOverridesAllMethods(Player.Listener.class, forwardingListenerClass); } private static Class getInnerClass(String className) { diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ForwardingRendererTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ForwardingRendererTest.java index 93c27792c7..cea585cfb0 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ForwardingRendererTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ForwardingRendererTest.java @@ -15,12 +15,8 @@ */ package androidx.media3.exoplayer; -import static com.google.common.truth.Truth.assertThat; - import androidx.media3.test.utils.TestUtil; import androidx.test.ext.junit.runners.AndroidJUnit4; -import java.lang.reflect.Method; -import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,15 +24,12 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class ForwardingRendererTest { @Test - public void forwardingRenderer_overridesAllMethods() throws NoSuchMethodException { - // Check with reflection that ForwardingRenderer overrides all Renderer methods. - List methods = TestUtil.getPublicMethods(Renderer.class); - for (Method method : methods) { - assertThat( - ForwardingRenderer.class - .getDeclaredMethod(method.getName(), method.getParameterTypes()) - .getDeclaringClass()) - .isEqualTo(ForwardingRenderer.class); - } + public void overridesAllMethods() throws NoSuchMethodException { + TestUtil.assertForwardingClassOverridesAllMethods(Renderer.class, ForwardingRenderer.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + TestUtil.assertForwardingClassForwardsAllMethods(Renderer.class, ForwardingRenderer::new); } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/ForwardingAudioSinkTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/ForwardingAudioSinkTest.java index a4c213ed76..007f942c64 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/ForwardingAudioSinkTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/ForwardingAudioSinkTest.java @@ -15,12 +15,10 @@ */ package androidx.media3.exoplayer.audio; -import static com.google.common.truth.Truth.assertThat; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethods; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethods; -import androidx.media3.test.utils.TestUtil; import androidx.test.ext.junit.runners.AndroidJUnit4; -import java.lang.reflect.Method; -import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,15 +26,12 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public final class ForwardingAudioSinkTest { @Test - public void forwardingAudioSink_overridesAllAudioSinkMethods() throws NoSuchMethodException { - // Check with reflection that ForwardingAudioSink overrides all AudioSink methods. - List methods = TestUtil.getPublicMethods(AudioSink.class); - for (Method method : methods) { - assertThat( - ForwardingAudioSink.class - .getDeclaredMethod(method.getName(), method.getParameterTypes()) - .getDeclaringClass()) - .isEqualTo(ForwardingAudioSink.class); - } + public void overridesAllMethods() throws NoSuchMethodException { + assertForwardingClassOverridesAllMethods(AudioSink.class, ForwardingAudioSink.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + assertForwardingClassForwardsAllMethods(AudioSink.class, ForwardingAudioSink::new); } } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorInputTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorInputTest.java new file mode 100644 index 0000000000..d3b21d94ed --- /dev/null +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorInputTest.java @@ -0,0 +1,38 @@ +/* + * Copyright 2024 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 androidx.media3.extractor; + +import androidx.media3.test.utils.TestUtil; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit tests for {@link ForwardingExtractorInput}. */ +@RunWith(AndroidJUnit4.class) +public class ForwardingExtractorInputTest { + + @Test + public void overridesAllMethods() throws Exception { + TestUtil.assertForwardingClassOverridesAllMethods( + ExtractorInput.class, ForwardingExtractorInput.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + TestUtil.assertForwardingClassForwardsAllMethods( + ExtractorInput.class, ForwardingExtractorInput::new); + } +} diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingSeekMapTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingSeekMapTest.java new file mode 100644 index 0000000000..14a607eaf5 --- /dev/null +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingSeekMapTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2024 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 androidx.media3.extractor; + +import androidx.media3.test.utils.TestUtil; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit tests for {@link ForwardingSeekMap}. */ +@RunWith(AndroidJUnit4.class) +public class ForwardingSeekMapTest { + + @Test + public void overridesAllMethods() throws Exception { + TestUtil.assertForwardingClassOverridesAllMethods(SeekMap.class, ForwardingSeekMap.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + TestUtil.assertForwardingClassForwardsAllMethods(SeekMap.class, ForwardingSeekMap::new); + } +} diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingTrackOutputTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingTrackOutputTest.java index f3993505aa..336ee29525 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingTrackOutputTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingTrackOutputTest.java @@ -15,12 +15,8 @@ */ package androidx.media3.extractor; -import static com.google.common.truth.Truth.assertThat; - import androidx.media3.test.utils.TestUtil; import androidx.test.ext.junit.runners.AndroidJUnit4; -import java.lang.reflect.Method; -import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,15 +24,13 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class ForwardingTrackOutputTest { @Test - public void forwardingTrackOutput_overridesAllMethods() throws NoSuchMethodException { - // Check with reflection that ForwardingTrackOutput overrides all TrackOutput methods. - List methods = TestUtil.getPublicMethods(TrackOutput.class); - for (Method method : methods) { - assertThat( - ForwardingTrackOutput.class - .getDeclaredMethod(method.getName(), method.getParameterTypes()) - .getDeclaringClass()) - .isEqualTo(ForwardingTrackOutput.class); - } + public void overridesAllMethods() throws Exception { + TestUtil.assertForwardingClassOverridesAllMethods( + TrackOutput.class, ForwardingTrackOutput.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + TestUtil.assertForwardingClassForwardsAllMethods(TrackOutput.class, ForwardingTrackOutput::new); } } diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java index 1d5d45a5f1..a71a4a953a 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java @@ -57,6 +57,7 @@ import androidx.media3.extractor.metadata.MetadataInputBuffer; import com.google.common.base.Function; import com.google.common.collect.BoundType; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Range; import com.google.common.io.ByteStreams; import com.google.common.primitives.Bytes; @@ -644,6 +645,19 @@ public class TestUtil { * Use reflection to assert calling every method declared on {@code superType} on an instance of * {@code forwardingType} results in the call being forwarded to the {@code superType} delegate. */ + public static + void assertForwardingClassForwardsAllMethods( + Class superType, Function forwardingInstanceFactory) + throws InvocationTargetException, IllegalAccessException { + assertForwardingClassForwardsAllMethodsExcept( + superType, forwardingInstanceFactory, /* excludedMethods= */ ImmutableSet.of()); + } + + /** + * Use reflection to assert calling every non-excluded method declared on {@code superType} on an + * instance of {@code forwardingType} results in the call being forwarded to the {@code superType} + * delegate. + */ // The nullness checker is deliberately over-conservative and doesn't permit passing a null // parameter to method.invoke(), even if the real method does accept null. Regardless, we expect // the null to be passed straight to our mocked delegate, so it's OK to pass null even for @@ -651,10 +665,13 @@ public class TestUtil { // https://github.com/typetools/checker-framework/blob/c26bb695ebc572fac1e9cd2e331fc5b9d3953ec0/checker/jdk/nullness/src/java/lang/reflect/Method.java#L109 @SuppressWarnings("nullness:argument.type.incompatible") public static - void assertForwardingClassForwardsAllMethods( - Class superType, Function forwardingInstanceFactory) + void assertForwardingClassForwardsAllMethodsExcept( + Class superType, Function forwardingInstanceFactory, Set excludedMethods) throws InvocationTargetException, IllegalAccessException { for (Method method : getPublicMethods(superType)) { + if (excludedMethods.contains(method.getName())) { + continue; + } T delegate = mock(superType); F forwardingInstance = forwardingInstanceFactory.apply(delegate); @NullableType Object[] parameters = new Object[method.getParameterCount()];