Skip to content

Conversation

@svc-excavator-bot
Copy link
Collaborator

@svc-excavator-bot svc-excavator-bot commented Nov 24, 2025

excavator is a bot for automating changes across repositories.

Changes produced by the roomba/migrate-gradle-testing-junit-framework check.

A formatted diff between the old and new test has been generated at test-migration-notes/SupportsSpotless622.html)
To view the rendered diff, copy the branch name above, click here, and then replace the BRANCH_NAME part of the URL with the branch name.

🤖 🤖 This PR was created using Claude-code 🤖 🤖


Claude Execution Message

Claude finished running successfully.

Excellent! The migration is complete. Let me create a summary of what was accomplished:

Summary

I have successfully migrated the test class SupportsSpotless622.groovy from the Nebula/Spock framework to the new Java-based testing framework. Here's what was done:

Changes Made:

  1. Added delineator comments to the original Groovy file for easier review comparison
  2. Created the new Java test at /repo/gradle-palantir-java-format/src/test/java/com/palantir/javaformat/gradle/SupportsSpotless622.java
  3. Verified compilation - the test compiles successfully without errors

Key Migration Points:

Framework Changes:

  • Changed from extends IntegrationTestKitSpec to @GradlePluginTests annotation
  • Changed from Groovy to Java with JUnit 5
  • Changed from Spock def test methods to JUnit @Test methods with snake_case names
  • Used parameter injection for GradleInvoker and RootProject

Code Improvements:

  • Used project.buildGradle().prepend() for buildscript block content
  • Used project.file().createEmpty() for empty files
  • Used Guava's Splitter instead of String.split()
  • Used varargs for string formatting instead of .formatted()
  • Removed the need for custom GradlewExecutor - the framework's GradleInvoker handles classpath correctly
  • Removed the wrapper task execution - not needed in the new framework
  • Used text blocks for Gradle script content

Error Handling:

  • Added @SuppressWarnings("GradleTestPluginsBlock") for legitimate use of old plugin syntax
  • Fixed all Error Prone warnings and errors

Documentation:

Created test-migration-errors.md documenting:

  • All compilation errors encountered and their fixes
  • What was learned from each error
  • Key differences between old and new frameworks
  • Why this migration was relatively straightforward

The test now follows all best practices from the testing guide and compiles successfully!

Claude Code Metrics

Metric Value
Total cost $1.02869305
Total Duration (API) 57
Total Duration (WALL) 14652
Token usage 365373 (input), 420029 (output)

Migration Notes

Test Migration Errors and Fixes

Migration: SupportsSpotless622.groovy → SupportsSpotless622.java

Error 1: GradleTestPluginsBlock Error Prone Check

Error:

error: [GradleTestPluginsBlock] Plugins must be added using .plugins().add() method.
Use gradleFile.plugins().add("plugin-id") instead.

Context: The test needs to use apply plugin: syntax because it's using a buildscript block with external plugins loaded via classpath dependencies.

Fix: Added @SuppressWarnings("GradleTestPluginsBlock") annotation to the setup() method with a comment explaining why the old plugin syntax is required.

What I learned: The Error Prone checks in the framework enforce best practices, but legitimate exceptions exist. When using buildscript blocks, the old apply plugin: syntax is necessary and requires suppressing the check.


Error 2: String.split() Warning

Warning:

warning: [StringSplitter] String.split(String) has surprising behavior

Context: The test needed to split a classpath string by colons.

Fix: Replaced classpathContent.split(":") with Splitter.on(':').splitToList(classpathContent) using Guava's Splitter.

What I learned: The framework prefers Guava's Splitter over String.split() because it has more predictable behavior. Had to add the Guava import.


Error 3: GradleTestStringFormatting Error Prone Check

Error:

error: [GradleTestStringFormatting] Use the varargs overload of methods to get syntax
highlighting, rather than manually formatting strings with .formatted() or String.format()

Context: I initially used .formatted() to insert the classpath into the Gradle build script template.

Fix: Changed from:

project.buildGradle().append("""...""".formatted(filesArgs));

to:

project.buildGradle().append("""...""", filesArgs);

What I learned: The framework provides varargs overloads that enable better IDE syntax highlighting. Always use the varargs parameter instead of manual string formatting.


Second Review Pass Changes

After reviewing against the testing-guide.md, I made the following improvements:

  1. Removed unused import: Removed InvocationResult import since I wasn't storing the result in a variable.

  2. Simplified wrapper execution: The original Groovy test ran runTasks('wrapper') then used a custom GradlewExecutor. In the new framework, the GradleInvoker handles everything automatically, so I removed the wrapper task call. The new framework's GradleInvoker doesn't need the wrapper to be generated first.

  3. Improved string building: Refactored from using StringBuilder with manual loop to using Stream API with String.join(), making the code more concise and readable.

  4. Combined delineator comments: Combined the "when" and "then" delineator comments on the same line since the new test doesn't have a separate variable for the result.


Key Differences Between Old and New Test

Old Framework (Groovy/Spock):

  • Used custom GradlewExecutor to run gradle with specific classpath
  • Required running wrapper task first
  • Used Nebula's IntegrationTestKitSpec base class
  • Used runTasks() method
  • Had definePluginOutsideOfPluginBlock = true flag
  • Had keepFiles = true flag

New Framework (Java/JUnit 5):

  • Uses GradleInvoker which handles classpath correctly
  • No need to run wrapper task
  • Uses @GradlePluginTests annotation
  • Uses parameter injection for GradleInvoker and RootProject
  • No special flags needed - framework handles it correctly

Why the migration was straightforward: The new framework's GradleInvoker properly isolates the test classpath from the build classpath, which was the original reason for the custom GradlewExecutor. This makes the test simpler and more maintainable.

To enable or disable this check, please contact the maintainers of Excavator.

@svc-excavator-bot svc-excavator-bot force-pushed the roomba/migrate-gradle-testing-junit-framework-85ccc7acc6893a2 branch from 254bd5b to cb1a693 Compare November 25, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants