Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,12 @@ tasks.named("check") {
}

tasks.withType<Test>().configureEach {
// Flaky tests management for JUnit 5
(options as? JUnitPlatformOptions)?.apply {
if (skipFlakyTestsProvider.isPresent) {
excludeTags("flaky")
} else if (runFlakyTestsProvider.isPresent) {
includeTags("flaky")
}
// Keep suites without test-utils out of flaky-only runs. Runtime extensions refine this tag.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-PskipFlakyTests no longer excludes flaky tests at JUnit discovery time. The old code called (options as? JUnitPlatformOptions)?.excludeTags("flaky") in skip mode, which skipped the whole class at discovery. That's been dropped in favor of FlakyJUnitExtension's per-method ExecutionCondition, which only disables the individual @Test method — not the container — when @Flaky is method-level rather than class-level.

For classes like CustomSystemLoaderSmokeTest / SampleTraceSmokeTest (annotated @Flaky only on the test method), running ./gradlew test -PskipFlakyTests will still trigger the class's @RegisterExtension beforeAll — launching the smoke-app subprocess and waiting on it — before the method itself is finally disabled. That defeats the point of -PskipFlakyTests, which was meant to avoid exactly that expensive setup.

Worth adding an equivalent discovery-time (or container-level) skip path for -PskipFlakyTests, e.g. extending FlakyJUnitFilter/the extension to also check for method-level @Flaky and disable at the container level, or restoring an excludeTags mechanism that JUnit5's tag-based filtering can still apply.

if (!skipFlakyTestsProvider.isPresent && runFlakyTestsProvider.isPresent) {
(options as? JUnitPlatformOptions)?.includeTags("flaky")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer add excludeTags("flaky") when skipFlakyTestsProvider.isPresent. This has no effective behavioral change.

We needed to remove this because excludeTags("flaky") runs before JUnit evaluates any conditions, so all @Flaky tests are skipped immediately.

The only change is that a raw @Tag("flaky") without @Flaky will no longer be skipped by -PskipFlakyTests, but @Tag("flaky") isn't used in the library so not relevant here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include a Forbidden API for @Tag("flaky") to ensure it won't be used accidentally in the future?

@sarahchen6 sarahchen6 Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Forbidden API can only forbid the tag in general and not flaky specification (unless I'm missing something), so I clarified one of the comments instead to note that @Tag("flaky") doesn't work!

}

// Set system property flag that is checked from tests to determine if they should be skipped or run
// Let the JUnit and Spock extensions evaluate @Flaky conditions before selecting tests.
if (skipFlakyTestsProvider.isPresent) {
jvmArgs("-Drun.flaky.tests=false")
} else if (runFlakyTestsProvider.isPresent) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package datadog.smoketest;

import static datadog.smoketest.backend.AgentBackend.testAgent;
import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.regex.Pattern.compile;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import datadog.environment.JavaVirtualMachine;
import datadog.trace.test.util.Flaky;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class CustomSystemLoaderSmokeTest {
Comment thread
sarahchen6 marked this conversation as resolved.
@RegisterExtension
static final SmokeCliApp app =
SmokeCliApp.named("custom-systemloader")
.jar(System.getProperty("datadog.smoketest.systemloader.shadowJar.path"))
.jvmArgs("-Djava.system.class.loader=datadog.smoketest.systemloader.TestLoader")
.workingDirectory(new File(System.getProperty("datadog.smoketest.builddir")))
.debugLogs()
.backend(testAgent())
// The app may exit before sending telemetry.
.skipTelemetryCheck()
.build();

@Test
@DisplayName("resource types loaded by custom system class-loader are transformed")
@Flaky(value = "Race condition with IBM. Check APMAPI-1194", condition = IbmJvm.class)
void resourceTypesLoadedByCustomSystemClassLoaderAreTransformed() {
app.assertCompletesWithValue(30, SECONDS, 0);

List<String> logLines = new ArrayList<>();
assertTrue(
app.waitForLogLine(
line -> {
logLines.add(line);
return "FIN".equals(line);
}));
Predicate<String> loadedResource =
compile("Loading sample.app.Resource[$]Test[1-3] from TestLoader").asPredicate();
Predicate<String> transformedResource =
compile(
"Transformed.*class=sample.app.Resource[$]Test[1-3].*classloader=datadog.smoketest.systemloader.TestLoader")
.asPredicate();
assertEquals(3, logLines.stream().filter(loadedResource).count());
assertEquals(3, logLines.stream().filter(transformedResource).count());
}

static class IbmJvm implements Predicate<String> {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of Predicate that returns true for the condition that we want to mark as flaky is what this PR proposes as the solution to Flaky conditionals in JUnit.

@Override
public boolean test(String suite) {
return JavaVirtualMachine.isIbm();
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package datadog.smoketest;

import static datadog.smoketest.backend.AgentBackend.testAgent;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.jupiter.api.Assertions.assertEquals;

import datadog.environment.JavaVirtualMachine;
import datadog.smoketest.backend.AgentBackend;
import datadog.trace.test.util.Flaky;
import java.io.File;
import java.util.function.Predicate;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class SampleTraceSmokeTest {
private static final AgentBackend BACKEND = testAgent();

@RegisterExtension
static final SmokeCliApp app =
SmokeCliApp.named("sample-trace")
.jar(System.getProperty("datadog.smoketest.agent.shadowJar.path"))
// tests tracer as a jar sending sample traces instead of a javaagent
.noAgent()
.backend(BACKEND)
.placeholder("agent.host", () -> BACKEND.url().getHost())
.placeholder("agent.port", () -> Integer.toString(BACKEND.port()))
.jvmArgs(
"-Ddd.agent.host=${agent.host}",
"-Ddd.trace.agent.port=${agent.port}",
"-Ddd.test.agent.session.token=" + BACKEND.sessionToken())
.args("sampleTrace", "-c", "10", "-i", "0.1")
.workingDirectory(new File(System.getProperty("datadog.smoketest.builddir")))
.build();

@Test
@DisplayName("sample traces are sent")
@Flaky(condition = IbmJvm.class)
void sampleTracesAreSent() {
app.traces().waitForTraceCount(10);
app.assertCompletesWithValue(30, SECONDS, 0);
assertEquals(10, app.traces().getTraces().size());
}

static class IbmJvm implements Predicate<String> {
@Override
public boolean test(String suite) {
return JavaVirtualMachine.isIbm();
}
}
}
29 changes: 27 additions & 2 deletions docs/how_to_test.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,33 @@ This mechanism exists to make sure either java agent state or static data are re

### Flaky Tests

If a test runs unreliably, or doesn't have a fully deterministic behavior, this will lead to recurrent unexpected errors in continuous integration.
In order to identify such tests and avoid the continuous integration to fail, they are marked as _flaky_ and must be annotated with the `@Flaky` annotation.
Mark unreliable test methods or classes with `@Flaky` in both JUnit and Spock.

All tests run by default. Use `-PskipFlakyTests` to skip flaky tests or `-PrunFlakyTests` to run only flaky tests.

If a test is flaky only in certain environments, use `condition`. In Java, supply a predicate class
with a no-argument constructor. Its `test` method returns `true` when the test is flaky:

```java
import datadog.environment.JavaVirtualMachine;
import java.util.function.Predicate;

@Test
@Flaky(condition = IbmJvm.class)
void testOnSupportedJvms() {
// ...
}

static class IbmJvm implements Predicate<String> {
@Override
public boolean test(String suite) {
return JavaVirtualMachine.isIbm();
}
}
```

Use `suites = {"SomeSubclass"}` to limit the annotation to particular test classes, such as subclasses
that inherit a test. When both `suites` and `condition` are specified, both must match.

> [!TIP]
> In case your pull request checks failed due to some unexpected flaky tests, you can retry the continuous
Expand Down
1 change: 1 addition & 0 deletions utils/test-utils/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ dependencies {

compileOnly(project(":components:annotations"))
compileOnly(libs.junit.jupiter)
compileOnly(libs.junit.platform.launcher)
compileOnly(libs.logback.core)
compileOnly(libs.logback.classic)

Expand Down
2 changes: 1 addition & 1 deletion utils/test-utils/gradle.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ org.junit.jupiter:junit-jupiter-params:5.14.1=compileClasspath,testCompileClassp
org.junit.jupiter:junit-jupiter:5.14.1=compileClasspath,testCompileClasspath,testRuntimeClasspath
org.junit.platform:junit-platform-commons:1.14.1=compileClasspath,testCompileClasspath,testRuntimeClasspath
org.junit.platform:junit-platform-engine:1.14.1=compileClasspath,testCompileClasspath,testRuntimeClasspath
org.junit.platform:junit-platform-launcher:1.14.1=testRuntimeClasspath
org.junit.platform:junit-platform-launcher:1.14.1=compileClasspath,testRuntimeClasspath
org.junit:junit-bom:5.14.1=compileClasspath,testCompileClasspath,testRuntimeClasspath
org.mockito:mockito-core:4.4.0=testRuntimeClasspath
org.objenesis:objenesis:3.3=compileClasspath,testCompileClasspath,testRuntimeClasspath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,30 @@
import java.lang.annotation.Target;
import java.util.function.Predicate;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.extension.ExtendWith;

/**
* Use this annotation for suites or test cases that are flaky. When running in CI, these will be
* segregated to a separate job.
* split to a separate job. Apply this annotation instead of {@code @Tag("flaky")} directly.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE, ElementType.METHOD})
@Tag("flaky")
Comment thread
sarahchen6 marked this conversation as resolved.
@ExtendWith(FlakyJUnitExtension.class)
public @interface Flaky {
/** Reason why the test is flaky (optional). */
String value() default "";

/**
* Fully qualified name of the test suite classes where this test is flaky. Only required when the
* test is flaky only when run in a subclass.
* Names of the test suite classes where this test is flaky, typically subclasses that inherit the
* test. Spock uses simple class names; JUnit accepts simple or fully qualified class names.
*/
String[] suites() default {};

/**
* Closure with a predicate to test at runtime if the actual spec is flaky (e.g. check the JVM
* vendor), the parameter is the actual name of the spec under test
* Predicate class with a no-argument constructor that determines whether the test is flaky (e.g.
* check the JVM vendor). JUnit passes the concrete test class's simple name. Spock also supports
* Groovy closures.
*/
Class<? extends Predicate<String>> condition() default True.class;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package datadog.trace.test.util;

import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.util.function.Predicate;
import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
import org.junit.jupiter.api.extension.ExtensionConfigurationException;
import org.junit.jupiter.api.extension.ExtensionContext;

/** Selects JUnit tests using the same flaky-test modes as {@link FlakySpockExtension}. */
public final class FlakyJUnitExtension implements ExecutionCondition {
private static final String RUN_FLAKY_TESTS = "run.flaky.tests";

@Override
public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) {
if (!"false".equals(System.getProperty(RUN_FLAKY_TESTS))
|| !context.getTestClass().isPresent()) {
return ConditionEvaluationResult.enabled("Flaky tests are not skipped");
}
Flaky flaky = findFlaky(context.getRequiredTestClass(), context.getTestMethod().orElse(null));
if (flaky == null) {
return ConditionEvaluationResult.enabled("Test is not flaky");
}
return ConditionEvaluationResult.disabled(
flaky.value().isEmpty() ? "Flaky test" : "Flaky test: " + flaky.value());
}

static Flaky findFlaky(Class<?> testClass, Method method) {
for (Class<?> enclosing = testClass;
enclosing != null;
enclosing = enclosing.getEnclosingClass()) {
for (Class<?> current = enclosing; current != null; current = current.getSuperclass()) {
Flaky flaky = matchingAnnotation(current, testClass);
if (flaky != null) {
return flaky;
}
}
}
return method == null ? null : matchingAnnotation(method, testClass);
}

private static Flaky matchingAnnotation(AnnotatedElement element, Class<?> testClass) {
Flaky flaky = findAnnotation(element, Flaky.class).orElse(null);
if (flaky == null) {
return null;
}
if (flaky.suites().length > 0) {
boolean matches = false;
for (String suite : flaky.suites()) {
if (suite.equals(testClass.getSimpleName()) || suite.equals(testClass.getName())) {
matches = true;
break;
}
}
if (!matches) {
return null;
}
}
if (flaky.condition() == Flaky.True.class) {
return flaky;
}
try {
Constructor<? extends Predicate<String>> constructor =
flaky.condition().getDeclaredConstructor();
constructor.setAccessible(true);
return constructor.newInstance().test(testClass.getSimpleName()) ? flaky : null;
} catch (ReflectiveOperationException | RuntimeException e) {
throw new ExtensionConfigurationException(
"Could not evaluate @Flaky condition " + flaky.condition().getName() + " on " + element,
e);
}
}
}
Loading
Loading