Skip to content

Commit af4163d

Browse files
sarahchen6sarahchen6
authored
Merge branch 'master' into cbeauchesne/final_status
2 parents e7b4064 + 08d4d6a commit af4163d

File tree

37 files changed

+1517
-364
lines changed

37 files changed

+1517
-364
lines changed

‎.claude/skills/migrate-groovy-to-java/SKILL.md‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,19 @@ description: migrate test groovy files to java
55

66
Migrate test Groovy files to Java using JUnit 5
77

8-
1. List all groovy files of the current gradle module
9-
2. convert groovy files to Java using Junit 5
10-
3. make sure the tests are still passing after migration
11-
4. remove groovy files
8+
1. List all Groovy files of the current Gradle module
9+
2. Convert Groovy files to Java using JUnit 5
10+
3. Make sure the tests are still passing after migration and that the test count has not changed
11+
4. Remove Groovy files
12+
5. Add the migrated module path(s) to `.github/g2j-migrated-modules.txt`
1213

13-
When converting groovy code to java code make sure that:
14-
- the Java code generated is compatible with JDK 8
15-
- when translating Spock test, favor using `@CsvSource` with `|` delimiters
16-
- when using a `@MethodSource`, use the test method name, and suffix it with `_arguments`
17-
- when converting tuples, create light dedicated structure instead to keep the typing system
14+
When converting Groovy code to Java code, make sure that:
15+
- The Java code generated is compatible with JDK 8
16+
- When translating Spock tests, favor using `@CsvSource` with `|` delimiters
17+
- When using `@MethodSource`, name the arguments method by appending `Arguments` using camelCase to the test method name (e.g. `testMethodArguments`)
18+
- Ensure parameterized test names are human-readable (i.e. no hashcodes); instead add a description string as the first `Arguments.of(...)` value or index the test case
19+
- When converting tuples, create a light dedicated structure instead to keep the typing system
1820
- Instead of checking a state and throwing an exception, use JUnit asserts
19-
- Do not wrap checked exception and throwing a Runtime exception, prefer adding a throws clause at method declaration
21+
- Do not wrap checked exceptions and throw a Runtime exception; prefer adding a throws clause at method declaration
2022
- Do not mark local variables `final`
23+
- Ensure variables are human-readable; avoid single-letter names and pre-define variables that are referenced multiple times

‎.github/g2j-migrated-modules.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# This file lists modules that have been migrated from Groovy to Java / JUnit 5.
2+
# New *.groovy files under any src/<*test*>/groovy/ path
3+
# in these modules will fail the 'enforce-groovy-migration' PR check.
4+
#
5+
# After a module is migrated, add it on a new line here.
6+
# Use the filesystem path prefix as seen below.
7+
8+
buildSrc/call-site-instrumentation-plugin
9+
components/json

‎.github/workflows/README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,19 @@ _Action:_
123123

124124
_Recovery:_ Check at the milestone for the related issues and update them manually.
125125

126+
### enforce-groovy-migration [🔗](enforce-groovy-migration.yaml)
127+
128+
_Trigger:_ When creating or updating a pull request targeting `master`, or when labels are updated.
129+
130+
_Actions:_
131+
132+
* Fail the PR if a new Groovy test file is added to a module listed in [`.github/g2j-migrated-modules.txt`](../g2j-migrated-modules.txt) (hard enforcement),
133+
* Post a warning comment on the PR if a new Groovy test file is added to any other non-exempt module (soft warning). Instrumentation (`dd-java-agent/instrumentation/`) and smoke-test (`dd-smoke-tests/`) modules are exempt from this warning.
134+
135+
_Recovery:_ Re-write the Groovy test files in Java / JUnit 5. To override this check entirely, add the `tag: override-groovy-enforcement` label to the PR. Remove the label to re-enable enforcement.
136+
137+
_Notes:_ The migrated modules list is always read from `master`. Add a new entry to `.github/g2j-migrated-modules.txt` each time a module is migrated to Java / JUnit 5.
138+
126139
## Code Quality and Security
127140

128141
### analyze-changes [🔗](analyze-changes.yaml)

