Skip to content

Commit 95b7e3f

Browse files
authored
Merge branch 'master' into mhlidd/run_spock_with_strict
2 parents b0147c9 + b664cc0 commit 95b7e3f

File tree

77 files changed

+1390
-754
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+1390
-754
lines changed

.github/workflows/run-system-tests.yaml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ jobs:
6363
- build
6464
# If you change the following comment, update the pattern in the update_system_test_reference.sh script to match.
6565
uses: DataDog/system-tests/.github/workflows/system-tests.yml@main # system tests are pinned on release branches only
66-
secrets:
67-
TEST_OPTIMIZATION_API_KEY: ${{ secrets.DATADOG_API_KEY_PROD }}
6866
permissions:
6967
contents: read
7068
id-token: write
@@ -78,7 +76,7 @@ jobs:
7876
scenarios_groups: tracer-release
7977
excluded_scenarios: APM_TRACING_E2E_OTEL,APM_TRACING_E2E_SINGLE_SPAN,PROFILING # exclude flaky scenarios
8078
skip_empty_scenarios: true
81-
push_to_test_optimization: true
79+
push_to_test_optimization: false # disabled to avoid pushing to Test Optimization while API key is transitioning to system-tests
8280

8381
# Ensure the main job is run to completion
8482
check:

dd-java-agent/agent-ci-visibility/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ dependencies {
3737
testImplementation group: 'org.skyscreamer', name: 'jsonassert', version: '1.5.1'
3838
testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.31'
3939
testImplementation group: 'org.msgpack', name: 'jackson-dataformat-msgpack', version: '0.9.6'
40+
testImplementation libs.bundles.mockito
4041
}
4142

4243
tasks.named("shadowJar", ShadowJar) {

dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,9 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat
128128
def metricCollector = Stub(CiVisibilityMetricCollectorImpl)
129129

130130
def sourcePathResolver = Stub(SourcePathResolver)
131-
sourcePathResolver.getSourcePath(_ as Class) >> DUMMY_SOURCE_PATH
132-
sourcePathResolver.getResourcePath(_ as String) >> {
133-
String path -> path
131+
sourcePathResolver.getSourcePaths(_ as Class) >> [DUMMY_SOURCE_PATH]
132+
sourcePathResolver.getResourcePaths(_ as String) >> {
133+
String path -> [path]
134134
}
135135

136136
def codeowners = Stub(Codeowners)

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import datadog.trace.api.civisibility.coverage.CoverageProbes;
55
import datadog.trace.api.civisibility.coverage.CoverageStore;
66
import datadog.trace.api.civisibility.coverage.TestReport;
7-
import datadog.trace.civisibility.source.SourceResolutionException;
87
import java.util.Collection;
98
import java.util.Map;
109
import java.util.concurrent.ConcurrentHashMap;
@@ -37,18 +36,13 @@ private T create(Thread thread) {
3736

3837
@Override
3938
public boolean report(DDTraceId testSessionId, Long testSuiteId, long testSpanId) {
40-
try {
41-
report = report(testSessionId, testSuiteId, testSpanId, probes.values());
42-
return report != null && report.isNotEmpty();
43-
} catch (SourceResolutionException e) {
44-
return false;
45-
}
39+
report = report(testSessionId, testSuiteId, testSpanId, probes.values());
40+
return report != null && report.isNotEmpty();
4641
}
4742

4843
@Nullable
4944
protected abstract TestReport report(
50-
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<T> probes)
51-
throws SourceResolutionException;
45+
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<T> probes);
5246

5347
@Nullable
5448
@Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java

Lines changed: 40 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import datadog.trace.api.civisibility.telemetry.tag.CoverageErrorType;
1212
import datadog.trace.civisibility.coverage.ConcurrentCoverageStore;
1313
import datadog.trace.civisibility.source.SourcePathResolver;
14-
import datadog.trace.civisibility.source.SourceResolutionException;
1514
import java.util.ArrayList;
1615
import java.util.Collection;
1716
import java.util.Collections;
@@ -47,61 +46,54 @@ private FileCoverageStore(
4746
@Nullable
4847
@Override
4948
protected TestReport report(
50-
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<FileProbes> probes)
51-
throws SourceResolutionException {
52-
try {
53-
Set<Class<?>> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>());
54-
Collection<String> combinedNonCodeResources = new HashSet<>();
55-
56-
for (FileProbes probe : probes) {
57-
combinedClasses.addAll(probe.getCoveredClasses());
58-
combinedNonCodeResources.addAll(probe.getNonCodeResources());
59-
}
49+
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<FileProbes> probes) {
50+
Set<Class<?>> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>());
51+
Collection<String> combinedNonCodeResources = new HashSet<>();
6052

61-
if (combinedClasses.isEmpty() && combinedNonCodeResources.isEmpty()) {
62-
return null;
63-
}
53+
for (FileProbes probe : probes) {
54+
combinedClasses.addAll(probe.getCoveredClasses());
55+
combinedNonCodeResources.addAll(probe.getNonCodeResources());
56+
}
6457

65-
Set<String> coveredPaths = set(combinedClasses.size() + combinedNonCodeResources.size());
66-
for (Class<?> clazz : combinedClasses) {
67-
String sourcePath = sourcePathResolver.getSourcePath(clazz);
68-
if (sourcePath == null) {
69-
log.debug(
70-
"Skipping coverage reporting for {} because source path could not be determined",
71-
clazz);
72-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
73-
continue;
74-
}
75-
coveredPaths.add(sourcePath);
76-
}
58+
if (combinedClasses.isEmpty() && combinedNonCodeResources.isEmpty()) {
59+
return null;
60+
}
7761

78-
for (String nonCodeResource : combinedNonCodeResources) {
79-
String resourcePath = sourcePathResolver.getResourcePath(nonCodeResource);
80-
if (resourcePath == null) {
81-
log.debug(
82-
"Skipping coverage reporting for {} because resource path could not be determined",
83-
nonCodeResource);
84-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
85-
continue;
86-
}
87-
coveredPaths.add(resourcePath);
62+
Set<String> coveredPaths = set(combinedClasses.size() + combinedNonCodeResources.size());
63+
for (Class<?> clazz : combinedClasses) {
64+
Collection<String> sourcePaths = sourcePathResolver.getSourcePaths(clazz);
65+
if (sourcePaths.isEmpty()) {
66+
log.debug(
67+
"Skipping coverage reporting for {} because source path could not be determined",
68+
clazz);
69+
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
70+
continue;
8871
}
72+
coveredPaths.addAll(sourcePaths);
73+
}
8974

90-
List<TestReportFileEntry> fileEntries = new ArrayList<>(coveredPaths.size());
91-
for (String path : coveredPaths) {
92-
fileEntries.add(new TestReportFileEntry(path, null));
75+
for (String nonCodeResource : combinedNonCodeResources) {
76+
Collection<String> resourcePaths = sourcePathResolver.getResourcePaths(nonCodeResource);
77+
if (resourcePaths.isEmpty()) {
78+
log.debug(
79+
"Skipping coverage reporting for {} because resource path could not be determined",
80+
nonCodeResource);
81+
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
82+
continue;
9383
}
84+
coveredPaths.addAll(resourcePaths);
85+
}
9486

95-
TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries);
96-
metrics.add(
97-
CiVisibilityDistributionMetric.CODE_COVERAGE_FILES,
98-
report.getTestReportFileEntries().size());
99-
return report;
100-
101-
} catch (Exception e) {
102-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1);
103-
throw e;
87+
List<TestReportFileEntry> fileEntries = new ArrayList<>(coveredPaths.size());
88+
for (String path : coveredPaths) {
89+
fileEntries.add(new TestReportFileEntry(path, null));
10490
}
91+
92+
TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries);
93+
metrics.add(
94+
CiVisibilityDistributionMetric.CODE_COVERAGE_FILES,
95+
report.getTestReportFileEntries().size());
96+
return report;
10597
}
10698

10799
private static <T> Set<T> set(int size) {

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java

Lines changed: 69 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import datadog.trace.api.civisibility.telemetry.tag.CoverageErrorType;
1212
import datadog.trace.civisibility.coverage.ConcurrentCoverageStore;
1313
import datadog.trace.civisibility.source.SourcePathResolver;
14-
import datadog.trace.civisibility.source.SourceResolutionException;
1514
import datadog.trace.civisibility.source.Utils;
1615
import java.io.InputStream;
1716
import java.util.ArrayList;
@@ -53,88 +52,84 @@ private LineCoverageStore(
5352
@Nullable
5453
@Override
5554
protected TestReport report(
56-
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<LineProbes> probes)
57-
throws SourceResolutionException {
58-
try {
59-
Map<Class<?>, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>();
60-
Collection<String> combinedNonCodeResources = new HashSet<>();
61-
62-
for (LineProbes probe : probes) {
63-
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) {
64-
combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge);
65-
}
66-
combinedNonCodeResources.addAll(probe.getNonCodeResources());
67-
}
55+
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<LineProbes> probes) {
56+
Map<Class<?>, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>();
57+
Collection<String> combinedNonCodeResources = new HashSet<>();
6858

69-
if (combinedExecutionData.isEmpty() && combinedNonCodeResources.isEmpty()) {
70-
return null;
59+
for (LineProbes probe : probes) {
60+
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) {
61+
combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge);
7162
}
63+
combinedNonCodeResources.addAll(probe.getNonCodeResources());
64+
}
7265

73-
Map<String, BitSet> coveredLinesBySourcePath = new HashMap<>();
74-
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) {
75-
ExecutionDataAdapter executionDataAdapter = e.getValue();
76-
String className = executionDataAdapter.getClassName();
77-
78-
Class<?> clazz = e.getKey();
79-
String sourcePath = sourcePathResolver.getSourcePath(clazz);
80-
if (sourcePath == null) {
81-
log.debug(
82-
"Skipping coverage reporting for {} because source path could not be determined",
83-
className);
84-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
85-
continue;
86-
}
87-
88-
try (InputStream is = Utils.getClassStream(clazz)) {
89-
BitSet coveredLines =
90-
coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet());
91-
ExecutionDataStore store = new ExecutionDataStore();
92-
store.put(executionDataAdapter.toExecutionData());
93-
94-
// TODO optimize this part to avoid parsing
95-
// the same class multiple times for different test cases
96-
Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines));
97-
analyzer.analyzeClass(is, null);
98-
99-
} catch (Exception exception) {
100-
log.debug(
101-
"Skipping coverage reporting for {} ({}) because of error",
102-
className,
103-
sourcePath,
104-
exception);
105-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1);
106-
}
107-
}
66+
if (combinedExecutionData.isEmpty() && combinedNonCodeResources.isEmpty()) {
67+
return null;
68+
}
10869

