Update Spock ITR instrumentation to skip spec setup methods if all fe…

…atures in a spec are skipped (#6782)
DataDog · Mar 6, 2024 · 37358b9 · 37358b9
1 parent 04f98d5
commit 37358b9
Show file tree

Hide file tree

Showing 15 changed files with 1,077 additions and 23 deletions.
diff --git a/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java
@@ -291,6 +291,11 @@ public boolean skip(TestIdentifier test) {
     return testModule.skip(test);
   }
 
+  @Override
+  public boolean isSkippable(TestIdentifier test) {
+    return testModule.isSkippable(test);
+  }
+
   @Override
   @Nonnull
   public TestRetryPolicy retryPolicy(TestIdentifier test) {

diff --git a/...t-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java b/...t-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java
@@ -13,14 +13,14 @@
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.bootstrap.CallDepthThreadLocalMap;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import java.util.Collection;
 import java.util.Set;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
-import org.junit.platform.engine.TestTag;
+import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.support.hierarchical.Node;
 import org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService;
+import org.spockframework.runtime.SpecNode;
 import org.spockframework.runtime.SpockNode;
 
 @AutoService(Instrumenter.class)
@@ -102,16 +102,38 @@ public static void shouldBeSkipped(
         return;
       }
 
-      Collection<TestTag> tags = SpockUtils.getTags(spockNode);
-      for (TestTag tag : tags) {
-        if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) {
-          return;
-        }
+      if (SpockUtils.isUnskippable(spockNode)) {
+        return;
       }
 
-      TestIdentifier test = SpockUtils.toTestIdentifier(spockNode);
-      if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) {
+      if (spockNode instanceof SpecNode) {
+        // suite
+        SpecNode specNode = (SpecNode) spockNode;
+        Set<? extends TestDescriptor> features = specNode.getChildren();
+        for (TestDescriptor feature : features) {
+          if (feature instanceof SpockNode && SpockUtils.isUnskippable((SpockNode<?>) feature)) {
+            return;
+          }
+
+          TestIdentifier featureIdentifier = SpockUtils.toTestIdentifier(feature);
+          if (featureIdentifier == null
+              || !TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(featureIdentifier)) {
+            return;
+          }
+        }
+
+        // all children are skippable
+        for (TestDescriptor feature : features) {
+          TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(SpockUtils.toTestIdentifier(feature));
+        }
         skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON);
+
+      } else {
+        // individual test case
+        TestIdentifier test = SpockUtils.toTestIdentifier(spockNode);
+        if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) {
+          skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON);
+        }
       }
     }
 

diff --git a/...unit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java b/...unit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockUtils.java
@@ -1,5 +1,6 @@
 package datadog.trace.instrumentation.junit5;
 
+import datadog.trace.api.civisibility.InstrumentationBridge;
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import java.lang.invoke.MethodHandle;
 import java.lang.reflect.Method;
@@ -64,6 +65,16 @@ public static Collection<TestTag> getTags(SpockNode<?> spockNode) {
     }
   }
 
