From 5f7066a9d99acecadb0446f5e9ba34e3ff2189f2 Mon Sep 17 00:00:00 2001 From: shahddaghash Date: Fri, 21 Mar 2025 05:34:57 -0700 Subject: [PATCH] Resolve race condition and NPE for EditingMetricsCollectorTest Previously, `exportSuccess_populatesEditingEndedEvent` and `exportError_populatesEditingEndedEvent` used an AtomicReference to capture the EditingEndedEvent. However, this led to a race condition where `EditingEndedEvent` was accessed before it was set in the `onMetricsReported` callback, resulting in a NullPointerException. This change replaces the AtomicReference with a SettableFuture to ensure the test waits for the event to be reported before asserting its value. PiperOrigin-RevId: 739146511 --- .../EditingMetricsCollectorTest.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/EditingMetricsCollectorTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/EditingMetricsCollectorTest.java index 608fce4977..cedd06341c 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/EditingMetricsCollectorTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/EditingMetricsCollectorTest.java @@ -43,6 +43,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.platform.app.InstrumentationRegistry; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; +import com.google.common.util.concurrent.SettableFuture; import java.nio.ByteBuffer; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeoutException; @@ -113,7 +114,7 @@ public class EditingMetricsCollectorTest { testId, /* inputFormat= */ MP4_ASSET.videoFormat, /* outputFormat= */ MP4_ASSET.videoFormat); - AtomicReference editingEndedEventAtomicReference = new AtomicReference<>(); + SettableFuture editingEndedEventFuture = SettableFuture.create(); Transformer transformer = new Transformer.Builder(context) .setUsePlatformDiagnostics(true) @@ -124,7 +125,7 @@ public class EditingMetricsCollectorTest { @Override public void onMetricsReported(EditingEndedEvent editingEndedEvent) { - editingEndedEventAtomicReference.set(editingEndedEvent); + editingEndedEventFuture.set(editingEndedEvent); } })) .build(); @@ -150,7 +151,7 @@ public class EditingMetricsCollectorTest { .build() .run(testId, composition); - EditingEndedEvent editingEndedEvent = editingEndedEventAtomicReference.get(); + EditingEndedEvent editingEndedEvent = editingEndedEventFuture.get(); assertThat(editingEndedEvent.getFinalState()) .isEqualTo(EditingEndedEvent.FINAL_STATE_SUCCEEDED); assertThat(editingEndedEvent.getTimeSinceCreatedMillis()).isAtLeast(0); @@ -247,7 +248,7 @@ public class EditingMetricsCollectorTest { testId, /* inputFormat= */ MP4_ASSET.videoFormat, /* outputFormat= */ MP4_ASSET.videoFormat); - AtomicReference editingEndedEventAtomicReference = new AtomicReference<>(); + SettableFuture editingEndedEventFuture = SettableFuture.create(); Transformer transformer = new Transformer.Builder(context) .setUsePlatformDiagnostics(true) @@ -258,7 +259,7 @@ public class EditingMetricsCollectorTest { @Override public void onMetricsReported(EditingEndedEvent editingEndedEvent) { - editingEndedEventAtomicReference.set(editingEndedEvent); + editingEndedEventFuture.set(editingEndedEvent); } })) .setMuxerFactory(new FailingMuxerFactory()) @@ -273,7 +274,7 @@ public class EditingMetricsCollectorTest { .build() .run(testId, audioVideoItem)); - EditingEndedEvent editingEndedEvent = editingEndedEventAtomicReference.get(); + EditingEndedEvent editingEndedEvent = editingEndedEventFuture.get(); assertThat(editingEndedEvent.getFinalState()).isEqualTo(EditingEndedEvent.FINAL_STATE_ERROR); assertThat(editingEndedEvent.getTimeSinceCreatedMillis()).isAtLeast(0); assertThat(editingEndedEvent.getExporterName()).isEqualTo(EXPORTER_NAME); @@ -290,7 +291,7 @@ public class EditingMetricsCollectorTest { testId, /* inputFormat= */ MP4_ASSET.videoFormat, /* outputFormat= */ MP4_ASSET.videoFormat); - AtomicReference editingEndedEventAtomicReference = new AtomicReference<>(); + SettableFuture editingEndedEventFuture = SettableFuture.create(); CountDownLatch countDownLatch = new CountDownLatch(1); Transformer transformer = new Transformer.Builder(context) @@ -302,7 +303,7 @@ public class EditingMetricsCollectorTest { @Override public void onMetricsReported(EditingEndedEvent editingEndedEvent) { - editingEndedEventAtomicReference.set(editingEndedEvent); + editingEndedEventFuture.set(editingEndedEvent); } })) .setMuxerFactory( @@ -321,7 +322,7 @@ public class EditingMetricsCollectorTest { } InstrumentationRegistry.getInstrumentation().runOnMainSync(transformer::cancel); - EditingEndedEvent editingEndedEvent = editingEndedEventAtomicReference.get(); + EditingEndedEvent editingEndedEvent = editingEndedEventFuture.get(); assertThat(editingEndedEvent.getFinalState()).isEqualTo(EditingEndedEvent.FINAL_STATE_CANCELED); assertThat(editingEndedEvent.getTimeSinceCreatedMillis()).isAtLeast(0); assertThat(editingEndedEvent.getExporterName()).isEqualTo(EXPORTER_NAME);