Replace string constants with a dedicated enum for test statuses (#7218)

nikita-tkachenko-datadog · web-flow · commit c9e04c17d09e · 2024-06-19T17:16:39.000+02:00
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java
@@ -3,7 +3,6 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
 
 import datadog.trace.api.Config;
-import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
 import datadog.trace.api.civisibility.telemetry.tag.EventType;
@@ -71,7 +70,7 @@ public AbstractTestModule(
 
     // setting status to skip initially,
     // as we do not know in advance whether the module will have any children
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
 
     testDecorator.afterStart(span);
 
@@ -89,11 +88,11 @@ public void setTag(String key, Object value) {
   public void setErrorInfo(Throwable error) {
     span.setError(true);
     span.addThrowable(error);
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_FAIL);
+    span.setTag(Tags.TEST_STATUS, TestStatus.fail);
   }
 
   public void setSkipReason(String skipReason) {
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
     if (skipReason != null) {
       span.setTag(Tags.TEST_SKIP_REASON, skipReason);
     }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java
@@ -3,7 +3,6 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
 
 import datadog.trace.api.Config;
-import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
 import datadog.trace.api.civisibility.telemetry.TagValue;
@@ -72,7 +71,7 @@ public AbstractTestSession(
 
     // setting status to skip initially,
     // as we do not know in advance whether the session will have any children
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
 
     span.setResourceName(projectName);
 
@@ -106,11 +105,11 @@ public void setTag(String key, Object value) {
   public void setErrorInfo(Throwable error) {
     span.setError(true);
     span.addThrowable(error);
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_FAIL);
+    span.setTag(Tags.TEST_STATUS, TestStatus.fail);
   }
 
   public void setSkipReason(String skipReason) {
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
     if (skipReason != null) {
       span.setTag(Tags.TEST_SKIP_REASON, skipReason);
     }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java
@@ -113,7 +113,7 @@ public TestImpl(
     span.setTag(Tags.TEST_MODULE_ID, moduleId);
     span.setTag(Tags.TEST_SESSION_ID, sessionId);
 
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_PASS);
+    span.setTag(Tags.TEST_STATUS, TestStatus.pass);
 
     if (testClass != null && !testClass.getName().equals(testSuiteName)) {
       span.setTag(Tags.TEST_SOURCE_CLASS, testClass.getName());
@@ -178,12 +178,12 @@ public void setTag(String key, Object value) {
   public void setErrorInfo(Throwable error) {
     span.setError(true);
     span.addThrowable(error);
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_FAIL);
+    span.setTag(Tags.TEST_STATUS, TestStatus.fail);
   }
 
   @Override
   public void setSkipReason(String skipReason) {
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
     if (skipReason != null) {
       span.setTag(Tags.TEST_SKIP_REASON, skipReason);
 
@@ -222,7 +222,7 @@ public void end(@Nullable Long endTime) {
     CoverageBridge.removeThreadLocalCoverageProbeStore();
     boolean coveragesGathered =
         context.getCoverageProbeStore().report(sessionId, suiteId, span.getSpanId());
-    if (!coveragesGathered && !CIConstants.TEST_SKIP.equals(span.getTag(Tags.TEST_STATUS))) {
+    if (!coveragesGathered && !TestStatus.skip.equals(span.getTag(Tags.TEST_STATUS))) {
       // test is not skipped, but no coverages were gathered
       metricCollector.add(CiVisibilityCountMetric.CODE_COVERAGE_IS_EMPTY, 1);
     }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestStatus.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestStatus.java
@@ -0,0 +1,7 @@
+package datadog.trace.civisibility.domain;
+
+public enum TestStatus {
+  pass,
+  fail,
+  skip
+}
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java
@@ -4,7 +4,6 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
 
 import datadog.trace.api.Config;
-import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.DDTestSuite;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
@@ -103,7 +102,7 @@ public TestSuiteImpl(
 
     // setting status to skip initially,
     // as we do not know in advance whether the suite will have any children
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
 
     this.testClass = testClass;
     if (this.testClass != null) {
@@ -138,12 +137,12 @@ public void setTag(String key, Object value) {
   public void setErrorInfo(Throwable error) {
     span.setError(true);
     span.addThrowable(error);
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_FAIL);
+    span.setTag(Tags.TEST_STATUS, TestStatus.fail);
   }
 
   @Override
   public void setSkipReason(String skipReason) {
-    span.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+    span.setTag(Tags.TEST_STATUS, TestStatus.skip);
     if (skipReason != null) {
       span.setTag(Tags.TEST_SKIP_REASON, skipReason);
     }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java
@@ -1,13 +1,13 @@
 package datadog.trace.civisibility.events;
 
-import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.config.ModuleExecutionSettings;
 import datadog.trace.api.civisibility.events.BuildEventsHandler;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 import datadog.trace.civisibility.config.JvmInfo;
 import datadog.trace.civisibility.config.JvmInfoFactory;
 import datadog.trace.civisibility.domain.BuildSystemModule;
 import datadog.trace.civisibility.domain.BuildSystemSession;
+import datadog.trace.civisibility.domain.TestStatus;
 import java.io.File;
 import java.nio.file.Path;
 import java.util.Collection;
@@ -86,7 +86,7 @@ public ModuleInfo onTestModuleStart(
 
     BuildSystemSession testSession = inProgressTestSessions.get(sessionKey);
     BuildSystemModule testModule = testSession.testModuleStart(moduleName, null, outputClassesDirs);
-    testModule.setTag(Tags.TEST_STATUS, CIConstants.TEST_PASS);
+    testModule.setTag(Tags.TEST_STATUS, TestStatus.pass);
 
     if (additionalTags != null) {
       for (Map.Entry<String, Object> e : additionalTags.entrySet()) {
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java
@@ -1,8 +1,8 @@
 package datadog.trace.civisibility.utils;
 
-import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
+import datadog.trace.civisibility.domain.TestStatus;
 import datadog.trace.civisibility.ipc.TestFramework;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -97,24 +97,24 @@ private static void setFrameworks(AgentSpan span, Collection<TestFramework> fram
   }
 
   private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) {
-    String childStatus = (String) childSpan.getTag(Tags.TEST_STATUS);
+    TestStatus childStatus = (TestStatus) childSpan.getTag(Tags.TEST_STATUS);
     if (childStatus == null) {
       return;
     }
 
-    String parentStatus = (String) parentSpan.getTag(Tags.TEST_STATUS);
+    TestStatus parentStatus = (TestStatus) parentSpan.getTag(Tags.TEST_STATUS);
     switch (childStatus) {
-      case CIConstants.TEST_PASS:
-        if (parentStatus == null || CIConstants.TEST_SKIP.equals(parentStatus)) {
-          parentSpan.setTag(Tags.TEST_STATUS, CIConstants.TEST_PASS);
+      case pass:
+        if (parentStatus == null || TestStatus.skip.equals(parentStatus)) {
+          parentSpan.setTag(Tags.TEST_STATUS, TestStatus.pass);
         }
         break;
-      case CIConstants.TEST_FAIL:
-        parentSpan.setTag(Tags.TEST_STATUS, CIConstants.TEST_FAIL);
+      case fail:
+        parentSpan.setTag(Tags.TEST_STATUS, TestStatus.fail);
         break;
-      case CIConstants.TEST_SKIP:
+      case skip:
         if (parentStatus == null) {
-          parentSpan.setTag(Tags.TEST_STATUS, CIConstants.TEST_SKIP);
+          parentSpan.setTag(Tags.TEST_STATUS, TestStatus.skip);
         }
         break;
       default:
diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/CIConstants.java b/internal-api/src/main/java/datadog/trace/api/civisibility/CIConstants.java
@@ -1,10 +1,6 @@
 package datadog.trace.api.civisibility;
 
 public interface CIConstants {
-  String TEST_PASS = "pass";
-  String TEST_FAIL = "fail";
-  String TEST_SKIP = "skip";
-
   /**
    * Indicates that early flakiness detection feature was aborted in a test session because too many
    * test cases were considered new.

-Original file line number
+Diff line change
@@ @@ -0,0 +1,7 @@ @@
 +package datadog.trace.civisibility.domain;
++
 +public enum TestStatus {
 +  pass,
 +  fail,
 +  skip
 +}