Extend context/scope tests

Author: mcculls

components/context/src/test/java/datadog/context/ContextContinuationTest.java

@@ -255,16 +255,15 @@ void testOutOfOrderScopeCloseReleasesImmediately() {
     Context contextD = root().with(CONTINUATION_KEY, "D");
     try (ContextScope scopeR = continuation.resume()) {
       assertEquals(contextC, current());
-      ContextScope scopeD = contextD.attach(); // attaching D fires detach:C, attach:D
-      assertEquals(contextD, current());
+      try (ContextScope scopeD = contextD.attach()) { // attaching D fires detach:C, attach:D
+        assertEquals(contextD, current());
 
-      // close the resume scope out-of-order while D is still nested on top;
-      // release fires immediately, but detach:C does not (C is not current)
-      scopeR.close();
-      assertEquals(asList("attach:C", "detach:C", "attach:D", "release:C"), events);
-      assertEquals(contextD, current()); // D is still current
-
-      scopeD.close(); // unwind D normally, restores C
+        // close the resume scope out-of-order while D is still nested on top;
+        // release fires immediately, but detach:C does not (C is not current)
+        scopeR.close();
+        assertEquals(asList("attach:C", "detach:C", "attach:D", "release:C"), events);
+        assertEquals(contextD, current()); // D is still current
+      } // scopeD closes here: unwind D normally, restores C
       assertEquals(
           asList("attach:C", "detach:C", "attach:D", "release:C", "detach:D", "attach:C"), events);
     } // try-with-resources closes scopeR again; no second release, C unwinds to root
@@ -292,14 +291,13 @@ void testHoldWithOutOfOrderScopeCloseFiresReleaseOnExplicitRelease() {
     Context contextD = root().with(CONTINUATION_KEY, "D");
     try (ContextScope scopeR = continuation.resume()) {
       assertEquals(contextC, current());
-      ContextScope scopeD = contextD.attach(); // detach:C, attach:D
-      assertEquals(contextD, current());
-
-      scopeR.close(); // out-of-order close while D is still on top; hold prevents auto-release
-      assertEquals(asList("attach:C", "detach:C", "attach:D"), events);
-      assertEquals(contextD, current());
+      try (ContextScope scopeD = contextD.attach()) { // detach:C, attach:D
+        assertEquals(contextD, current());
 
-      scopeD.close(); // unwind D, restores C
+        scopeR.close(); // out-of-order close while D is still on top; hold prevents auto-release
+        assertEquals(asList("attach:C", "detach:C", "attach:D"), events);
+        assertEquals(contextD, current());
+      } // scopeD closes here: unwind D, restores C
     } // TWR closes scopeR again (now in-order); detach:C, no release yet (hold is active)
 
     assertEquals(root(), current());
@@ -312,6 +310,46 @@ void testHoldWithOutOfOrderScopeCloseFiresReleaseOnExplicitRelease() {
         events);
   }
 
+  @Test
+  void testMultipleHoldCallsAreIdempotent() {
+    // Calling hold() more than once should not require more than one explicit release().
+    List<String> events = new ArrayList<>();
+    ContextManager.register(trackingListener(events));
+    Context context = root().with(CONTINUATION_KEY, "value");
+    ContextContinuation continuation;
+    try (ContextScope scope = context.attach()) {
+      continuation = context.capture();
+      continuation.hold();
+      continuation.hold(); // second hold must be a no-op
+    }
+    // One explicit release() is enough — no extra releases needed for the second hold().
+    continuation.release();
+    assertEquals(asList("attach", "capture", "detach", "release"), events);
+    continuation.release(); // still idempotent after the final release
+    assertEquals(asList("attach", "capture", "detach", "release"), events);
+  }
+
+  @Test
+  void testHoldAfterReleaseIsIgnored() {
+    // hold() on an already-released continuation must not resurrect it.
+    List<String> events = new ArrayList<>();
+    ContextManager.register(trackingListener(events));
+    Context context = root().with(CONTINUATION_KEY, "value");
+    ContextContinuation continuation;
+    try (ContextScope scope = context.attach()) {
+      continuation = context.capture();
+    }
+    continuation.release();
+    assertEquals(asList("attach", "capture", "detach", "release"), events);
+    continuation.hold(); // must be silently ignored
+    // resume() after release is already a noop, even with the spurious hold()
+    try (ContextScope scope = continuation.resume()) {
+      assertEquals(root(), current());
+    }
+    continuation.release(); // must not fire a second release event
+    assertEquals(asList("attach", "capture", "detach", "release"), events);
+  }
+
   @Test
   void testHoldAllowsMultipleReleaseCalls() {
     List<String> events = new ArrayList<>();

components/context/src/test/java/datadog/context/ContextListenerEventTest.java

@@ -28,6 +28,8 @@ void testListenersNotifiedOnAttachAndDetach() {
   void testListenersNotNotifiedForRootContext() {
     List<String> events = new ArrayList<>();
     ContextManager.register(trackingListener(events));
+    root().attach(); // current is already root, no events
+    assertTrue(events.isEmpty(), "root attach should not trigger listeners");
     root().swap(); // current is already root, no events
     assertTrue(events.isEmpty(), "root swap should not trigger listeners");
     Context context = root().with(STRING_KEY, "value");
@@ -42,14 +44,14 @@ void testListenersNotNotifiedOnSameContextAttach() {
     List<String> events = new ArrayList<>();
     ContextManager.register(trackingListener(events));
     Context context = root().with(STRING_KEY, "same");
-    ContextScope outer = context.attach();
-    assertEquals(asList("attach"), events);
-    try (ContextScope noop = context.attach()) {
-      assertEquals(context, current());
-      assertEquals(asList("attach"), events); // no new events on same-context attach
+    try (ContextScope outer = context.attach()) {
+      assertEquals(asList("attach"), events);
+      try (ContextScope noop = context.attach()) {
+        assertEquals(context, current());
+        assertEquals(asList("attach"), events); // no new events on same-context attach
+      }
+      assertEquals(asList("attach"), events); // noop close fires no events either
     }
-    assertEquals(asList("attach"), events); // noop close fires no events either
-    outer.close();
     assertEquals(asList("attach", "detach"), events);
   }
 

components/context/src/test/java/datadog/context/ContextListenerExceptionTest.java

@@ -46,11 +46,13 @@ public void onCapture(Context c) {
         });
     Context context = root().with(STRING_KEY, "value");
     try (ContextScope scope = context.attach()) {
-      ContextContinuation[] ref = {null};
-      assertDoesNotThrow(() -> ref[0] = context.capture());
-      assertNotNull(ref[0]);
-      assertEquals(context, ref[0].context());
-      ref[0].release();
+      assertDoesNotThrow(
+          () -> {
+            ContextContinuation continuation = context.capture();
+            assertNotNull(continuation);
+            assertEquals(context, continuation.context());
+            continuation.release();
+          });
     }
   }
 

components/context/src/test/java/datadog/context/ContextManagerTest.java

@@ -66,8 +66,8 @@ void testNoopScopeCacheHitReturnsSameScope() {
     Context context = root().with(STRING_KEY, "value");
     try (ContextScope outer = context.attach()) {
       // two consecutive noop scopes for the same context should be the same cached instance
-      ContextScope noop1 = context.attach();
-      try (ContextScope noop2 = context.attach()) {
+      try (ContextScope noop1 = context.attach();
+          ContextScope noop2 = context.attach()) {
         assertSame(noop1, noop2);
       }
     }
@@ -127,15 +127,16 @@ void testClosingMultipleTimes() {
     Context context1 = root().with(STRING_KEY, "value1");
     try (ContextScope ignored = context1.attach()) {
       Context context2 = context1.with(STRING_KEY, "value2");
-      ContextScope scope = context2.attach();
-      // Test current context
-      assertEquals(context2, current());
-      // Test current context deactivation
-      scope.close();
-      assertEquals(context1, current());
-      // Test multiple context deactivations don’t change current context
-      scope.close();
-      assertEquals(context1, current());
+      try (ContextScope scope = context2.attach()) {
+        // Test current context
+        assertEquals(context2, current());
+        // Test current context deactivation
+        scope.close();
+        assertEquals(context1, current());
+        // Test multiple context deactivations don’t change current context
+        scope.close();
+        assertEquals(context1, current());
+      }
     }
   }
 

dd-trace-core/src/test/java/datadog/trace/core/scopemanager/ScopeManagerTest.java

@@ -5,6 +5,7 @@
 import static datadog.trace.core.scopemanager.ScopeManagerTest.EVENT.ACTIVATE;
 import static datadog.trace.core.scopemanager.ScopeManagerTest.EVENT.CLOSE;
 import static datadog.trace.test.util.GCUtils.awaitGC;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
@@ -1133,13 +1134,13 @@ void continuationReleaseIsSameAsCancel() throws Exception {
   void captureContextWithoutSpanUsesNoopTraceCollector() {
     ContextKey<String> key = ContextKey.named("test-key");
     Context ctx = Context.root().with(key, "value");
-    try (ContextScope scope = ctx.attach()) {
-      // context with no span uses NoopAgentTraceCollector — should not crash
-      ContextContinuation continuation = Context.current().capture();
-      assertNotNull(continuation);
-      assertEquals(ctx, continuation.context());
-      continuation.release(); // no-op on NoopAgentTraceCollector
-    }
+    assertDoesNotThrow(
+        () -> {
+          // NoopAgentTraceCollector handles capture/release without throwing
+          try (ContextScope scope = ctx.attach()) {
+            Context.current().capture().release();
+          }
+        });
   }
 
   private boolean spanFinished(AgentSpan span) {