Fix TestNG session status reported with EFD

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/EarlyFlakeDetection.java

@@ -71,6 +71,15 @@ public boolean suppressFailures() {
     return suppressFailures;
   }
 
+  @Override
+  public boolean propagateFailure() {
+    // used to bypass TestNG's RetryAnalyzer, which made the framework session status depend on
+    // result order:
+    // pass + fail -> fail (correct)
+    // fail + pass -> pass (incorrect)
+    return !suppressFailures && results == ExecutionAggregation.MIXED;
+  }
+
   private int getExecutions(long durationMillis) {
     for (ExecutionsByDuration e : executionsByDuration) {
       if (durationMillis <= e.getDurationMillis()) {

dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java

@@ -47,7 +47,7 @@ public void methodAdvice(MethodTransformer transformer) {
 
     transformer.applyAdvice(
         named("runTestResultListener").and(takesArgument(0, named("org.testng.ITestResult"))),
-        TestNGExecutionInstrumentation.class.getName() + "$SuppressFailuresAdvice");
+        TestNGExecutionInstrumentation.class.getName() + "$ModifyStatusAdvice");
   }
 
   @Override
@@ -106,24 +106,27 @@ public static void shouldRetryTestMethod(
     }
   }
 
-  public static class SuppressFailuresAdvice {
+  public static class ModifyStatusAdvice {
     @SuppressWarnings("bytebuddy-exception-suppression")
     @Advice.OnMethodEnter
-    public static void suppressFailures(@Advice.Argument(0) final ITestResult result) {
-      if (result.getStatus() != ITestResult.FAILURE) {
-        // nothing to suppress
-        return;
-      }
-
+    public static void modifyStatus(@Advice.Argument(0) final ITestResult result) {
       IRetryAnalyzer retryAnalyzer = TestNGUtils.getRetryAnalyzer(result);
       if (!(retryAnalyzer instanceof RetryAnalyzer)) {
         // test execution policies not injected
         return;
       }
       RetryAnalyzer ddRetryAnalyzer = (RetryAnalyzer) retryAnalyzer;
-      if (ddRetryAnalyzer.getAndResetSuppressFailures()) {
+
+      if (result.getStatus() == ITestResult.FAILURE
+          && ddRetryAnalyzer.getAndResetSuppressFailures()) {
         // "failed but within success percentage"
         result.setStatus(ITestResult.SUCCESS_PERCENTAGE_FAILURE);
+      } else if (result.isSuccess() && ddRetryAnalyzer.shouldPropagateFailure()) {
+        // mark status as failed to propagate an earlier failure suppressed by TestNG
+        result.setStatus(ITestResult.FAILURE);
+        result.setThrowable(
+            new AssertionError(
+                "Datadog: propagating test failure based on aggregated execution results"));
       }
     }
   }

dd-java-agent/instrumentation/testng/testng-7.0/src/test/resources/test-efd-new-flaky-test-fail-pass/coverages.ftl

@@ -0,0 +1 @@
+[ ]

dd-java-agent/instrumentation/testng/testng-7.0/src/test/resources/test-efd-new-flaky-test-fail-pass/events.ftl

@@ -0,0 +1,207 @@
+[ {
+  "content" : {
+    "duration" : ${content_duration},
+    "error" : 0,
+    "meta" : {
+      "_dd.p.tid" : ${content_meta__dd_p_tid},
+      "component" : "testng",
+      "dummy_ci_tag" : "dummy_ci_tag_value",
+      "env" : "none",
+      "library_version" : ${content_meta_library_version},
+      "span.kind" : "test_suite_end",
+      "test.codeowners" : "[\"owner1\",\"owner2\"]",
+      "test.framework" : "testng",
+      "test.framework_version" : ${content_meta_test_framework_version},
+      "test.module" : "testng-7.0",
+      "test.source.file" : "dummy_source_path",
+      "test.status" : "fail",
+      "test.suite" : "org.example.TestFailAndThenSucceed",
+      "test.type" : "test",
+      "test_session.name" : "session-name"
+    },
+    "metrics" : {
+      "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count},
+      "test.source.end" : 19,
+      "test.source.start" : 11
+    },
+    "name" : "testng.test_suite",
+    "resource" : "org.example.TestFailAndThenSucceed",
+    "service" : "worker.org.gradle.process.internal.worker.gradleworkermain",
+    "start" : ${content_start},
+    "test_module_id" : ${content_test_module_id},
+    "test_session_id" : ${content_test_session_id},
+    "test_suite_id" : ${content_test_suite_id}
+  },
+  "type" : "test_suite_end",
+  "version" : 1
+}, {
+  "content" : {
+    "duration" : ${content_duration_2},
+    "error" : 1,
+    "meta" : {
+      "_dd.profiling.ctx" : "test",
+      "_dd.tracer_host" : ${content_meta__dd_tracer_host},
+      "component" : "testng",
+      "dummy_ci_tag" : "dummy_ci_tag_value",
+      "env" : "none",
+      "error.message" : ${content_meta_error_message},
+      "error.stack" : ${content_meta_error_stack},
+      "error.type" : "java.lang.AssertionError",
+      "language" : "jvm",
+      "library_version" : ${content_meta_library_version},
+      "runtime-id" : ${content_meta_runtime_id},
+      "span.kind" : "test",
+      "test.codeowners" : "[\"owner1\",\"owner2\"]",
+      "test.framework" : "testng",
+      "test.framework_version" : ${content_meta_test_framework_version},
+      "test.is_new" : "true",
+      "test.module" : "testng-7.0",
+      "test.name" : "test_fail_and_then_succeed",
+      "test.source.file" : "dummy_source_path",
+      "test.source.method" : "test_fail_and_then_succeed()V",
+      "test.status" : "fail",
+      "test.suite" : "org.example.TestFailAndThenSucceed",
+      "test.type" : "test",
+      "test_session.name" : "session-name"
+    },
+    "metrics" : {
+      "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_2},
+      "_dd.profiling.enabled" : 0,
+      "_dd.trace_span_attribute_schema" : 0,
+      "process_id" : ${content_metrics_process_id},
+      "test.source.end" : 18,
+      "test.source.start" : 12
+    },
+    "name" : "testng.test",
+    "parent_id" : ${content_parent_id},
+    "resource" : "org.example.TestFailAndThenSucceed.test_fail_and_then_succeed",
+    "service" : "worker.org.gradle.process.internal.worker.gradleworkermain",
+    "span_id" : ${content_span_id},
+    "start" : ${content_start_2},
+    "test_module_id" : ${content_test_module_id},
+    "test_session_id" : ${content_test_session_id},
+    "test_suite_id" : ${content_test_suite_id},
+    "trace_id" : ${content_trace_id}
+  },
+  "type" : "test",
+  "version" : 2
+}, {
+  "content" : {
+    "duration" : ${content_duration_3},
+    "error" : 0,
+    "meta" : {
+      "_dd.profiling.ctx" : "test",
+      "_dd.tracer_host" : ${content_meta__dd_tracer_host},
+      "component" : "testng",
+      "dummy_ci_tag" : "dummy_ci_tag_value",
+      "env" : "none",
+      "language" : "jvm",
+      "library_version" : ${content_meta_library_version},
+      "runtime-id" : ${content_meta_runtime_id},
+      "span.kind" : "test",
+      "test.codeowners" : "[\"owner1\",\"owner2\"]",
+      "test.final_status" : "fail",
+      "test.framework" : "testng",
+      "test.framework_version" : ${content_meta_test_framework_version},
+      "test.is_new" : "true",
+      "test.is_retry" : "true",
+      "test.module" : "testng-7.0",
+      "test.name" : "test_fail_and_then_succeed",
+      "test.retry_reason" : "early_flake_detection",
+      "test.source.file" : "dummy_source_path",
+      "test.source.method" : "test_fail_and_then_succeed()V",
+      "test.status" : "pass",
+      "test.suite" : "org.example.TestFailAndThenSucceed",
+      "test.type" : "test",
+      "test_session.name" : "session-name"
+    },
+    "metrics" : {
+      "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_3},
+      "_dd.profiling.enabled" : 0,
+      "_dd.trace_span_attribute_schema" : 0,
+      "process_id" : ${content_metrics_process_id},
+      "test.source.end" : 18,
+      "test.source.start" : 12
+    },
+    "name" : "testng.test",
+    "parent_id" : ${content_parent_id},
+    "resource" : "org.example.TestFailAndThenSucceed.test_fail_and_then_succeed",
+    "service" : "worker.org.gradle.process.internal.worker.gradleworkermain",
+    "span_id" : ${content_span_id_2},
+    "start" : ${content_start_3},
+    "test_module_id" : ${content_test_module_id},
+    "test_session_id" : ${content_test_session_id},
+    "test_suite_id" : ${content_test_suite_id},
+    "trace_id" : ${content_trace_id_2}
+  },
+  "type" : "test",
+  "version" : 2
+}, {
+  "content" : {
+    "duration" : ${content_duration_4},
+    "error" : 0,
+    "meta" : {
+      "_dd.p.tid" : ${content_meta__dd_p_tid_2},
+      "_dd.profiling.ctx" : "test",
+      "_dd.tracer_host" : ${content_meta__dd_tracer_host},
+      "component" : "testng",
+      "dummy_ci_tag" : "dummy_ci_tag_value",
+      "env" : "none",
+      "language" : "jvm",
+      "library_version" : ${content_meta_library_version},
+      "runtime-id" : ${content_meta_runtime_id},
+      "span.kind" : "test_session_end",
+      "test.command" : "testng-7.0",
+      "test.early_flake.enabled" : "true",
+      "test.framework" : "testng",
+      "test.framework_version" : ${content_meta_test_framework_version},
+      "test.status" : "fail",
+      "test.type" : "test",
+      "test_session.name" : "session-name"
+    },
+    "metrics" : {
+      "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_4},
+      "_dd.profiling.enabled" : 0,
+      "_dd.trace_span_attribute_schema" : 0,
+      "process_id" : ${content_metrics_process_id}
+    },
+    "name" : "testng.test_session",
+    "resource" : "testng-7.0",
+    "service" : "worker.org.gradle.process.internal.worker.gradleworkermain",
+    "start" : ${content_start_4},
+    "test_session_id" : ${content_test_session_id}
+  },
+  "type" : "test_session_end",
+  "version" : 1
+}, {
+  "content" : {
+    "duration" : ${content_duration_5},
+    "error" : 0,
+    "meta" : {
+      "_dd.p.tid" : ${content_meta__dd_p_tid_3},
+      "component" : "testng",
+      "dummy_ci_tag" : "dummy_ci_tag_value",
+      "env" : "none",
+      "library_version" : ${content_meta_library_version},
+      "span.kind" : "test_module_end",
+      "test.early_flake.enabled" : "true",
+      "test.framework" : "testng",
+      "test.framework_version" : ${content_meta_test_framework_version},
+      "test.module" : "testng-7.0",
+      "test.status" : "fail",
+      "test.type" : "test",
+      "test_session.name" : "session-name"
+    },
+    "metrics" : {
+      "_dd.host.vcpu_count" : ${content_metrics__dd_host_vcpu_count_5}
+    },
+    "name" : "testng.test_module",
+    "resource" : "testng-7.0",
+    "service" : "worker.org.gradle.process.internal.worker.gradleworkermain",
+    "start" : ${content_start_5},
+    "test_module_id" : ${content_test_module_id},
+    "test_session_id" : ${content_test_session_id}
+  },
+  "type" : "test_module_end",
+  "version" : 1
+} ]

dd-java-agent/instrumentation/testng/testng-7.0/src/test/resources/test-efd-new-flaky-test-pass-fail/coverages.ftl

@@ -0,0 +1 @@
+[ ]