‎.github/workflows/enforce-groovy-migration.yaml‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
name: Enforce Groovy Migration
2+
on:
3+
pull_request:
4+
types: [opened, edited, ready_for_review, labeled, unlabeled, synchronize]
5+
branches:
6+
- master
7+
8+
concurrency:
9+
group: ${{ github.workflow }}-${{ github.ref }}
10+
cancel-in-progress: true
11+
12+
jobs:
13+
enforce_groovy_migration:
14+
name: Enforce Groovy migration
15+
permissions:
16+
issues: write # Required to create a comment on the pull request
17+
pull-requests: write # Required to create a comment on the pull request
18+
contents: read # Required to read migrated modules file
19+
runs-on: ubuntu-latest
20+
steps:
21+
- name: Check for Groovy regressions
22+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # 8.0.0
23+
with:
24+
github-token: ${{ secrets.GITHUB_TOKEN }}
25+
script: |
26+
// Skip draft pull requests
27+
if (context.payload.pull_request.draft) {
28+
return
29+
}
30+
31+
// Check for override label — skip all checks if label present
32+
const labels = context.payload.pull_request.labels.map(l => l.name)
33+
if (labels.includes('tag: override-groovy-enforcement')) {
34+
console.log('tag: override-groovy-enforcement label detected — skipping all checks.')
35+
return
36+
}
37+
38+
// Read migrated modules list from master
39+
const migratedMods = await github.rest.repos.getContent({
40+
owner: context.repo.owner,
41+
repo: context.repo.repo,
42+
path: '.github/g2j-migrated-modules.txt',
43+
ref: 'master'
44+
})
45+
const migratedPrefixes = Buffer.from(migratedMods.data.content, 'base64')
46+
.toString()
47+
.split('\n')
48+
.map(l => l.trim())
49+
.filter(l => l && !l.startsWith('#'))
50+
51+
// Get all files changed in this PR
52+
const allFiles = await github.paginate(github.rest.pulls.listFiles, {
53+
owner: context.repo.owner,
54+
repo: context.repo.repo,
55+
pull_number: context.payload.pull_request.number
56+
})
57+
58+
// Filter these changed files to newly added Groovy files in any test source set
59+
const addedGroovy = allFiles.filter(f =>
60+
f.status === 'added' &&
61+
/\/src\/[^/]*[tT]est[^/]*\/groovy\/.*\.groovy$/.test(f.filename)
62+
)
63+
64+
// Extract module prefix from file path (everything before /src/(test|testFixtures)/groovy/)
65+
const moduleOf = path => {
66+
const m = path.match(/^(.*?)\/src\/(test|testFixtures)\/groovy\//)
67+
return m ? m[1] : null
68+
}
69+
70+
// Classify each added Groovy file
71+
const regressions = []
72+
const warnings = []
73+
for (const file of addedGroovy) {
74+
const path = file.filename
75+
const mod = moduleOf(path)
76+
if (migratedPrefixes.some(prefix => path.startsWith(prefix + '/'))) {
77+
regressions.push({ path, mod })
78+
} else if (
79+
path.startsWith('dd-java-agent/instrumentation/') ||
80+
path.startsWith('dd-smoke-tests/')
81+
) {
82+
// ignore Groovy file additions to instrumentations and smoke-tests for now
83+
} else {
84+
warnings.push({ path, mod })
85+
}
86+
}
87+
88+
// Fetch existing comments once
89+
const comments = await github.rest.issues.listComments({
90+
issue_number: context.payload.pull_request.number,
91+
owner: context.repo.owner,
92+
repo: context.repo.repo
93+
})
94+
95+
const regressionMarker = '<!-- dd-trace-java-groovy-regression -->'
96+
const warningMarker = '<!-- dd-trace-java-groovy-warning -->'
97+
const existingRegressionComment = comments.data.find(c => c.body.includes(regressionMarker))
98+
const existingWarningComment = comments.data.find(c => c.body.includes(warningMarker))
99+
100+
// Handle regression comment
101+
if (regressions.length > 0) {
102+
const fileList = regressions
103+
.map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`)
104+
.join('\n')
105+
const body = `**❌ Groovy Test Regression Detected**\n\n` +
106+
`The following files add Groovy tests to modules that have been fully migrated to Java / JUnit 5:\n\n` +
107+
`${fileList}\n\n` +
108+
`These modules no longer accept Groovy test files. Please rewrite the test in Java / JUnit 5 instead.\n\n` +
109+
regressionMarker
110+
if (existingRegressionComment) {
111+
await github.rest.issues.updateComment({
112+
comment_id: existingRegressionComment.id,
113+
owner: context.repo.owner,
114+
repo: context.repo.repo,
115+
body
116+
})
117+
} else {
118+
await github.rest.issues.createComment({
119+
issue_number: context.payload.pull_request.number,
120+
owner: context.repo.owner,
121+
repo: context.repo.repo,
122+
body
123+
})
124+
}
125+
} else if (existingRegressionComment) {
126+
await github.rest.issues.deleteComment({
127+
comment_id: existingRegressionComment.id,
128+
owner: context.repo.owner,
129+
repo: context.repo.repo
130+
})
131+
}
132+
133+
// Handle warning comment
134+
if (warnings.length > 0) {
135+
const fileList = warnings
136+
.map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`)
137+
.join('\n')
138+
const body = `**⚠️ New Groovy Test Files Added**\n\n` +
139+
`The following files add Groovy tests to modules that are candidates for migration to Java / JUnit 5:\n\n` +
140+
`${fileList}\n\n` +
141+
`Consider writing these tests in Java / JUnit 5 instead to help with the ongoing migration effort.\n\n` +
142+
warningMarker
143+
if (existingWarningComment) {
144+
await github.rest.issues.updateComment({
145+
comment_id: existingWarningComment.id,
146+
owner: context.repo.owner,
147+
repo: context.repo.repo,
148+
body
149+
})
150+
} else {
151+
await github.rest.issues.createComment({
152+
issue_number: context.payload.pull_request.number,
153+
owner: context.repo.owner,
154+
repo: context.repo.repo,
155+
body
156+
})
157+
}
158+
} else if (existingWarningComment) {
159+
await github.rest.issues.deleteComment({
160+
comment_id: existingWarningComment.id,
161+
owner: context.repo.owner,
162+
repo: context.repo.repo
163+
})
164+
}
165+
166+
// Fail the check if there are regressions
167+
if (regressions.length > 0) {
168+
core.setFailed(`${regressions.length} Groovy regression(s) detected in migrated module(s). See PR comment for details. To skip this check entirely, add the 'tag: override-groovy-enforcement' label.`)
169+
}

