Skip to content

Avoid empty headless test sessions #12146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
daniel-mohedano wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Avoid empty headless test sessions #12146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions ...ixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -266,9 +265,10 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat

@Override
<SuiteKey, TestKey> TestEventsHandler<SuiteKey, TestKey> create(String component, ContextStore<SuiteKey, DDTestSuite> suiteStore, ContextStore<TestKey, DDTest> testStore, Collection<LibraryCapability> 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<>())
}
Expand Down
14 changes: 7 additions & 7 deletions ...gent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -190,14 +189,15 @@ public <SuiteKey, TestKey> TestEventsHandler<SuiteKey, TestKey> create(
@Nullable ContextStore<SuiteKey, DDTestSuite> suiteStore,
@Nullable ContextStore<TestKey, DDTest> testStore,
Collection<LibraryCapability> capabilities) {
TestFrameworkSession testSession =
sessionFactory.startSession(repoServices.moduleName, component, null, capabilities);
TestFrameworkModule testModule = testSession.testModuleStart(repoServices.moduleName, null);
TestEventsHandlerImpl<SuiteKey, TestKey> handler =
boolean eagerSessionStart = !services.processHierarchy.isHeadless();
TestEventsHandler<SuiteKey, TestKey> 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);
Expand Down
54 changes: 43 additions & 11 deletions ...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,22 +38,42 @@ public class TestEventsHandlerImpl<SuiteKey, TestKey>
private static final Logger log = LoggerFactory.getLogger(TestEventsHandlerImpl.class);

private final CiVisibilityMetricCollector metricCollector;
private final TestFrameworkSession testSession;
private final TestFrameworkModule testModule;
private final Supplier<TestFrameworkSession> testSessionSupplier;
private final String moduleName;
private final ContextStore<SuiteKey, TestSuiteImpl> inProgressTestSuites;
private final ContextStore<TestKey, TestImpl> inProgressTests;
private TestFrameworkSession testSession;
private volatile TestFrameworkModule testModule;

public TestEventsHandlerImpl(
CiVisibilityMetricCollector metricCollector,
TestFrameworkSession testSession,
TestFrameworkModule testModule,
Supplier<TestFrameworkSession> testSessionSupplier,
String moduleName,
boolean eagerSessionStart,
ContextStore<SuiteKey, DDTestSuite> suiteStore,
ContextStore<TestKey, DDTest> 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) {
Expand All @@ -70,6 +91,7 @@ public void onTestSuiteStart(
boolean parallelized,
TestFrameworkInstrumentation instrumentation,
@Nullable Long startTime) {
TestFrameworkModule testModule = getOrCreateTestModule();
if (skipTrace(testClass)) {
return;
}
Expand Down Expand Up @@ -104,6 +126,7 @@ private String getTestTraits(Collection<String> categories) {

@Override
public void onTestSuiteFinish(SuiteKey descriptor, @Nullable Long endTime) {
getOrCreateTestModule();
if (skipTrace(descriptor.getClass())) {
return;
}
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -318,23 +347,26 @@ public void onTestIgnore(
@Nonnull
public TestExecutionPolicy executionPolicy(
TestIdentifier test, TestSourceData testSource, Collection<String> 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);
}
}
}
102 changes: 102 additions & 0 deletions ...visibility/src/test/java/datadog/trace/civisibility/events/TestEventsHandlerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Object, Object> 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<Object, Object> 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<Object, Object> handler =
handler(
() -> {
creations.incrementAndGet();
return session;
},
true);

assertEquals(1, creations.get());
handler.close();
verify(module).end(null);
verify(session).end(null);
}

private static TestEventsHandlerImpl<Object, Object> handler(
Supplier<TestFrameworkSession> testSessionSupplier) {
return handler(testSessionSupplier, false);
}

private static TestEventsHandlerImpl<Object, Object> handler(
Supplier<TestFrameworkSession> testSessionSupplier, boolean eagerSessionStart) {
ContextStore<Object, DDTestSuite> suiteStore = new ConcurrentHashMapContextStore<>();
ContextStore<Object, DDTest> testStore = new ConcurrentHashMapContextStore<>();
return new TestEventsHandlerImpl<>(
NoOpMetricCollector.INSTANCE,
testSessionSupplier,
"module",
eagerSessionStart,
suiteStore,
testStore);
}
}
8 changes: 8 additions & 0 deletions dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/test/groovy/JUnit5Test.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading