diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java index 1a4ca843aa4..05d9a0130f3 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java @@ -25,9 +25,9 @@ TestSuiteImpl testSuiteStart( * @return {@code true} if the test is new, {@code false} if it is an existing test or if the * list of known tests is not available. */ - boolean isNew(TestIdentifier test); + boolean isNew(@Nonnull TestIdentifier test); - boolean isFlaky(TestIdentifier test); + boolean isFlaky(@Nonnull TestIdentifier test); boolean isModified(TestSourceData testSourceData); 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 index 29f2e643912..679b3c46894 100644 --- 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 @@ -46,6 +46,7 @@ import java.util.Collection; import java.util.Collections; import java.util.function.Consumer; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -190,6 +191,7 @@ private void populateSourceDataTags( } } + @Nonnull public TestIdentifier getIdentifier() { return identifier; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java index b0b7572379a..8784278770b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java @@ -90,12 +90,12 @@ public ProxyTestModule( } @Override - public boolean isNew(TestIdentifier test) { + public boolean isNew(@Nonnull TestIdentifier test) { return executionStrategy.isNew(test); } @Override - public boolean isFlaky(TestIdentifier test) { + public boolean isFlaky(@Nonnull TestIdentifier test) { return executionStrategy.isFlaky(test); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java index 82362eb9baf..997b8fbc020 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java @@ -75,12 +75,12 @@ public HeadlessTestModule( } @Override - public boolean isNew(TestIdentifier test) { + public boolean isNew(@Nonnull TestIdentifier test) { return executionStrategy.isNew(test); } @Override - public boolean isFlaky(TestIdentifier test) { + public boolean isFlaky(@Nonnull TestIdentifier test) { return executionStrategy.isFlaky(test); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java index fc8b4613b82..2a0f408a56e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java @@ -105,12 +105,12 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData s } @Override - public boolean isNew(TestIdentifier test) { + public boolean isNew(@Nonnull TestIdentifier test) { return false; } @Override - public boolean isFlaky(TestIdentifier test) { + public boolean isFlaky(@Nonnull TestIdentifier test) { return false; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java index 79ce2af9161..d94cfa89008 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java @@ -300,12 +300,12 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData t } @Override - public boolean isNew(TestIdentifier test) { + public boolean isNew(@Nonnull TestIdentifier test) { return testModule.isNew(test); } @Override - public boolean isFlaky(TestIdentifier test) { + public boolean isFlaky(@Nonnull TestIdentifier test) { return testModule.isFlaky(test); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java index 3bab64b55ce..7f2f794120e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java @@ -53,12 +53,12 @@ public ExecutionSettings getExecutionSettings() { return executionSettings; } - public boolean isNew(TestIdentifier test) { + public boolean isNew(@Nonnull TestIdentifier test) { return executionSettings.isKnownTestsDataAvailable() && !executionSettings.isKnown(test.toFQN()); } - public boolean isFlaky(TestIdentifier test) { + public boolean isFlaky(@Nonnull TestIdentifier test) { return executionSettings.isFlaky(test.toFQN()); } @@ -163,7 +163,7 @@ private boolean isAutoRetryApplicable(TestIdentifier test) { && autoRetriesUsed.get() < config.getCiVisibilityTotalFlakyRetryCount(); } - private boolean isEFDApplicable(TestIdentifier test, TestSourceData testSource) { + private boolean isEFDApplicable(@Nonnull TestIdentifier test, TestSourceData testSource) { EarlyFlakeDetectionSettings efdSettings = executionSettings.getEarlyFlakeDetectionSettings(); return efdSettings.isEnabled() && !isEFDLimitReached() diff --git a/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy b/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy index da5c0787886..e25d41f6402 100644 --- a/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy @@ -59,7 +59,9 @@ import java.util.function.Predicate abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { - static String dummyModule + public static final int SLOW_TEST_THRESHOLD_MILLIS = 1000 + public static final int VERY_SLOW_TEST_THRESHOLD_MILLIS = 2000 + static final String DUMMY_CI_TAG = "dummy_ci_tag" static final String DUMMY_CI_TAG_VALUE = "dummy_ci_tag_value" static final String DUMMY_SOURCE_PATH = "dummy_source_path" @@ -69,29 +71,50 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { static final int DUMMY_TEST_CLASS_END = 19 static final Collection DUMMY_CODE_OWNERS = ["owner1", "owner2"] - private static Path agentKeyFile + static final AGENT_KEY_FILE = Files.createTempFile("TestFrameworkTest", "dummy_agent_key") - private static final List skippableTests = new ArrayList<>() - private static final List flakyTests = new ArrayList<>() - private static final List knownTests = new ArrayList<>() - private static final List quarantinedTests = new ArrayList<>() - private static final List disabledTests = new ArrayList<>() - private static final List attemptToFixTests = new ArrayList<>() - private static volatile Diff diff = LineDiff.EMPTY + def cleanupSpec() { + Files.deleteIfExists(AGENT_KEY_FILE) + } - private static volatile boolean itrEnabled = false - private static volatile boolean flakyRetryEnabled = false - private static volatile boolean earlyFlakinessDetectionEnabled = false - private static volatile boolean impactedTestsDetectionEnabled = false - private static volatile boolean testManagementEnabled = false + @Override + void configurePreAgent() { + super.configurePreAgent() - public static final int SLOW_TEST_THRESHOLD_MILLIS = 1000 - public static final int VERY_SLOW_TEST_THRESHOLD_MILLIS = 2000 + Files.write(AGENT_KEY_FILE, "dummy".getBytes()) - def setupSpec() { - def currentPath = Paths.get("").toAbsolutePath() - def rootPath = currentPath.parent - dummyModule = rootPath.relativize(currentPath) + injectSysConfig(GeneralConfig.API_KEY_FILE, AGENT_KEY_FILE.toString()) + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_ENABLED, "true") + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED, "true") + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED, "true") + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_FLAKY_RETRY_ENABLED, "true") + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_EARLY_FLAKE_DETECTION_LOWER_LIMIT, "1") + injectSysConfig(CiVisibilityConfig.TEST_MANAGEMENT_ENABLED, "true") + injectSysConfig(CiVisibilityConfig.TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES, "5") + injectSysConfig(CiVisibilityConfig.CIVISIBILITY_TEST_ORDER, CIConstants.FAIL_FAST_TEST_ORDER) + } + + private static final class Settings { + private volatile List skippableTests = [] + private volatile List flakyTests + private volatile List knownTests + private volatile List quarantinedTests = [] + private volatile List disabledTests = [] + private volatile List attemptToFixTests = [] + private volatile Diff diff = LineDiff.EMPTY + + private volatile boolean itrEnabled + private volatile boolean flakyRetryEnabled + private volatile boolean earlyFlakinessDetectionEnabled + private volatile boolean impactedTestsDetectionEnabled + private volatile boolean testManagementEnabled + } + + private final Settings settings = new Settings() + + @Override + def setup() { + TEST_WRITER.setFilter(spanFilter) def ciProvider = Provider.GITHUBACTIONS @@ -110,44 +133,7 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { linesResolver.getMethodLines(_ as Method) >> new LinesResolver.Lines(DUMMY_TEST_METHOD_START, DUMMY_TEST_METHOD_END) linesResolver.getClassLines(_ as Class) >> new LinesResolver.Lines(DUMMY_TEST_CLASS_START, DUMMY_TEST_CLASS_END) - def executionSettingsFactory = new ExecutionSettingsFactory() { - @Override - ExecutionSettings create(JvmInfo jvmInfo, String moduleName) { - def earlyFlakinessDetectionSettings = earlyFlakinessDetectionEnabled - ? new EarlyFlakeDetectionSettings(true, [ - new ExecutionsByDuration(SLOW_TEST_THRESHOLD_MILLIS, 3), - new ExecutionsByDuration(VERY_SLOW_TEST_THRESHOLD_MILLIS, 2) - ], 0) - : EarlyFlakeDetectionSettings.DEFAULT - - def testManagementSettings = testManagementEnabled - ? new TestManagementSettings(true, 5) - : TestManagementSettings.DEFAULT - - Map skippableTestsWithMetadata = new HashMap<>() - for (TestIdentifier skippableTest : skippableTests) { - skippableTestsWithMetadata.put(skippableTest, new TestMetadata(false)) - } - - return new ExecutionSettings( - itrEnabled, - false, - itrEnabled, - flakyRetryEnabled, - impactedTestsDetectionEnabled, - earlyFlakinessDetectionSettings, - testManagementSettings, - itrEnabled ? "itrCorrelationId" : null, - skippableTestsWithMetadata, - [:], - flakyTests, - earlyFlakinessDetectionEnabled || CIConstants.FAIL_FAST_TEST_ORDER.equalsIgnoreCase(Config.get().ciVisibilityTestOrder) ? knownTests : null, - quarantinedTests, - disabledTests, - attemptToFixTests, - diff) - } - } + def executionSettingsFactory = new MockExecutionSettingsFactory(settings) def coverageStoreFactory = new FileCoverageStore.Factory(metricCollector, sourcePathResolver) TestFrameworkSession.Factory testFrameworkSessionFactory = (String projectName, String component, Long startTime) -> { @@ -173,7 +159,10 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { ) } - InstrumentationBridge.registerTestEventsHandlerFactory(new TestEventHandlerFactory(testFrameworkSessionFactory, metricCollector)) + def currentPath = Paths.get("").toAbsolutePath() + def rootPath = currentPath.parent + def moduleName = rootPath.relativize(currentPath) + InstrumentationBridge.registerTestEventsHandlerFactory(new MockTestEventHandlerFactory(testFrameworkSessionFactory, metricCollector, moduleName)) BuildSystemSession.Factory buildSystemSessionFactory = (String projectName, Path projectRoot, String startCommand, String component, Long startTime) -> { def config = Config.get() @@ -208,19 +197,66 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { CoveragePerTestBridge.registerCoverageStoreRegistry(coverageStoreFactory) } - private static final class TestEventHandlerFactory implements TestEventsHandler.Factory { + private static final class MockExecutionSettingsFactory implements ExecutionSettingsFactory { + private final Settings settings + + MockExecutionSettingsFactory(Settings settings) { + this.settings = settings + } + + @Override + ExecutionSettings create(JvmInfo jvmInfo, String moduleName) { + def earlyFlakinessDetectionSettings = settings.earlyFlakinessDetectionEnabled + ? new EarlyFlakeDetectionSettings(true, [ + new ExecutionsByDuration(SLOW_TEST_THRESHOLD_MILLIS, 3), + new ExecutionsByDuration(VERY_SLOW_TEST_THRESHOLD_MILLIS, 2) + ], 0) + : EarlyFlakeDetectionSettings.DEFAULT + + def testManagementSettings = settings.testManagementEnabled + ? new TestManagementSettings(true, 5) + : TestManagementSettings.DEFAULT + + Map skippableTestsWithMetadata = new HashMap<>() + for (TestIdentifier skippableTest : settings.skippableTests) { + skippableTestsWithMetadata.put(skippableTest, new TestMetadata(false)) + } + + return new ExecutionSettings( + settings.itrEnabled, + false, + settings.itrEnabled, + settings.flakyRetryEnabled, + settings.impactedTestsDetectionEnabled, + earlyFlakinessDetectionSettings, + testManagementSettings, + settings.itrEnabled ? "itrCorrelationId" : null, + skippableTestsWithMetadata, + [:], + settings.flakyTests, + settings.knownTests, + settings.quarantinedTests, + settings.disabledTests, + settings.attemptToFixTests, + settings.diff) + } + } + + private static final class MockTestEventHandlerFactory implements TestEventsHandler.Factory { private final TestFrameworkSession.Factory testFrameworkSessionFactory private final CiVisibilityMetricCollector metricCollector + private final String moduleName - TestEventHandlerFactory(testFrameworkSessionFactory, metricCollector) { + MockTestEventHandlerFactory(testFrameworkSessionFactory, metricCollector, moduleName) { this.testFrameworkSessionFactory = testFrameworkSessionFactory this.metricCollector = metricCollector + this.moduleName = moduleName } @Override TestEventsHandler create(String component, ContextStore suiteStore, ContextStore testStore) { - TestFrameworkSession testSession = testFrameworkSessionFactory.startSession(dummyModule, component, null) - TestFrameworkModule testModule = testSession.testModuleStart(dummyModule, null) + TestFrameworkSession testSession = testFrameworkSessionFactory.startSession(moduleName, component, null) + TestFrameworkModule testModule = testSession.testModuleStart(moduleName, null) new TestEventsHandlerImpl(metricCollector, testSession, testModule, suiteStore != null ? suiteStore : new ConcurrentHashMapContextStore<>(), testStore != null ? testStore : new ConcurrentHashMapContextStore<>()) @@ -261,91 +297,48 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner { } } - @Override - void setup() { - skippableTests.clear() - flakyTests.clear() - knownTests.clear() - quarantinedTests.clear() - disabledTests.clear() - attemptToFixTests.clear() - diff = LineDiff.EMPTY - itrEnabled = false - flakyRetryEnabled = false - earlyFlakinessDetectionEnabled = false - impactedTestsDetectionEnabled = false - testManagementEnabled = false - - TEST_WRITER.setFilter(spanFilter) - } - def givenSkippableTests(List tests) { - skippableTests.addAll(tests) - itrEnabled = true + settings.skippableTests = new ArrayList<>(tests) + settings.itrEnabled = true } def givenFlakyTests(List tests) { - flakyTests.addAll(tests) + settings.flakyTests = new ArrayList<>(tests) } def givenKnownTests(List tests) { - knownTests.addAll(tests) + settings.knownTests = new ArrayList<>(tests) } def givenQuarantinedTests(List tests) { - quarantinedTests.addAll(tests) - testManagementEnabled = true + settings.quarantinedTests = new ArrayList<>(tests) + settings.testManagementEnabled = true } def givenDisabledTests(List tests) { - disabledTests.addAll(tests) - testManagementEnabled = true + settings.disabledTests = new ArrayList<>(tests) + settings.testManagementEnabled = true } def givenAttemptToFixTests(List tests) { - attemptToFixTests.addAll(tests) - testManagementEnabled = true + settings.attemptToFixTests = new ArrayList<>(tests) + settings.testManagementEnabled = true } def givenDiff(Diff diff) { - this.diff = diff + settings.diff = diff } def givenFlakyRetryEnabled(boolean flakyRetryEnabled) { - this.flakyRetryEnabled = flakyRetryEnabled + settings.flakyRetryEnabled = flakyRetryEnabled } def givenEarlyFlakinessDetectionEnabled(boolean earlyFlakinessDetectionEnabled) { - this.earlyFlakinessDetectionEnabled = earlyFlakinessDetectionEnabled + settings.earlyFlakinessDetectionEnabled = earlyFlakinessDetectionEnabled } def givenImpactedTestsDetectionEnabled(boolean impactedTestsDetectionEnabled) { - this.impactedTestsDetectionEnabled = impactedTestsDetectionEnabled - } - - def givenTestsOrder(String testsOrder) { - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_TEST_ORDER, testsOrder) - } - - @Override - void configurePreAgent() { - super.configurePreAgent() - - agentKeyFile = Files.createTempFile("TestFrameworkTest", "dummy_agent_key") - Files.write(agentKeyFile, "dummy".getBytes()) - - injectSysConfig(GeneralConfig.API_KEY_FILE, agentKeyFile.toString()) - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_ENABLED, "true") - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED, "true") - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED, "true") - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_FLAKY_RETRY_ENABLED, "true") - injectSysConfig(CiVisibilityConfig.CIVISIBILITY_EARLY_FLAKE_DETECTION_LOWER_LIMIT, "1") - injectSysConfig(CiVisibilityConfig.TEST_MANAGEMENT_ENABLED, "true") - injectSysConfig(CiVisibilityConfig.TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES, "5") - } - - def cleanupSpec() { - Files.deleteIfExists(agentKeyFile) + settings.impactedTestsDetectionEnabled = impactedTestsDetectionEnabled } def assertSpansData(String testcaseName, Map replacements = [:]) { diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy index 7079be44751..03e2ed6c2f7 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy @@ -1,5 +1,4 @@ import datadog.trace.api.DisableTestTrace -import datadog.trace.api.civisibility.CIConstants import datadog.trace.civisibility.CiVisibilityInstrumentationTest import datadog.trace.instrumentation.junit5.TestEventsHandlerHolder import org.example.* @@ -22,12 +21,6 @@ import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass @DisableTestTrace(reason = "avoid self-tracing") class JUnit58Test extends CiVisibilityInstrumentationTest { - @Override - void configurePreAgent() { - super.configurePreAgent() - givenTestsOrder(CIConstants.FAIL_FAST_TEST_ORDER) - } - def "test #testcaseName"() { runTests(tests, success) diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-all-after-all/events.ftl b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-all-after-all/events.ftl index 5b7a75f2ac9..8c831f4aa29 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-all-after-all/events.ftl +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-all-after-all/events.ftl @@ -117,7 +117,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "another_test_succeed", "test.source.file" : "dummy_source_path", @@ -165,7 +164,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-each-after-each/events.ftl b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-each-after-each/events.ftl index 6d2c5d5c95c..7bcbb753c7b 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-each-after-each/events.ftl +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-before-each-after-each/events.ftl @@ -117,7 +117,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "another_test_succeed", "test.source.file" : "dummy_source_path", @@ -165,7 +164,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-all/events.ftl b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-all/events.ftl index 42a0fcfe47d..ff119a128b8 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-all/events.ftl +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-all/events.ftl @@ -120,7 +120,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "another_test_succeed", "test.source.file" : "dummy_source_path", @@ -168,7 +167,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-each/events.ftl b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-each/events.ftl index 22f95e7c8da..b57eb25fc68 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-each/events.ftl +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-after-each/events.ftl @@ -120,7 +120,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "another_test_succeed", "test.source.file" : "dummy_source_path", @@ -171,7 +170,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-before-each/events.ftl b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-before-each/events.ftl index 46bd5467bdd..5ad935a108d 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-before-each/events.ftl +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/resources/test-failed-before-each/events.ftl @@ -120,7 +120,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "another_test_succeed", "test.source.file" : "dummy_source_path", @@ -171,7 +170,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "junit5", "test.framework_version" : ${content_meta_test_framework_version}, - "test.is_new" : "true", "test.module" : "junit-5.8", "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java index b4e6b9f5d6d..0a6b01a8e9e 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java @@ -21,6 +21,9 @@ @AutoService(InstrumenterModule.class) public class TestNGInstrumentation extends InstrumenterModule.CiVisibility implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public static final int ORDER = 0; + public TestNGInstrumentation() { super("testng"); } @@ -30,6 +33,11 @@ public String instrumentedType() { return "org.testng.TestNG"; } + @Override + public int order() { + return ORDER; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( diff --git a/dd-java-agent/instrumentation/testng/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy b/dd-java-agent/instrumentation/testng/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy index 17090ba155f..636b3a6e07f 100644 --- a/dd-java-agent/instrumentation/testng/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy +++ b/dd-java-agent/instrumentation/testng/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.testng + import datadog.trace.api.civisibility.config.TestFQN import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.civisibility.CiVisibilityInstrumentationTest @@ -18,6 +19,7 @@ abstract class TestNGTest extends CiVisibilityInstrumentationTest { static ComparableVersion currentTestNGVersion = new ComparableVersion(TracingListener.FRAMEWORK_VERSION) static ComparableVersion testNGv75 = new ComparableVersion("7.5") + static ComparableVersion testNGv70 = new ComparableVersion("7.0") def "test #testcaseName"() { runTests(tests, null, success) @@ -241,6 +243,33 @@ abstract class TestNGTest extends CiVisibilityInstrumentationTest { "test-attempt-to-fix-disabled-succeeded" | true | [TestSucceed] | [new TestFQN("org.example.TestSucceed", "test_succeed")] | [] | [new TestFQN("org.example.TestSucceed", "test_succeed")] } + def "test known tests ordering #testcaseName"() { + Assumptions.assumeTrue(isTestOrderingSupported()) + + givenKnownTests(knownTestsList) + + runTests(tests) + + assertTestsOrder(expectedOrder) + + where: + testcaseName | tests | knownTestsList | expectedOrder + "ordering-methods" | [TestSucceedAndSkipped] | [test("org.example.TestSucceedAndSkipped", "test_skipped")] | [ + test("org.example.TestSucceedAndSkipped", "test_succeed"), + test("org.example.TestSucceedAndSkipped", "test_skipped") + ] + "ordering-classes" | [TestSucceedNested, TestSucceedAndSkipped] | [ + test('org.example.TestSucceedAndSkipped', 'test_succeed'), + test('org.example.TestSucceedAndSkipped', 'test_skipped'), + test('org.example.TestSucceedNested$NestedSuite', 'test_succeed_nested'), + ] | [ + test('org.example.TestSucceedNested', 'test_succeed'), + test('org.example.TestSucceedAndSkipped', 'test_skipped'), + test('org.example.TestSucceedAndSkipped', 'test_succeed'), + test('org.example.TestSucceedNested$NestedSuite', 'test_succeed_nested'), + ] + } + private static boolean isEFDSupported() { currentTestNGVersion >= testNGv75 } @@ -249,6 +278,10 @@ abstract class TestNGTest extends CiVisibilityInstrumentationTest { currentTestNGVersion >= testNGv75 } + private static boolean isTestOrderingSupported() { + currentTestNGVersion >= testNGv70 + } + protected void runTests(List testClasses, String parallelMode = null, boolean expectSuccess = true) { TestEventsHandlerHolder.start() diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/FailFastOrderInterceptor.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/FailFastOrderInterceptor.java new file mode 100644 index 00000000000..62c7734f246 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/FailFastOrderInterceptor.java @@ -0,0 +1,42 @@ +package datadog.trace.instrumentation.testng7.order; + +import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.events.TestEventsHandler; +import datadog.trace.api.civisibility.events.TestSuiteDescriptor; +import datadog.trace.instrumentation.testng.TestNGUtils; +import java.util.Comparator; +import java.util.List; +import org.testng.IMethodInstance; +import org.testng.IMethodInterceptor; +import org.testng.ITestContext; +import org.testng.ITestNGMethod; +import org.testng.ITestResult; +import org.testng.internal.TestResult; + +public class FailFastOrderInterceptor implements IMethodInterceptor { + + private final TestEventsHandler testEventsHandler; + private final Comparator knownAndStableTestsComparator; + + public FailFastOrderInterceptor( + TestEventsHandler testEventsHandler) { + this.testEventsHandler = testEventsHandler; + this.knownAndStableTestsComparator = Comparator.comparing(this::isKnownAndStable); + } + + private boolean isKnownAndStable(IMethodInstance methodInstance) { + ITestNGMethod method = methodInstance.getMethod(); + if (method == null) { + return true; + } + ITestResult testResult = TestResult.newTestResultFor(method); + TestIdentifier testIdentifier = TestNGUtils.toTestIdentifier(testResult); + return !testEventsHandler.isNew(testIdentifier) && !testEventsHandler.isFlaky(testIdentifier); + } + + @Override + public List intercept(List list, ITestContext iTestContext) { + list.sort(knownAndStableTestsComparator); + return list; + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java new file mode 100644 index 00000000000..e081ebcfdf0 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java @@ -0,0 +1,94 @@ +package datadog.trace.instrumentation.testng7.order; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Config; +import datadog.trace.api.civisibility.CIConstants; +import datadog.trace.instrumentation.testng.TestEventsHandlerHolder; +import datadog.trace.instrumentation.testng.TestNGInstrumentation; +import datadog.trace.instrumentation.testng.TestNGUtils; +import datadog.trace.util.Strings; +import java.util.List; +import java.util.Set; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import org.testng.IMethodInterceptor; +import org.testng.annotations.CustomAttribute; + +@AutoService(InstrumenterModule.class) +public class TestNGOrderInstrumentation extends InstrumenterModule.CiVisibility + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + private final String parentPackageName = Strings.getPackageName(TestNGUtils.class.getName()); + + public TestNGOrderInstrumentation() { + super("ci-visibility", "testng", "test-order"); + } + + @Override + public boolean isApplicable(Set enabledSystems) { + return super.isApplicable(enabledSystems) && Config.get().getCiVisibilityTestOrder() != null; + } + + @Override + public String instrumentedType() { + return "org.testng.TestNG"; + } + + @Override + public int order() { + // Depends on datadog.trace.instrumentation.testng.TestNGInstrumentation, + // as it needs datadog.trace.instrumentation.testng.TestEventsHandlerHolder.start to be called; + // The bytecode insertion order is inverse: + // for lower order values the bytecode instructions are executed later + // (at least for @Advice.OnMethodExit methods) + return TestNGInstrumentation.ORDER - 1; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + MethodDescription::isConstructor, + TestNGOrderInstrumentation.class.getName() + "$InsertInterceptorAdvice"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + parentPackageName + ".TestNGClassListener", + parentPackageName + ".TestNGUtils", + parentPackageName + ".TestEventsHandlerHolder", + packageName + ".FailFastOrderInterceptor", + }; + } + + public static class InsertInterceptorAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") + @Advice.OnMethodExit + public static void prependFailFastInterceptor( + @Advice.FieldValue("m_methodInterceptors") List methodInterceptors) { + String testOrder = Config.get().getCiVisibilityTestOrder(); + if (CIConstants.FAIL_FAST_TEST_ORDER.equalsIgnoreCase(testOrder)) { + for (IMethodInterceptor methodInterceptor : methodInterceptors) { + if (methodInterceptor instanceof FailFastOrderInterceptor) { + return; + } + } + + // adding our interceptor as the first one: + // that way custom interceptors added by the users will have higher priority + methodInterceptors.add( + 0, new FailFastOrderInterceptor(TestEventsHandlerHolder.TEST_EVENTS_HANDLER)); + + } else { + throw new IllegalArgumentException("Unknown test order: " + testOrder); + } + } + + // TestNG 7.0 and above + public static String muzzleCheck(final CustomAttribute customAttribute) { + return customAttribute.name(); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java index 7f683c515e8..0d40bb98201 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java @@ -99,9 +99,9 @@ void onTestIgnore( @Nullable SkipReason skipReason(TestIdentifier test); - boolean isNew(TestIdentifier test); + boolean isNew(@Nonnull TestIdentifier test); - boolean isFlaky(TestIdentifier test); + boolean isFlaky(@Nonnull TestIdentifier test); @Override void close();