Generalize tests skipping logic

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java

@@ -3,6 +3,7 @@
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestSourceData;
 import datadog.trace.api.civisibility.retry.TestRetryPolicy;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -31,12 +32,13 @@ TestSuiteImpl testSuiteStart(
   boolean isModified(TestSourceData testSourceData);
 
   /**
-   * Checks if a given test can be skipped with Intelligent Test Runner or not.
+   * Returns the reason for skipping a test, IF it can be skipped.
    *
    * @param test Test to be checked
-   * @return {@code true} if the test can be skipped, {@code false} otherwise
+   * @return skip reason, or {@code null} if the test cannot be skipped
    */
-  boolean isSkippable(TestIdentifier test);
+  @Nullable
+  SkipReason skipReason(TestIdentifier test);
 
   @Nonnull
   TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource);

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java

@@ -8,7 +8,6 @@
 import datadog.trace.api.DDTraceId;
 import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.DDTest;
-import datadog.trace.api.civisibility.InstrumentationBridge;
 import datadog.trace.api.civisibility.InstrumentationTestBridge;
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge;
@@ -23,6 +22,7 @@
 import datadog.trace.api.civisibility.telemetry.tag.IsRetry;
 import datadog.trace.api.civisibility.telemetry.tag.IsRum;
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.api.gateway.RequestContextSlot;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
@@ -208,9 +208,10 @@ public void setSkipReason(String skipReason) {
     if (skipReason != null) {
       span.setTag(Tags.TEST_SKIP_REASON, skipReason);
 
-      if (skipReason.equals(InstrumentationBridge.ITR_SKIP_REASON)) {
+      if (skipReason.equals(SkipReason.ITR.getDescription())) {
         span.setTag(Tags.TEST_SKIPPED_BY_ITR, true);
         metricCollector.add(CiVisibilityCountMetric.ITR_SKIPPED, 1, EventType.TEST);
+        executionResults.incrementTestsSkippedByItr();
       }
     }
   }
@@ -262,10 +263,6 @@ public void end(@Nullable Long endTime) {
       span.finish();
     }
 
-    if (InstrumentationBridge.ITR_SKIP_REASON.equals(span.getTag(Tags.TEST_SKIP_REASON))) {
-      executionResults.incrementTestsSkippedByItr();
-    }
-
     metricCollector.add(
         CiVisibilityCountMetric.EVENT_FINISHED,
         1,

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java

@@ -7,6 +7,7 @@
 import datadog.trace.api.civisibility.coverage.CoverageStore;
 import datadog.trace.api.civisibility.retry.TestRetryPolicy;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -102,9 +103,10 @@ public boolean isModified(TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
+  @Nullable
   @Override
-  public boolean isSkippable(TestIdentifier test) {
-    return executionStrategy.isSkippable(test);
+  public SkipReason skipReason(TestIdentifier test) {
+    return executionStrategy.skipReason(test);
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java

@@ -8,6 +8,7 @@
 import datadog.trace.api.civisibility.coverage.CoverageStore;
 import datadog.trace.api.civisibility.retry.TestRetryPolicy;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -87,9 +88,10 @@ public boolean isModified(TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
+  @Nullable
   @Override
-  public boolean isSkippable(TestIdentifier test) {
-    return executionStrategy.isSkippable(test);
+  public SkipReason skipReason(TestIdentifier test) {
+    return executionStrategy.skipReason(test);
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java

@@ -6,6 +6,7 @@
 import datadog.trace.api.civisibility.config.TestSourceData;
 import datadog.trace.api.civisibility.events.TestEventsHandler;
 import datadog.trace.api.civisibility.retry.TestRetryPolicy;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.civisibility.retry.NeverRetry;
@@ -91,8 +92,8 @@ public void onTestIgnore(
   }
 
   @Override
-  public boolean isSkippable(TestIdentifier test) {
-    return false;
+  public SkipReason skipReason(TestIdentifier test) {
+    return null;
   }
 
   @NotNull

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java

@@ -12,6 +12,7 @@
 import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
 import datadog.trace.api.civisibility.telemetry.tag.EventType;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -187,7 +188,7 @@ public void onTestStart(
           test.setTag(Tags.TEST_ITR_UNSKIPPABLE, true);
           metricCollector.add(CiVisibilityCountMetric.ITR_UNSKIPPABLE, 1, EventType.TEST);
 
-          if (testModule.isSkippable(thisTest)) {
+          if (testModule.skipReason(thisTest) == SkipReason.ITR) {
             test.setTag(Tags.TEST_ITR_FORCED_RUN, true);
             metricCollector.add(CiVisibilityCountMetric.ITR_FORCED_RUN, 1, EventType.TEST);
           }
@@ -276,9 +277,10 @@ public boolean isFlaky(TestIdentifier test) {
     return testModule.isFlaky(test);
   }
 
+  @Nullable
   @Override
-  public boolean isSkippable(TestIdentifier test) {
-    return testModule.isSkippable(test);
+  public SkipReason skipReason(TestIdentifier test) {
+    return testModule.skipReason(test);
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java

@@ -5,6 +5,7 @@
 import datadog.trace.api.civisibility.config.TestMetadata;
 import datadog.trace.api.civisibility.config.TestSourceData;
 import datadog.trace.api.civisibility.retry.TestRetryPolicy;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings;
 import datadog.trace.civisibility.config.ExecutionSettings;
 import datadog.trace.civisibility.retry.NeverRetry;
@@ -17,6 +18,7 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -58,18 +60,23 @@ public boolean isFlaky(TestIdentifier test) {
     return flakyTests != null && flakyTests.contains(test.withoutParameters());
   }
 
-  public boolean isSkippable(TestIdentifier test) {
+  @Nullable
+  public SkipReason skipReason(TestIdentifier test) {
     if (test == null) {
-      return false;
+      return null;
     }
     if (!executionSettings.isTestSkippingEnabled()) {
-      return false;
+      return null;
     }
     Map<TestIdentifier, TestMetadata> skippableTests = executionSettings.getSkippableTests();
     TestMetadata testMetadata = skippableTests.get(test);
-    return testMetadata != null
-        && !(config.isCiVisibilityCoverageLinesEnabled()
-            && testMetadata.isMissingLineCodeCoverage());
+    if (testMetadata == null) {
+      return null;
+    }
+    if (config.isCiVisibilityCoverageLinesEnabled() && testMetadata.isMissingLineCodeCoverage()) {
+      return null;
+    }
+    return SkipReason.ITR;
   }
 
   @Nonnull

dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy

@@ -7,6 +7,7 @@ import com.jayway.jsonpath.JsonPath
 import com.jayway.jsonpath.Option
 import com.jayway.jsonpath.ReadContext
 import com.jayway.jsonpath.WriteContext
+import freemarker.core.InvalidReferenceException
 import freemarker.template.Template
 import freemarker.template.TemplateExceptionHandler
 import org.skyscreamer.jsonassert.JSONAssert
@@ -130,6 +131,16 @@ abstract class CiVisibilityTestUtils {
       StringWriter coveragesOut = new StringWriter()
       coveragesTemplate.process(replacements, coveragesOut)
       return coveragesOut.toString()
+    } catch (InvalidReferenceException e) {
+      def missingExpression = e.getBlamedExpressionString()
+      if (missingExpression != null) {
+        replacements.put(missingExpression, "<VALUE_MISSING>") // to simplify debugging failures
+        return getFreemarkerTemplate(templatePath, replacements, replacementsSource)
+
+      } else {
+        throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e)
+      }
+
     } catch (Exception e) {
       throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e)
     }

dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java

@@ -36,7 +36,7 @@ public String[] helperClassNames() {
     return new String[] {
       packageName + ".CucumberUtils",
       packageName + ".TestEventsHandlerHolder",
-      packageName + ".SkippedByItr",
+      packageName + ".SkippedByDatadog",
       packageName + ".JUnit4Utils",
       packageName + ".TracingListener",
       packageName + ".CucumberTracingListener",

dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java → dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberSkipInstrumentation.java

@@ -11,6 +11,7 @@
 import datadog.trace.api.Config;
 import datadog.trace.api.civisibility.InstrumentationBridge;
 import datadog.trace.api.civisibility.config.TestIdentifier;
+import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.cucumber.core.gherkin.Pickle;
 import java.util.List;
@@ -24,10 +25,10 @@
 import org.junit.runner.notification.RunNotifier;
 
 @AutoService(InstrumenterModule.class)
-public class JUnit4CucumberItrInstrumentation extends InstrumenterModule.CiVisibility
+public class JUnit4CucumberSkipInstrumentation extends InstrumenterModule.CiVisibility
     implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
 
-  public JUnit4CucumberItrInstrumentation() {
+  public JUnit4CucumberSkipInstrumentation() {
     super("ci-visibility", "junit-4", "junit-4-cucumber");
   }
 
@@ -51,7 +52,7 @@ public String[] helperClassNames() {
     return new String[] {
       packageName + ".CucumberUtils",
       packageName + ".TestEventsHandlerHolder",
-      packageName + ".SkippedByItr",
+      packageName + ".SkippedByDatadog",
       packageName + ".JUnit4Utils",
       packageName + ".TracingListener",
       packageName + ".CucumberTracingListener",
@@ -67,34 +68,36 @@ public Reference[] additionalMuzzleReferences() {
   public void methodAdvice(MethodTransformer transformer) {
     transformer.applyAdvice(
         named("run").and(takesArgument(0, named("org.junit.runner.notification.RunNotifier"))),
-        JUnit4CucumberItrInstrumentation.class.getName() + "$CucumberItrAdvice");
+        JUnit4CucumberSkipInstrumentation.class.getName() + "$CucumberSkipAdvice");
   }
 
-  public static class CucumberItrAdvice {
+  public static class CucumberSkipAdvice {
+    @SuppressWarnings("bytebuddy-exception-suppression")
     @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
     @Advice.OnMethodEnter(skipOn = Boolean.class)
     public static Boolean run(
         @Advice.FieldValue("pickle") Pickle pickle,
         @Advice.FieldValue("description") Description description,
         @Advice.Argument(0) RunNotifier notifier) {
 
-      List<String> tags = pickle.getTags();
-      for (String tag : tags) {
-        if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) {
-          return null;
-        }
-      }
-
       TestIdentifier test = CucumberUtils.toTestIdentifier(description);
-      if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) {
-        notifier.fireTestAssumptionFailed(
-            new Failure(
-                description,
-                new AssumptionViolatedException(InstrumentationBridge.ITR_SKIP_REASON)));
-        return Boolean.FALSE;
-      } else {
+      SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test);
+      if (skipReason == null) {
         return null;
       }
+
+      if (skipReason == SkipReason.ITR) {
+        List<String> tags = pickle.getTags();
+        for (String tag : tags) {
+          if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) {
+            return null;
+          }
+        }
+      }
+
+      notifier.fireTestAssumptionFailed(
+          new Failure(description, new AssumptionViolatedException(skipReason.getDescription())));
+      return Boolean.FALSE;
     }
   }
 }

dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/Cucumber4RetryInstrumentation.java

@@ -49,7 +49,7 @@ public String instrumentedType() {
   @Override
   public String[] helperClassNames() {
     return new String[] {
-      parentPackageName + ".SkippedByItr",
+      parentPackageName + ".SkippedByDatadog",
       parentPackageName + ".CucumberUtils",
       parentPackageName + ".JUnit4Utils",
       parentPackageName + ".TracingListener",
@@ -79,6 +79,7 @@ public void methodAdvice(MethodTransformer transformer) {
   }
 
   public static class RetryAdvice {
+    @SuppressWarnings("bytebuddy-exception-suppression")
     @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
     @Advice.OnMethodEnter(skipOn = Boolean.class)
     public static Boolean retryIfNeeded(

dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitInstrumentation.java

@@ -33,7 +33,7 @@ public String instrumentedType() {
   public String[] helperClassNames() {
     return new String[] {
       packageName + ".TestEventsHandlerHolder",
-      packageName + ".SkippedByItr",
+      packageName + ".SkippedByDatadog",
       packageName + ".JUnit4Utils",
       packageName + ".MUnitUtils",
       packageName + ".TracingListener",

dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/retry/MUnitRetryInstrumentation.java

@@ -50,7 +50,7 @@ public String instrumentedType() {
   public String[] helperClassNames() {
     return new String[] {
       parentPackageName + ".MUnitUtils",
-      parentPackageName + ".SkippedByItr",
+      parentPackageName + ".SkippedByDatadog",
       parentPackageName + ".JUnit4Utils",
       parentPackageName + ".TracingListener",
       parentPackageName + ".TestEventsHandlerHolder",
@@ -72,6 +72,7 @@ public void methodAdvice(MethodTransformer transformer) {
   }
 
   public static class RetryAdvice {
+    @SuppressWarnings("bytebuddy-exception-suppression")
     @Advice.OnMethodEnter(skipOn = Future.class)
     public static Future<?> retryIfNeeded(
         @Advice.Origin Method runTest,
@@ -118,6 +119,7 @@ public static Future<?> retryIfNeeded(
       return result;
     }
 
+    @SuppressWarnings("bytebuddy-exception-suppression")
     @SuppressFBWarnings(
         value = "UC_USELESS_OBJECT",
         justification = "result is the return value of the original method")

dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4Instrumentation.java

@@ -65,7 +65,7 @@ public ElementMatcher<TypeDescription> hierarchyMatcher() {
   public String[] helperClassNames() {
     return new String[] {
       packageName + ".TestEventsHandlerHolder",
-      packageName + ".SkippedByItr",
+      packageName + ".SkippedByDatadog",
       packageName + ".JUnit4Utils",
       packageName + ".TracingListener",
       packageName + ".JUnit4TracingListener",