Skip to content

[G2J] call site instrumentation plugin #10462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jpbempel wants to merge 9 commits into
base: master
Choose a base branch
from
Open

[G2J] call site instrumentation plugin #10462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
44aeab8
[G2J] call site instrumentation plugin
jpbempel Jan 28, 2026
7f9c2d3
restore before migration
jpbempel Jan 29, 2026
d32ba4d
Moving groovy to java to keep history
jpbempel Jan 29, 2026
f97d8fc
restore migrated files
jpbempel Jan 29, 2026
68ecb35
Add script to move groovy files as java ones
jpbempel Jan 29, 2026
c7521ee
address comments
jpbempel Jan 29, 2026
4747f93
address comments
jpbempel Jan 31, 2026
f613ec8
add claude skill
jpbempel Feb 2, 2026
d85492f
remove script
jpbempel Feb 2, 2026
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
20 changes: 20 additions & 0 deletions .claude/skills/migrate-groovy-to-java/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
name: migrate-groovy-to-java
description: migrate test groovy files to java
---

Migrate test Groovy files to Java using JUnit 5

1. List all groovy files of the current gradle module
2. convert groovy files to Java using Junit 5
3. make sure the tests are still passing after migration
4. remove groovy files

Copy link
Contributor

@PerfectSlayer PerfectSlayer Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 suggestion: ‏

Suggested change
5. avoid duplicating code and refactor if possible to keep the codebase as light as possible

Copy link
Contributor

@AlexeyKuznetsov-DD AlexeyKuznetsov-DD Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Did you get a better code with this skill? Can we have some instructions how to use it?

Copy link
Member Author

@jpbempel jpbempel Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just type /migrate-groovy-to-java in claude code prompt

Copy link
Member Author

@jpbempel jpbempel Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding suggestion: Prefer to keep refactoring separate from migration

Copy link
Contributor

@PerfectSlayer PerfectSlayer Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion was more to make sure the generated code stays closer to what we have already. It sometimes use and abuse of patterns it copies/pastes everywhere.
But without way to evaluate prompt, it's tough to find out if it will be effective or not - if it applies it without refactoring the code logic.

When converting groovy code to java code make sure that:
Copy link
Contributor

@bric3 bric3 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New point:

- Use snake case for converted test methods.

- the Java code generated is compatible with JDK 8
- when translating Spock test, favor using `@CsvSource` with `|` delimiters
Copy link
Contributor

@bric3 bric3 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: New point:

when using a `@MethodSource`, use the test method name, and suffix it with `_parameters`

Copy link
Contributor

@PerfectSlayer PerfectSlayer Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be arguments according to JUnit naming?

Copy link
Contributor

@bric3 bric3 Feb 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes possibly, both are fine by me, but let's keep it closer to JUnit for this one.

- when using a `@MethodSource`, use the test method name, and suffix it with `_arguments`
- when converting tuples, create light dedicated structure instead to keep the typing system
- Instead of checking a state and throwing an exception, use JUnit asserts
- Do not wrap checked exception and throwing a Runtime exception, prefer adding a throws clause at method declaration
- Do not mark local variables `final`
Copy link
Contributor

@bric3 bric3 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: What about using AssertJ's richer assertion API rather than using JUnit ones. This would avoid unnecessary type conversions and would ease reading the code.

6 changes: 3 additions & 3 deletions buildSrc/call-site-instrumentation-plugin/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
plugins {
java
groovy
id("com.diffplug.spotless") version "8.2.1"
id("com.gradleup.shadow") version "8.3.9"
}
Expand Down Expand Up @@ -34,8 +33,9 @@ dependencies {
implementation(libs.javaparser.symbol.solver)

testImplementation(libs.bytebuddy)
testImplementation(libs.bundles.groovy)
testImplementation(libs.bundles.spock)
testImplementation(libs.bundles.junit5)
testRuntimeOnly(libs.junit.platform.launcher)
testImplementation(libs.bundles.mockito)
testImplementation("javax.servlet", "javax.servlet-api", "3.0.1")
testImplementation(libs.spotbugs.annotations)
}
Expand Down
Loading
Loading