From 8164b95343326e34fe4d60a602b628aa4b888fa7 Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Tue, 3 Mar 2026 15:18:52 -0500 Subject: [PATCH 1/3] Add workflow to prevent new Groovy test files --- .github/migrated-modules.txt | 9 + .github/workflows/README.md | 13 ++ .../workflows/check-pull-request-labels.yaml | 3 +- .../workflows/enforce-groovy-migration.yaml | 169 ++++++++++++++++++ 4 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 .github/migrated-modules.txt create mode 100644 .github/workflows/enforce-groovy-migration.yaml diff --git a/.github/migrated-modules.txt b/.github/migrated-modules.txt new file mode 100644 index 00000000000..b8f1d062ddb --- /dev/null +++ b/.github/migrated-modules.txt @@ -0,0 +1,9 @@ +# This file lists modules that have been migrated from Groovy to Java. +# New *.groovy files under any src/<*test*>/groovy/ path +# in these modules will fail the 'enforce-groovy-migration' PR check. +# +# After a module is migrated, add it on a new line here. +# Use the filesystem path prefix as seen below. + +buildSrc/call-site-instrumentation-plugin +components/json diff --git a/.github/workflows/README.md b/.github/workflows/README.md index e540781d8c3..b046ba176e1 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -123,6 +123,19 @@ _Action:_ _Recovery:_ Check at the milestone for the related issues and update them manually. +### enforce-groovy-migration [🔗](enforce-groovy-migration.yaml) + +_Trigger:_ When creating or updating a pull request, or when labels are added or removed. + +_Actions:_ + +* Fail the PR if a new Groovy test file is added to a module listed in [`.github/migrated-modules.txt`](../migrated-modules.txt) (hard enforcement), +* 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. + +_Recovery:_ Re-write the Groovy test files in Java. To override this check entirely, add the `override-groovy-enforcement` label to the PR. Remove the label to re-enable enforcement. + +_Notes:_ The migrated modules list is always read from `master`. Add a new entry to `.github/migrated-modules.txt` each time a module is migrated to Java. + ## Code Quality and Security ### analyze-changes [🔗](analyze-changes.yaml) diff --git a/.github/workflows/check-pull-request-labels.yaml b/.github/workflows/check-pull-request-labels.yaml index 53df94eaf51..5265d243516 100644 --- a/.github/workflows/check-pull-request-labels.yaml +++ b/.github/workflows/check-pull-request-labels.yaml @@ -33,7 +33,8 @@ jobs: 'mergequeue-status:', 'team:', 'performance:', // To refactor to 'ci: ' in the future - 'run-tests:' // Unused since GitLab migration + 'run-tests:', // Unused since GitLab migration + 'override-groovy-enforcement' ] // Look for invalid labels const invalidLabels = context.payload.pull_request.labels diff --git a/.github/workflows/enforce-groovy-migration.yaml b/.github/workflows/enforce-groovy-migration.yaml new file mode 100644 index 00000000000..900da7f1675 --- /dev/null +++ b/.github/workflows/enforce-groovy-migration.yaml @@ -0,0 +1,169 @@ +name: Enforce Groovy Migration +on: + pull_request: + types: [opened, edited, ready_for_review, labeled, unlabeled, synchronize] + branches: + - master + - release/v* + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + enforce_groovy_migration: + name: Enforce Groovy migration + permissions: + issues: write # Required to create a comment on the pull request + pull-requests: write # Required to create a comment on the pull request + runs-on: ubuntu-latest + steps: + - name: Check for Groovy regressions + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # 8.0.0 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + // Skip draft pull requests + if (context.payload.pull_request.draft) { + return + } + + // Check for override label — skip all checks if label present + const labels = context.payload.pull_request.labels.map(l => l.name) + if (labels.includes('override-groovy-enforcement')) { + console.log('override-groovy-enforcement label detected — skipping all checks.') + return + } + + // Read migrated modules list from master + const migratedMods = await github.rest.repos.getContent({ + owner: context.repo.owner, + repo: context.repo.repo, + path: '.github/migrated-modules.txt', + ref: 'master' + }) + const migratedPrefixes = Buffer.from(migratedMods.data.content, 'base64') + .toString() + .split('\n') + .map(l => l.trim()) + .filter(l => l && !l.startsWith('#')) + + // Get all files changed in this PR + const allFiles = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + }) + + // Filter these changed files to newly added Groovy files in any test source set + const addedGroovy = allFiles.filter(f => + f.status === 'added' && + /\/src\/[^/]*[tT]est[^/]*\/groovy\/.*\.groovy$/.test(f.filename) + ) + + // Extract module prefix from file path (everything before /src/(test|testFixtures)/groovy/) + const moduleOf = path => { + const m = path.match(/^(.*?)\/src\/(test|testFixtures)\/groovy\//) + return m ? m[1] : null + } + + // Classify each added Groovy file + const regressions = [] + const warnings = [] + for (const file of addedGroovy) { + const path = file.filename + const mod = moduleOf(path) + if (migratedPrefixes.some(prefix => path.startsWith(prefix + '/'))) { + regressions.push({ path, mod }) + } else if ( + path.startsWith('dd-java-agent/instrumentation/') || + path.startsWith('dd-smoke-tests/') + ) { + // ignore Groovy file additions to instrumentations and smoke-tests for now + } else { + warnings.push({ path, mod }) + } + } + + // Fetch existing comments once + const comments = await github.rest.issues.listComments({ + issue_number: context.payload.pull_request.number, + owner: context.repo.owner, + repo: context.repo.repo + }) + + const regressionMarker = '' + const warningMarker = '' + const existingRegressionComment = comments.data.find(c => c.body.includes(regressionMarker)) + const existingWarningComment = comments.data.find(c => c.body.includes(warningMarker)) + + // Handle regression comment + if (regressions.length > 0) { + const fileList = regressions + .map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`) + .join('\n') + const body = `**❌ Groovy Test Regression Detected**\n\n` + + `The following files add Groovy tests to modules that have been fully migrated to Java:\n\n` + + `${fileList}\n\n` + + `These modules no longer accept Groovy test files. Please rewrite the test in Java instead.\n\n` + + regressionMarker + if (existingRegressionComment) { + await github.rest.issues.updateComment({ + comment_id: existingRegressionComment.id, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } else { + await github.rest.issues.createComment({ + issue_number: context.payload.pull_request.number, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } + } else if (existingRegressionComment) { + await github.rest.issues.deleteComment({ + comment_id: existingRegressionComment.id, + owner: context.repo.owner, + repo: context.repo.repo + }) + } + + // Handle warning comment + if (warnings.length > 0) { + const fileList = warnings + .map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`) + .join('\n') + const body = `**⚠️ New Groovy Test Files Added**\n\n` + + `The following files add Groovy tests to modules that are candidates for migration to Java:\n\n` + + `${fileList}\n\n` + + `Consider writing these tests in Java instead to help with the ongoing migration effort.\n\n` + + warningMarker + if (existingWarningComment) { + await github.rest.issues.updateComment({ + comment_id: existingWarningComment.id, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } else { + await github.rest.issues.createComment({ + issue_number: context.payload.pull_request.number, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } + } else if (existingWarningComment) { + await github.rest.issues.deleteComment({ + comment_id: existingWarningComment.id, + owner: context.repo.owner, + repo: context.repo.repo + }) + } + + // Fail the check if there are regressions + if (regressions.length > 0) { + core.setFailed(`${regressions.length} Groovy regression(s) detected in migrated module(s). See PR comment for details. To skip this check entirely, add the 'override-groovy-enforcement' label.`) + } From 909a40d3e8d5c5e9bb936abdf2d72eaa9f913dbc Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Wed, 4 Mar 2026 11:32:54 -0500 Subject: [PATCH 2/3] Address review comments --- .claude/skills/migrate-groovy-to-java/SKILL.md | 1 + ...ed-modules.txt => g2j-migrated-modules.txt} | 0 .github/workflows/README.md | 8 ++++---- .../workflows/check-pull-request-labels.yaml | 3 +-- .../workflows/enforce-groovy-migration.yaml | 18 +++++++++--------- 5 files changed, 15 insertions(+), 15 deletions(-) rename .github/{migrated-modules.txt => g2j-migrated-modules.txt} (100%) diff --git a/.claude/skills/migrate-groovy-to-java/SKILL.md b/.claude/skills/migrate-groovy-to-java/SKILL.md index dd6556e53f9..586cd8fda72 100644 --- a/.claude/skills/migrate-groovy-to-java/SKILL.md +++ b/.claude/skills/migrate-groovy-to-java/SKILL.md @@ -9,6 +9,7 @@ Migrate test Groovy files to Java using JUnit 5 2. convert groovy files to Java using Junit 5 3. make sure the tests are still passing after migration 4. remove groovy files +5. Add the migrated module path(s) to `.github/g2j-migrated-modules.txt` When converting groovy code to java code make sure that: - the Java code generated is compatible with JDK 8 diff --git a/.github/migrated-modules.txt b/.github/g2j-migrated-modules.txt similarity index 100% rename from .github/migrated-modules.txt rename to .github/g2j-migrated-modules.txt diff --git a/.github/workflows/README.md b/.github/workflows/README.md index b046ba176e1..c33a4521c4d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -125,16 +125,16 @@ _Recovery:_ Check at the milestone for the related issues and update them manual ### enforce-groovy-migration [🔗](enforce-groovy-migration.yaml) -_Trigger:_ When creating or updating a pull request, or when labels are added or removed. +_Trigger:_ When creating or updating a pull request targeting `master`, or when labels are updated. _Actions:_ -* Fail the PR if a new Groovy test file is added to a module listed in [`.github/migrated-modules.txt`](../migrated-modules.txt) (hard enforcement), +* 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), * 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. -_Recovery:_ Re-write the Groovy test files in Java. To override this check entirely, add the `override-groovy-enforcement` label to the PR. Remove the label to re-enable enforcement. +_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. -_Notes:_ The migrated modules list is always read from `master`. Add a new entry to `.github/migrated-modules.txt` each time a module is migrated to Java. +_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. ## Code Quality and Security diff --git a/.github/workflows/check-pull-request-labels.yaml b/.github/workflows/check-pull-request-labels.yaml index 5265d243516..53df94eaf51 100644 --- a/.github/workflows/check-pull-request-labels.yaml +++ b/.github/workflows/check-pull-request-labels.yaml @@ -33,8 +33,7 @@ jobs: 'mergequeue-status:', 'team:', 'performance:', // To refactor to 'ci: ' in the future - 'run-tests:', // Unused since GitLab migration - 'override-groovy-enforcement' + 'run-tests:' // Unused since GitLab migration ] // Look for invalid labels const invalidLabels = context.payload.pull_request.labels diff --git a/.github/workflows/enforce-groovy-migration.yaml b/.github/workflows/enforce-groovy-migration.yaml index 900da7f1675..1201152119e 100644 --- a/.github/workflows/enforce-groovy-migration.yaml +++ b/.github/workflows/enforce-groovy-migration.yaml @@ -4,7 +4,6 @@ on: types: [opened, edited, ready_for_review, labeled, unlabeled, synchronize] branches: - master - - release/v* concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -16,6 +15,7 @@ jobs: permissions: issues: write # Required to create a comment on the pull request pull-requests: write # Required to create a comment on the pull request + contents: read # Required to read migrated modules file runs-on: ubuntu-latest steps: - name: Check for Groovy regressions @@ -30,8 +30,8 @@ jobs: // Check for override label — skip all checks if label present const labels = context.payload.pull_request.labels.map(l => l.name) - if (labels.includes('override-groovy-enforcement')) { - console.log('override-groovy-enforcement label detected — skipping all checks.') + if (labels.includes('tag: override-groovy-enforcement')) { + console.log('tag: override-groovy-enforcement label detected — skipping all checks.') return } @@ -39,7 +39,7 @@ jobs: const migratedMods = await github.rest.repos.getContent({ owner: context.repo.owner, repo: context.repo.repo, - path: '.github/migrated-modules.txt', + path: '.github/g2j-migrated-modules.txt', ref: 'master' }) const migratedPrefixes = Buffer.from(migratedMods.data.content, 'base64') @@ -103,9 +103,9 @@ jobs: .map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`) .join('\n') const body = `**❌ Groovy Test Regression Detected**\n\n` + - `The following files add Groovy tests to modules that have been fully migrated to Java:\n\n` + + `The following files add Groovy tests to modules that have been fully migrated to Java / JUnit 5:\n\n` + `${fileList}\n\n` + - `These modules no longer accept Groovy test files. Please rewrite the test in Java instead.\n\n` + + `These modules no longer accept Groovy test files. Please rewrite the test in Java / JUnit 5 instead.\n\n` + regressionMarker if (existingRegressionComment) { await github.rest.issues.updateComment({ @@ -136,9 +136,9 @@ jobs: .map(({ path, mod }) => `- \`${path}\` (module: \`${mod}\`)`) .join('\n') const body = `**⚠️ New Groovy Test Files Added**\n\n` + - `The following files add Groovy tests to modules that are candidates for migration to Java:\n\n` + + `The following files add Groovy tests to modules that are candidates for migration to Java / JUnit 5:\n\n` + `${fileList}\n\n` + - `Consider writing these tests in Java instead to help with the ongoing migration effort.\n\n` + + `Consider writing these tests in Java / JUnit 5 instead to help with the ongoing migration effort.\n\n` + warningMarker if (existingWarningComment) { await github.rest.issues.updateComment({ @@ -165,5 +165,5 @@ jobs: // Fail the check if there are regressions if (regressions.length > 0) { - core.setFailed(`${regressions.length} Groovy regression(s) detected in migrated module(s). See PR comment for details. To skip this check entirely, add the 'override-groovy-enforcement' label.`) + 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.`) } From 84233f3e1a73cf4b2165a2011c75256432e964a3 Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Wed, 4 Mar 2026 11:38:25 -0500 Subject: [PATCH 3/3] Update comment --- .github/g2j-migrated-modules.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/g2j-migrated-modules.txt b/.github/g2j-migrated-modules.txt index b8f1d062ddb..229a57d87f0 100644 --- a/.github/g2j-migrated-modules.txt +++ b/.github/g2j-migrated-modules.txt @@ -1,4 +1,4 @@ -# This file lists modules that have been migrated from Groovy to Java. +# This file lists modules that have been migrated from Groovy to Java / JUnit 5. # New *.groovy files under any src/<*test*>/groovy/ path # in these modules will fail the 'enforce-groovy-migration' PR check. #