Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions .github/migrated-modules.txt
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated

_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.
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated

## Code Quality and Security

### analyze-changes [🔗](analyze-changes.yaml)
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/check-pull-request-labels.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated
]
// Look for invalid labels
const invalidLabels = context.payload.pull_request.labels
Expand Down
169 changes: 169 additions & 0 deletions .github/workflows/enforce-groovy-migration.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
name: Enforce Groovy Migration
on:
pull_request:
types: [opened, edited, ready_for_review, labeled, unlabeled, synchronize]
branches:
- master
- release/v*
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated

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
Comment thread
sarahchen6 marked this conversation as resolved.
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({
Comment thread
sarahchen6 marked this conversation as resolved.
owner: context.repo.owner,
repo: context.repo.repo,
path: '.github/migrated-modules.txt',
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated
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 = '<!-- dd-trace-java-groovy-regression -->'
const warningMarker = '<!-- dd-trace-java-groovy-warning -->'
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` +
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated
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` +
Comment thread
sarahchen6 marked this conversation as resolved.
Outdated
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.`)
}
Loading