diff --git a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy index e71a8a5d8c9..2618d70d520 100644 --- a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy +++ b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy @@ -31,7 +31,6 @@ import datadog.trace.civisibility.decorator.TestDecoratorImpl import datadog.trace.civisibility.diff.Diff import datadog.trace.civisibility.diff.LineDiff import datadog.trace.civisibility.domain.BuildSystemSession -import datadog.trace.civisibility.domain.TestFrameworkModule import datadog.trace.civisibility.domain.TestFrameworkSession import datadog.trace.civisibility.domain.buildsystem.BuildSystemSessionImpl import datadog.trace.civisibility.domain.buildsystem.ModuleSignalRouter @@ -266,9 +265,10 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat @Override TestEventsHandler create(String component, ContextStore suiteStore, ContextStore testStore, Collection capabilities) { - TestFrameworkSession testSession = testFrameworkSessionFactory.startSession(moduleName, component, null, capabilities) - TestFrameworkModule testModule = testSession.testModuleStart(moduleName, null) - new TestEventsHandlerImpl(metricCollector, testSession, testModule, + new TestEventsHandlerImpl(metricCollector, + { testFrameworkSessionFactory.startSession(moduleName, component, null, capabilities) }, + moduleName, + false, suiteStore != null ? suiteStore : new ConcurrentHashMapContextStore<>(), testStore != null ? testStore : new ConcurrentHashMapContextStore<>()) } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java index 246ebe2b336..d094cdf838a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java @@ -26,7 +26,6 @@ import datadog.trace.civisibility.decorator.TestDecorator; import datadog.trace.civisibility.decorator.TestDecoratorImpl; import datadog.trace.civisibility.domain.BuildSystemSession; -import datadog.trace.civisibility.domain.TestFrameworkModule; import datadog.trace.civisibility.domain.TestFrameworkSession; import datadog.trace.civisibility.domain.buildsystem.BuildSystemSessionImpl; import datadog.trace.civisibility.domain.buildsystem.ProxyTestSession; @@ -190,14 +189,15 @@ public TestEventsHandler create( @Nullable ContextStore suiteStore, @Nullable ContextStore testStore, Collection capabilities) { - TestFrameworkSession testSession = - sessionFactory.startSession(repoServices.moduleName, component, null, capabilities); - TestFrameworkModule testModule = testSession.testModuleStart(repoServices.moduleName, null); - TestEventsHandlerImpl handler = + boolean eagerSessionStart = !services.processHierarchy.isHeadless(); + TestEventsHandler handler = new TestEventsHandlerImpl<>( services.metricCollector, - testSession, - testModule, + () -> + sessionFactory.startSession( + repoServices.moduleName, component, null, capabilities), + repoServices.moduleName, + eagerSessionStart, suiteStore != null ? suiteStore : new ConcurrentHashMapContextStore<>(), testStore != null ? testStore : new ConcurrentHashMapContextStore<>()); handlers.add(handler); 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 26b083f0c17..0e237f02abb 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 @@ -25,6 +25,7 @@ import datadog.trace.civisibility.domain.TestImpl; import datadog.trace.civisibility.domain.TestSuiteImpl; import java.util.Collection; +import java.util.function.Supplier; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.objectweb.asm.Type; @@ -37,22 +38,42 @@ public class TestEventsHandlerImpl private static final Logger log = LoggerFactory.getLogger(TestEventsHandlerImpl.class); private final CiVisibilityMetricCollector metricCollector; - private final TestFrameworkSession testSession; - private final TestFrameworkModule testModule; + private final Supplier testSessionSupplier; + private final String moduleName; private final ContextStore inProgressTestSuites; private final ContextStore inProgressTests; + private TestFrameworkSession testSession; + private volatile TestFrameworkModule testModule; public TestEventsHandlerImpl( CiVisibilityMetricCollector metricCollector, - TestFrameworkSession testSession, - TestFrameworkModule testModule, + Supplier testSessionSupplier, + String moduleName, + boolean eagerSessionStart, ContextStore suiteStore, ContextStore testStore) { this.metricCollector = metricCollector; - this.testSession = testSession; - this.testModule = testModule; + this.testSessionSupplier = testSessionSupplier; + this.moduleName = moduleName; this.inProgressTestSuites = (ContextStore) suiteStore; this.inProgressTests = (ContextStore) testStore; + if (eagerSessionStart) { + getOrCreateTestModule(); + } + } + + private TestFrameworkModule getOrCreateTestModule() { + TestFrameworkModule current = testModule; + if (current == null) { + synchronized (this) { + current = testModule; + if (current == null) { + testSession = testSessionSupplier.get(); + testModule = current = testSession.testModuleStart(moduleName, null); + } + } + } + return current; } private static boolean skipTrace(final Class testClass) { @@ -70,6 +91,7 @@ public void onTestSuiteStart( boolean parallelized, TestFrameworkInstrumentation instrumentation, @Nullable Long startTime) { + TestFrameworkModule testModule = getOrCreateTestModule(); if (skipTrace(testClass)) { return; } @@ -104,6 +126,7 @@ private String getTestTraits(Collection categories) { @Override public void onTestSuiteFinish(SuiteKey descriptor, @Nullable Long endTime) { + getOrCreateTestModule(); if (skipTrace(descriptor.getClass())) { return; } @@ -114,6 +137,7 @@ public void onTestSuiteFinish(SuiteKey descriptor, @Nullable Long endTime) { @Override public void onTestSuiteSkip(SuiteKey descriptor, @Nullable String reason) { + getOrCreateTestModule(); TestSuiteImpl testSuite = inProgressTestSuites.get(descriptor); if (testSuite == null) { log.debug("Ignoring skip event, could not find test suite {}", descriptor); @@ -124,6 +148,7 @@ public void onTestSuiteSkip(SuiteKey descriptor, @Nullable String reason) { @Override public void onTestSuiteFailure(SuiteKey descriptor, @Nullable Throwable throwable) { + getOrCreateTestModule(); TestSuiteImpl testSuite = inProgressTestSuites.get(descriptor); if (testSuite == null) { log.debug("Ignoring fail event, could not find test suite {}", descriptor); @@ -144,6 +169,7 @@ public void onTestStart( final @Nonnull TestSourceData testSourceData, final @Nullable Long startTime, final @Nullable TestExecutionTracker testExecutionTracker) { + TestFrameworkModule testModule = getOrCreateTestModule(); if (skipTrace(testSourceData.getTestClass())) { return; } @@ -222,6 +248,7 @@ public void onTestStart( @Override public void onTestSkip(TestKey descriptor, @Nullable String reason) { + getOrCreateTestModule(); TestImpl test = inProgressTests.get(descriptor); if (test == null) { log.debug("Ignoring skip event, could not find test {}}", descriptor); @@ -232,6 +259,7 @@ public void onTestSkip(TestKey descriptor, @Nullable String reason) { @Override public void onTestFailure(TestKey descriptor, @Nullable Throwable throwable) { + getOrCreateTestModule(); TestImpl test = inProgressTests.get(descriptor); if (test == null) { log.debug("Ignoring fail event, could not find test {}", descriptor); @@ -245,6 +273,7 @@ public void onTestFinish( TestKey descriptor, @Nullable Long endTime, @Nullable TestExecutionTracker testExecutionTracker) { + TestFrameworkModule testModule = getOrCreateTestModule(); TestImpl test = inProgressTests.remove(descriptor); if (test == null) { log.debug("Ignoring finish event, could not find test {}", descriptor); @@ -318,23 +347,26 @@ public void onTestIgnore( @Nonnull public TestExecutionPolicy executionPolicy( TestIdentifier test, TestSourceData testSource, Collection testTags) { - return testModule.executionPolicy(test, testSource, testTags); + return getOrCreateTestModule().executionPolicy(test, testSource, testTags); } @Override public int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource) { - return testModule.executionPriority(test, testSource); + return getOrCreateTestModule().executionPriority(test, testSource); } @Nullable @Override public SkipReason skipReason(TestIdentifier test) { - return testModule.skipReason(test); + return getOrCreateTestModule().skipReason(test); } @Override public void close() { - testModule.end(null); - testSession.end(null); + TestFrameworkModule current = testModule; + if (current != null) { + current.end(null); + testSession.end(null); + } } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/events/TestEventsHandlerImplTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/events/TestEventsHandlerImplTest.java new file mode 100644 index 00000000000..dc62a3261c7 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/events/TestEventsHandlerImplTest.java @@ -0,0 +1,102 @@ +package datadog.trace.civisibility.events; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.trace.api.civisibility.DDTest; +import datadog.trace.api.civisibility.DDTestSuite; +import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.NoOpMetricCollector; +import datadog.trace.api.civisibility.telemetry.tag.SkipReason; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.civisibility.domain.TestFrameworkModule; +import datadog.trace.civisibility.domain.TestFrameworkSession; +import datadog.trace.civisibility.utils.ConcurrentHashMapContextStore; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +import org.junit.jupiter.api.Test; + +class TestEventsHandlerImplTest { + + @Test + void doesNotCreateSessionWhenUnused() { + AtomicInteger creations = new AtomicInteger(); + TestEventsHandlerImpl handler = + handler( + () -> { + creations.incrementAndGet(); + return mock(TestFrameworkSession.class); + }); + + handler.close(); + + assertEquals(0, creations.get()); + } + + @Test + void createsSessionAndModuleOnceAndClosesThem() { + TestFrameworkSession session = mock(TestFrameworkSession.class); + TestFrameworkModule module = mock(TestFrameworkModule.class); + when(session.testModuleStart("module", null)).thenReturn(module); + TestIdentifier test = new TestIdentifier("suite", "test", null); + when(module.skipReason(test)).thenReturn(SkipReason.ITR); + AtomicInteger creations = new AtomicInteger(); + TestEventsHandlerImpl handler = + handler( + () -> { + creations.incrementAndGet(); + return session; + }); + + assertSame(SkipReason.ITR, handler.skipReason(test)); + assertSame(SkipReason.ITR, handler.skipReason(test)); + handler.close(); + + assertEquals(1, creations.get()); + verify(session).testModuleStart("module", null); + verify(module).end(null); + verify(session).end(null); + } + + @Test + void createsSessionImmediatelyWhenRequested() { + TestFrameworkSession session = mock(TestFrameworkSession.class); + TestFrameworkModule module = mock(TestFrameworkModule.class); + when(session.testModuleStart("module", null)).thenReturn(module); + AtomicInteger creations = new AtomicInteger(); + + TestEventsHandlerImpl handler = + handler( + () -> { + creations.incrementAndGet(); + return session; + }, + true); + + assertEquals(1, creations.get()); + handler.close(); + verify(module).end(null); + verify(session).end(null); + } + + private static TestEventsHandlerImpl handler( + Supplier testSessionSupplier) { + return handler(testSessionSupplier, false); + } + + private static TestEventsHandlerImpl handler( + Supplier testSessionSupplier, boolean eagerSessionStart) { + ContextStore suiteStore = new ConcurrentHashMapContextStore<>(); + ContextStore testStore = new ConcurrentHashMapContextStore<>(); + return new TestEventsHandlerImpl<>( + NoOpMetricCollector.INSTANCE, + testSessionSupplier, + "module", + eagerSessionStart, + suiteStore, + testStore); + } +} diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/test/groovy/JUnit5Test.groovy b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/test/groovy/JUnit5Test.groovy index eab2dc21249..eaa5f9767d9 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/test/groovy/JUnit5Test.groovy +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/test/groovy/JUnit5Test.groovy @@ -61,6 +61,14 @@ This can manifest when creating mocks. @DisableTestTrace(reason = "avoid self-tracing") class JUnit5Test extends CiVisibilityInstrumentationTest { + def "does not report a session when no tests are discovered"() { + when: + runTests([]) + + then: + TEST_WRITER.size() == 0 + } + def "test #testcaseName"() { runTests(tests, success)