‎.gitlab/ci_visibility_generate_job.sh‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,19 @@ if [ -z "$pr_number" ]; then
4242
exit 0
4343
fi
4444

45-
echo "PR #${pr_number} found, checking labels..."
45+
echo "PR #${pr_number} found, checking target branch..."
46+
set +e
47+
base_branch=$(gh pr view "$pr_number" --repo DataDog/dd-trace-java --json baseRefName --jq '.baseRefName' 2>&1)
48+
base_branch_status=$?
49+
set -e
50+
51+
if [ $base_branch_status -eq 0 ] && [[ "$base_branch" == release/* ]]; then
52+
echo "PR #$pr_number targets release branch '$base_branch' - skipping trigger"
53+
add_dummy_job
54+
exit 0
55+
fi
56+
57+
echo "Checking labels..."
4658
set +e
4759
labels=$(gh pr view "$pr_number" --repo DataDog/dd-trace-java --json labels --jq '.labels[].name' 2>&1)
4860
labels_status=$?

‎buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/TypeResolverPoolTest.java‎

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,86 +11,57 @@
1111

1212
class TypeResolverPoolTest {
1313

14+
TypeResolverPool resolver = new TypeResolverPool();
15+
1416
@Test
1517
void testResolvePrimitive() {
16-
TypeResolverPool resolver = new TypeResolverPool();
17-
18-
Class<?> result = resolver.resolveType(Type.INT_TYPE);
19-
20-
assertEquals(int.class, result);
18+
assertEquals(int.class, resolver.resolveType(Type.INT_TYPE));
2119
}
2220

2321
@Test
2422
void testResolvePrimitiveArray() {
25-
TypeResolverPool resolver = new TypeResolverPool();
2623
Type type = Type.getType("[I");
27-
28-
Class<?> result = resolver.resolveType(type);
29-
30-
assertEquals(int[].class, result);
24+
assertEquals(int[].class, resolver.resolveType(type));
3125
}
3226

3327
@Test
3428
void testResolvePrimitiveMultidimensionalArray() {
35-
TypeResolverPool resolver = new TypeResolverPool();
3629
Type type = Type.getType("[[[I");
37-
38-
Class<?> result = resolver.resolveType(type);
39-
40-
assertEquals(int[][][].class, result);
30+
assertEquals(int[][][].class, resolver.resolveType(type));
4131
}
4232

4333
@Test
4434
void testResolveClass() {
45-
TypeResolverPool resolver = new TypeResolverPool();
4635
Type type = Type.getType(String.class);
47-
48-
Class<?> result = resolver.resolveType(type);
49-
50-
assertEquals(String.class, result);
36+
assertEquals(String.class, resolver.resolveType(type));
5137
}
5238

5339
@Test
5440
void testResolveClassArray() {
55-
TypeResolverPool resolver = new TypeResolverPool();
5641
Type type = Type.getType(String[].class);
57-
58-
Class<?> result = resolver.resolveType(type);
59-
60-
assertEquals(String[].class, result);
42+
assertEquals(String[].class, resolver.resolveType(type));
6143
}
6244

6345
@Test
6446
void testResolveClassMultidimensionalArray() {
65-
TypeResolverPool resolver = new TypeResolverPool();
6647
Type type = Type.getType(String[][][].class);
67-
68-
Class<?> result = resolver.resolveType(type);
69-
70-
assertEquals(String[][][].class, result);
48+
assertEquals(String[][][].class, resolver.resolveType(type));
7149
}
7250

7351
@Test
7452
void testTypeResolverFromMethod() {
75-
TypeResolverPool resolver = new TypeResolverPool();
7653
Type type =
7754
Type.getMethodType(
7855
Type.getType(String[].class), Type.getType(String.class), Type.getType(String.class));
79-
80-
Class<?> result = resolver.resolveType(type.getReturnType());
81-
82-
assertEquals(String[].class, result);
56+
assertEquals(String[].class, resolver.resolveType(type.getReturnType()));
8357
}
8458

8559
@Test
8660
void testInheritedMethods() throws Exception {
87-
TypeResolverPool resolver = new TypeResolverPool();
8861
Type owner = Type.getType(HttpServletRequest.class);
8962
String name = "getParameter";
9063
Type descriptor = Type.getMethodType(Type.getType(String.class), Type.getType(String.class));
91-
9264
Method result = (Method) resolver.resolveMethod(new MethodType(owner, name, descriptor));
93-
9465
assertEquals(ServletRequest.class.getDeclaredMethod("getParameter", String.class), result);
9566
}
9667
}

‎buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/ext/IastExtensionTest.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import datadog.trace.plugin.csi.impl.assertion.CallSiteAssert;
2323
import datadog.trace.plugin.csi.impl.ext.tests.IastExtensionCallSite;
2424
import java.io.File;
25+
import java.io.IOException;
2526
import java.lang.reflect.Method;
2627
import java.nio.file.Files;
2728
import java.nio.file.Path;
@@ -46,12 +47,15 @@ class IastExtensionTest extends BaseCsiPluginTest {
4647

4748
@BeforeEach
4849
void setup() throws Exception {
49-
targetFolder = buildDir.toPath().resolve("target");
50-
Files.createDirectories(targetFolder);
51-
projectFolder = buildDir.toPath().resolve("project");
52-
Files.createDirectories(projectFolder);
53-
srcFolder = projectFolder.resolve("src/main/java");
54-
Files.createDirectories(srcFolder);
50+
targetFolder = createFolder("target");
51+
projectFolder = createFolder("project");
52+
srcFolder = createFolder("src/main/java");
53+
}
54+
55+
private Path createFolder(String folderName) throws IOException {
56+
Path folder = buildDir.toPath().resolve(folderName);
57+
Files.createDirectories(folder);
58+
return folder;
5559
}
5660

5761
@ParameterizedTest

‎buildSrc/src/main/groovy/datadog/gradle/plugin/instrument/BuildTimeInstrumentationExtension.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package datadog.gradle.plugin.instrument
22

3+
import org.gradle.api.file.ConfigurableFileCollection
34
import org.gradle.api.file.DirectoryProperty
45
import org.gradle.api.provider.ListProperty
56

67
abstract class BuildTimeInstrumentationExtension {
78
abstract ListProperty<String> getPlugins()
89
abstract ListProperty<DirectoryProperty> getAdditionalClasspath()
10+
abstract ConfigurableFileCollection getIncludeClassDirectories()
911
}

0 commit comments

Comments
 (0)