Skip to content

Commit dccf6c8

Browse files
committed
Fix AutoCleanupExtension.deferAfterAll firing per @nested container
AfterAllCallback.afterAll() fires once per container the extension is registered in, including each @nested class (since static @RegisterExtension fields are inherited). This caused deferAfterAll cleanups to run as soon as the first nested container completed. Track the outermost container via beforeAll and only run deferAfterAll cleanups when afterAll fires on that same context. This was breaking ApacheHttpAsyncClientTest, where the shared async clients were closed after the first nested test class finished, causing all subsequent nested tests to fail with CancellationException.
1 parent d9fba53 commit dccf6c8

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AutoCleanupExtension.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.util.concurrent.ConcurrentLinkedDeque;
1212
import org.junit.jupiter.api.extension.AfterAllCallback;
1313
import org.junit.jupiter.api.extension.AfterEachCallback;
14+
import org.junit.jupiter.api.extension.BeforeAllCallback;
1415
import org.junit.jupiter.api.extension.ExtensionContext;
1516

1617
/**
@@ -21,9 +22,13 @@
2122
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
2223
* at any time.
2324
*/
24-
public class AutoCleanupExtension implements AfterEachCallback, AfterAllCallback {
25+
public class AutoCleanupExtension
26+
implements BeforeAllCallback, AfterEachCallback, AfterAllCallback {
2527
private final Deque<AutoCloseable> thingsToCleanUp = new ConcurrentLinkedDeque<>();
2628
private final Deque<AutoCloseable> thingsToCleanUpAfterAll = new ConcurrentLinkedDeque<>();
29+
// Tracks the first (outermost) container the extension is registered in, so that afterAll only
30+
// runs deferAfterAll cleanups once — not once per @Nested container.
31+
private volatile ExtensionContext rootContext;
2732

2833
public static AutoCleanupExtension create() {
2934
return new AutoCleanupExtension();
@@ -41,6 +46,13 @@ public void deferAfterAll(AutoCloseable cleanupAction) {
4146
thingsToCleanUpAfterAll.push(cleanupAction);
4247
}
4348

49+
@Override
50+
public void beforeAll(ExtensionContext extensionContext) {
51+
if (rootContext == null) {
52+
rootContext = extensionContext;
53+
}
54+
}
55+
4456
@Override
4557
public void afterEach(ExtensionContext extensionContext) throws Exception {
4658
List<Exception> exceptions = new ArrayList<>();
@@ -56,6 +68,11 @@ public void afterEach(ExtensionContext extensionContext) throws Exception {
5668

5769
@Override
5870
public void afterAll(ExtensionContext extensionContext) throws Exception {
71+
// afterAll fires once per container the extension is registered in, including each @Nested
72+
// class. Only run deferAfterAll cleanups when the outermost container completes.
73+
if (extensionContext != rootContext) {
74+
return;
75+
}
5976
List<Exception> exceptions = new ArrayList<>();
6077
while (!thingsToCleanUpAfterAll.isEmpty()) {
6178
try {

testing-common/src/test/java/io/opentelemetry/instrumentation/testing/junit/AutoCleanupExtensionTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
package io.opentelemetry.instrumentation.testing.junit;
77

88
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.mockito.Mockito.mock;
910

1011
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
1112
import java.util.ArrayList;
1213
import java.util.List;
1314
import java.util.concurrent.atomic.AtomicInteger;
1415
import org.junit.jupiter.api.AfterAll;
1516
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.api.extension.ExtensionContext;
1618
import org.junit.jupiter.api.extension.RegisterExtension;
1719

1820
class AutoCleanupExtensionTest {
@@ -60,4 +62,27 @@ void shouldRunDeferCleanupInLifoOrder() throws Exception {
6062

6163
assertThat(order).containsExactly(3, 2, 1);
6264
}
65+
66+
@Test
67+
void shouldRunAfterAllCleanupOnlyOnceAcrossNestedContainers() throws Exception {
68+
AutoCleanupExtension cleanup = AutoCleanupExtension.create();
69+
AtomicInteger closed = new AtomicInteger(0);
70+
71+
ExtensionContext rootContext = mock(ExtensionContext.class);
72+
ExtensionContext nestedContext = mock(ExtensionContext.class);
73+
74+
// Simulate JUnit registering the extension on the outer container, then entering a @Nested
75+
// container. AfterAllCallback#afterAll fires once per container, so it will fire for the
76+
// nested context first and then for the root context.
77+
cleanup.beforeAll(rootContext);
78+
cleanup.beforeAll(nestedContext);
79+
80+
cleanup.deferAfterAll(closed::incrementAndGet);
81+
82+
cleanup.afterAll(nestedContext);
83+
assertThat(closed.get()).isEqualTo(0);
84+
85+
cleanup.afterAll(rootContext);
86+
assertThat(closed.get()).isEqualTo(1);
87+
}
6388
}

0 commit comments

Comments
 (0)