Convert test-published-dependencies to Gradle Kotlin DSL#11445
Conversation
… DSL Convert the test-published-dependencies build scripts from Groovy to Kotlin DSL: root settings/build, plus the four subprojects (all-deps-exist, ot-pulls-in-api, ot-is-shaded, agent-logs-on-java-7). In ot-is-shaded, the CheckJarContentsTask is rewritten with managed Property/ListProperty inputs and idiomatic Kotlin. Verified locally by reproducing the GitLab `test_published_artifacts` job: publishToMavenLocal then `./gradlew check` in the subproject. All modules pass; the Java 7 sub-test of agent-logs-on-java-7 is skipped locally only because no JDK 7 build exists for arm64 macOS. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
bric3
added
tag: no release notes
This comment has been minimized.
This comment has been minimized.
dd-octo-sts
Bot
added
the
tag: ai generated
bric3
requested review from
AlexeyKuznetsov-DD and
sarahchen6
and removed request for
dougqh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 076c5dc8a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| doLast { | ||
| // Arbitrary limit to prevent unintentional increases to the dd-trace-ot jar size | ||
| // Raise or lower as required | ||
| assert(jarFile.length() <= 8 * 1024 * 1024) |
There was a problem hiding this comment.
In this Kotlin DSL task, assert(...) calls Kotlin's assertion helper, which only throws when JVM assertions are enabled. Gradle does not enable -ea for task actions by default, so :ot-is-shaded:checkJarSize will pass even when dd-trace-ot grows past the 8 MiB limit. The Groovy assert this replaced was enforced for the build script; use check(...) or throw explicitly so this published-dependency guard still catches oversized artifacts.
Useful? React with 👍 / 👎.
AlexeyKuznetsov-DD
left a comment
AlexeyKuznetsov-DD
left a comment
There was a problem hiding this comment.
LGTM, added one optional nit comment.
| doLast { | ||
| // Arbitrary limit to prevent unintentional increases to the dd-trace-ot jar size | ||
| // Raise or lower as required | ||
| assert(jarFile.length() <= 8 * 1024 * 1024) |
There was a problem hiding this comment.
From Claude:
"kotlin.assert(...) only fires when the JVM is started with -ea (same semantics as Java's assert). Groovy assert was always enabled, so the original guard reliably failed the build when the jar exceeded 8MB; under the Gradle daemon's default JVM args this check is silently a no-op and the size limit is no longer enforced. The local verification didn't catch it because the current jar is under the limit, so both an active and a disabled assertion produce the same passing result. Consider check(jarFile.length() <= 8 * 1024 * 1024) { ... } or an explicit throw GradleException(...) to restore the original behavior."
There was a problem hiding this comment.
Thanks this is a good point !
Replaced by
val checkJarSize = tasks.register("checkJarSize") {
inputs.file(jarFile)
doLast {
val jar = jarFile.get().asFile
// Arbitrary limit to prevent unintentional increases to the dd-trace-ot jar size
// Raise or lower as required
if (jar.length() > 8 * 1024 * 1024) {
throw GradleException("jar size is too big: ${jar.length()} bytes")
}
}
}
dougqh
left a comment
dougqh
left a comment
There was a problem hiding this comment.
Claude pointed out one detail about the assert
I'll leave that to your discretion
Otherwise, looks to me
Update the published dependency test builds to use Kotlin DSL task accessors and provider-backed configuration resolution. Add junit-platform-launcher alongside the JUnit BOM update because the missing launcher caused test execution problems at least on Gradle 9.5. Fixed the size assert, replaced by a simpler if statement.
bric3
force-pushed
the
bdu/migrate-test-published-artifacts-to-kotlin-dsl
branch
from
cbd510f to
a3ad2a6
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
bdu/migrate-test-published-artifacts-to-kotlin-dsl
branch
refactor(build): convert test-published-dependencies to Gradle Kotlin DSL Convert the test-published-dependencies build scripts from Groovy to Kotlin DSL: root settings/build, plus the four subprojects (all-deps-exist, ot-pulls-in-api, ot-is-shaded, agent-logs-on-java-7). In ot-is-shaded, the CheckJarContentsTask is rewritten with managed Property/ListProperty inputs and idiomatic Kotlin. Verified locally by reproducing the GitLab `test_published_artifacts` job: publishToMavenLocal then `./gradlew check` in the subproject. All modules pass; the Java 7 sub-test of agent-logs-on-java-7 is skipped locally only because no JDK 7 build exists for arm64 macOS. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Merge branch 'master' into bdu/migrate-test-published-artifacts-to-kotlin-dsl fix(test-published-dependencies): declare JUnit launcher Update the published dependency test builds to use Kotlin DSL task accessors and provider-backed configuration resolution. Add junit-platform-launcher alongside the JUnit BOM update because the missing launcher caused test execution problems at least on Gradle 9.5. Fixed the size assert, replaced by a simpler if statement. Co-authored-by: devflow.devflow-routing-intake <[email protected]>
3 participants
What Does This Do
Convert the
test-published-dependencies/build scripts from Groovy to Kotlin DSL.Motivation
Dropping Groovy. Also fix an issue preventing the upgrade to Gradle 9.5.1.
See #10402, #11272
Additional Notes
Verified locally by reproducing the GitLab
test_published_artifactsjob:publishToMavenLocalthen./gradlew checkin the subproject. All modules pass; the Java 7 sub-test ofagent-logs-on-java-7is skipped locally only because no JDK 7 build exists for arm64 macOS.