+  public static boolean isUnskippable(SpockNode<?> spockNode) {
+    Collection<TestTag> tags = SpockUtils.getTags(spockNode);
+    for (TestTag tag : tags) {
+      if (InstrumentationBridge.ITR_UNSKIPPABLE_TAG.equals(tag.getName())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   public static Method getTestMethod(MethodSource methodSource) {
     String methodName = methodSource.getMethodName();
     if (methodName == null) {

diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/test/groovy/SpockTest.groovy b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/test/groovy/SpockTest.groovy
@@ -6,8 +6,10 @@ import org.example.TestFailedParameterizedSpock
 import org.example.TestFailedSpock
 import org.example.TestFailedThenSucceedParameterizedSpock
 import org.example.TestFailedThenSucceedSpock
+import org.example.TestParameterizedSetupSpecSpock
 import org.example.TestParameterizedSpock
 import org.example.TestSucceedAndFailedSpock
+import org.example.TestSucceedSetupSpecSpock
 import org.example.TestSucceedSpock
 import org.example.TestSucceedSpockSlow
 import org.example.TestSucceedSpockUnskippable
@@ -47,13 +49,21 @@ class SpockTest extends CiVisibilityInstrumentationTest {
     assertSpansData(testcaseName, expectedTracesCount)
 
     where:
-    testcaseName                      | tests                              | expectedTracesCount | skippedTests
-    "test-itr-skipping"               | [TestSucceedSpock]                 | 2                   | [new TestIdentifier("org.example.TestSucceedSpock", "test success", null, null)]
-    "test-itr-skipping-parameterized" | [TestParameterizedSpock]           | 3                   | [
+    testcaseName                                     | tests                              | expectedTracesCount | skippedTests
+    "test-itr-skipping"                              | [TestSucceedSpock]                 | 2                   | [new TestIdentifier("org.example.TestSucceedSpock", "test success", null, null)]
+    "test-itr-skipping-parameterized"                | [TestParameterizedSpock]           | 3                   | [
       new TestIdentifier("org.example.TestParameterizedSpock", "test add 1 and 2", '{"metadata":{"test_name":"test add 1 and 2"}}', null)
     ]
-    "test-itr-unskippable"            | [TestSucceedSpockUnskippable]      | 2                   | [new TestIdentifier("org.example.TestSucceedSpockUnskippable", "test success", null, null)]
-    "test-itr-unskippable-suite"      | [TestSucceedSpockUnskippableSuite] | 2                   | [new TestIdentifier("org.example.TestSucceedSpockUnskippableSuite", "test success", null, null)]
+    "test-itr-unskippable"                           | [TestSucceedSpockUnskippable]      | 2                   | [new TestIdentifier("org.example.TestSucceedSpockUnskippable", "test success", null, null)]
+    "test-itr-unskippable-suite"                     | [TestSucceedSpockUnskippableSuite] | 2                   | [new TestIdentifier("org.example.TestSucceedSpockUnskippableSuite", "test success", null, null)]
+    "test-itr-skipping-spec-setup"                   | [TestSucceedSetupSpecSpock]        | 2                   | [
+      new TestIdentifier("org.example.TestSucceedSetupSpecSpock", "test success", null, null),
+      new TestIdentifier("org.example.TestSucceedSetupSpecSpock", "test another success", null, null)
+    ]
+    "test-itr-not-skipping-spec-setup"               | [TestSucceedSetupSpecSpock]        | 2                   | [new TestIdentifier("org.example.TestSucceedSetupSpecSpock", "test success", null, null)]
+    "test-itr-not-skipping-parameterized-spec-setup" | [TestParameterizedSetupSpecSpock]  | 2                   | [
+      new TestIdentifier("org.example.TestParameterizedSetupSpecSpock", "test add 1 and 2", '{"metadata":{"test_name":"test add 1 and 2"}}', null)
+    ]
   }
 
   def "test flaky retries #testcaseName"() {
@@ -86,18 +96,18 @@ class SpockTest extends CiVisibilityInstrumentationTest {
     assertSpansData(testcaseName, expectedTracesCount)
 
     where:
-    testcaseName                        | tests                        | expectedTracesCount | knownTestsList
-    "test-efd-known-test"               | [TestSucceedSpock]           | 2                   | [new TestIdentifier("org.example.TestSucceedSpock", "test success", null, null)]
-    "test-efd-known-parameterized-test" | [TestParameterizedSpock]     | 3                   | [
+    testcaseName                        | tests                       | expectedTracesCount | knownTestsList
+    "test-efd-known-test"               | [TestSucceedSpock]          | 2                   | [new TestIdentifier("org.example.TestSucceedSpock", "test success", null, null)]
+    "test-efd-known-parameterized-test" | [TestParameterizedSpock]    | 3                   | [
       new TestIdentifier("org.example.TestParameterizedSpock", "test add 1 and 2", null, null),
       new TestIdentifier("org.example.TestParameterizedSpock", "test add 4 and 4", null, null)
     ]
-    "test-efd-new-test"                 | [TestSucceedSpock]           | 4                   | []
-    "test-efd-new-parameterized-test"   | [TestParameterizedSpock]     | 7                   | []
-    "test-efd-known-tests-and-new-test" | [TestParameterizedSpock]     | 5                   | [new TestIdentifier("org.example.TestParameterizedSpock", "test add 1 and 2", null, null)]
-    "test-efd-new-slow-test"            | [TestSucceedSpockSlow]       | 3                   | [] // is executed only twice
-    "test-efd-new-very-slow-test"       | [TestSucceedSpockVerySlow]   | 2                   | [] // is executed only once
-    "test-efd-faulty-session-threshold" | [TestSucceedAndFailedSpock]  | 8                   | []
+    "test-efd-new-test"                 | [TestSucceedSpock]          | 4                   | []
+    "test-efd-new-parameterized-test"   | [TestParameterizedSpock]    | 7                   | []
+    "test-efd-known-tests-and-new-test" | [TestParameterizedSpock]    | 5                   | [new TestIdentifier("org.example.TestParameterizedSpock", "test add 1 and 2", null, null)]
+    "test-efd-new-slow-test"            | [TestSucceedSpockSlow]      | 3                   | [] // is executed only twice
+    "test-efd-new-very-slow-test"       | [TestSucceedSpockVerySlow]  | 2                   | [] // is executed only once
+    "test-efd-faulty-session-threshold" | [TestSucceedAndFailedSpock] | 8                   | []
   }
 
   private static void runTests(List<Class<?>> classes) {

diff --git a/...unit-5.3/spock-junit-5/src/test/groovy/org/example/TestParameterizedSetupSpecSpock.groovy b/...unit-5.3/spock-junit-5/src/test/groovy/org/example/TestParameterizedSetupSpecSpock.groovy
@@ -0,0 +1,29 @@
+package org.example
+
+import datadog.trace.bootstrap.instrumentation.api.AgentScope
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer
+import datadog.trace.bootstrap.instrumentation.api.ScopeSource
+import spock.lang.Specification
+
+class TestParameterizedSetupSpecSpock extends Specification {
+
+  def setupSpec() {
+    AgentTracer.TracerAPI agentTracer = AgentTracer.get()
+    AgentSpan span = agentTracer.buildSpan("spock-manual", "spec-setup").start()
+    try (AgentScope scope = agentTracer.activateSpan(span, ScopeSource.MANUAL)) {
+      // manually trace spec setup to check whether ITR skips it or not
+    }
+    span.finish()
+  }
+
+  def "test add #a and #b"() {
+    expect:
+    a + b == c
+
+    where:
+    a | b | c
+    1 | 2 | 3
+    4 | 4 | 8
+  }
+}
diff --git a/...tion/junit-5.3/spock-junit-5/src/test/groovy/org/example/TestSucceedSetupSpecSpock.groovy b/...tion/junit-5.3/spock-junit-5/src/test/groovy/org/example/TestSucceedSetupSpecSpock.groovy
@@ -0,0 +1,29 @@
+package org.example
+
+import datadog.trace.bootstrap.instrumentation.api.AgentScope
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer
+import datadog.trace.bootstrap.instrumentation.api.ScopeSource
+import spock.lang.Specification
+
+class TestSucceedSetupSpecSpock extends Specification {
+
+  def setupSpec() {
+    AgentTracer.TracerAPI agentTracer = AgentTracer.get()
+    AgentSpan span = agentTracer.buildSpan("spock-manual", "spec-setup").start()
+    try (AgentScope scope = agentTracer.activateSpan(span, ScopeSource.MANUAL)) {
+      // manually trace spec setup to check whether ITR skips it or not
+    }
+    span.finish()
+  }
+
+  def "test success"() {
+    expect:
+    1 == 1
+  }
+
+  def "test another success"() {
+    expect:
+    1 == 1
+  }
+}
diff --git a/...k-junit-5/src/test/resources/test-itr-not-skipping-parameterized-spec-setup/coverages.ftl b/...k-junit-5/src/test/resources/test-itr-not-skipping-parameterized-spec-setup/coverages.ftl
@@ -0,0 +1 @@
+[ ]