109-
List<TestReportFileEntry> fileEntries = new ArrayList<>(coveredLinesBySourcePath.size());
110-
for (Map.Entry<String, BitSet> e : coveredLinesBySourcePath.entrySet()) {
111-
String sourcePath = e.getKey();
112-
BitSet coveredLines = e.getValue();
113-
fileEntries.add(new TestReportFileEntry(sourcePath, coveredLines));
70+
Map<String, BitSet> coveredLinesBySourcePath = new HashMap<>();
71+
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) {
72+
ExecutionDataAdapter executionDataAdapter = e.getValue();
73+
String className = executionDataAdapter.getClassName();
74+
75+
Class<?> clazz = e.getKey();
76+
Collection<String> sourcePaths = sourcePathResolver.getSourcePaths(clazz);
77+
if (sourcePaths.size() != 1) {
78+
log.debug(
79+
"Skipping coverage reporting for {} because source path could not be determined",
80+
className);
81+
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
82+
continue;
11483
}
115-
116-
for (String nonCodeResource : combinedNonCodeResources) {
117-
String resourcePath = sourcePathResolver.getResourcePath(nonCodeResource);
118-
if (resourcePath == null) {
119-
log.debug(
120-
"Skipping coverage reporting for {} because resource path could not be determined",
121-
nonCodeResource);
122-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
123-
continue;
124-
}
125-
fileEntries.add(new TestReportFileEntry(resourcePath, null));
84+
String sourcePath = sourcePaths.iterator().next();
85+
86+
try (InputStream is = Utils.getClassStream(clazz)) {
87+
BitSet coveredLines =
88+
coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet());
89+
ExecutionDataStore store = new ExecutionDataStore();
90+
store.put(executionDataAdapter.toExecutionData());
91+
92+
// TODO optimize this part to avoid parsing
93+
// the same class multiple times for different test cases
94+
Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines));
95+
analyzer.analyzeClass(is, null);
96+
97+
} catch (Exception exception) {
98+
log.debug(
99+
"Skipping coverage reporting for {} ({}) because of error",
100+
className,
101+
sourcePath,
102+
exception);
103+
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1);
126104
}
105+
}
127106

128-
TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries);
129-
metrics.add(
130-
CiVisibilityDistributionMetric.CODE_COVERAGE_FILES,
131-
report.getTestReportFileEntries().size());
132-
return report;
107+
List<TestReportFileEntry> fileEntries = new ArrayList<>(coveredLinesBySourcePath.size());
108+
for (Map.Entry<String, BitSet> e : coveredLinesBySourcePath.entrySet()) {
109+
String sourcePath = e.getKey();
110+
BitSet coveredLines = e.getValue();
111+
fileEntries.add(new TestReportFileEntry(sourcePath, coveredLines));
112+
}
133113

134-
} catch (Exception e) {
135-
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1);
136-
throw e;
114+
for (String nonCodeResource : combinedNonCodeResources) {
115+
Collection<String> resourcePaths = sourcePathResolver.getResourcePaths(nonCodeResource);
116+
if (resourcePaths.isEmpty()) {
117+
log.debug(
118+
"Skipping coverage reporting for {} because resource path could not be determined",
119+
nonCodeResource);
120+
metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH);
121+
continue;
122+
}
123+
for (String resourcePath : resourcePaths) {
124+
fileEntries.add(new TestReportFileEntry(resourcePath, null));
125+
}
137126
}
127+
128+
TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries);
129+
metrics.add(
130+
CiVisibilityDistributionMetric.CODE_COVERAGE_FILES,
131+
report.getTestReportFileEntries().size());
132+
return report;
138133
}
139134

140135
public static final class Factory implements CoverageStore.Factory {

0 commit comments

Comments
 (0)