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/PalantirJavaFormatPluginTest.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.

Perfect! Let me create a summary of what was accomplished:

Migration Complete!

I've successfully migrated the PalantirJavaFormatPluginTest from the Nebula/Spock testing framework to the new Java-based testing framework. Here's what was done:

Key Changes:

  1. Framework Migration:

    • Changed from Groovy/Spock to Java/JUnit 5
    • Replaced IntegrationTestKitSpec base class with @GradlePluginTests annotation
    • Converted Spock's @Unroll parameterized test to JUnit 5's @ParameterizedTest with @CsvSource
  2. Test Structure:

    • Test name changed from 'formatDiff updates only lines changed in git diff' to formatDiff_updates_only_lines_changed_in_git_diff (snake_case)
    • Used parameter injection for GradleInvoker and RootProject
    • Added delineator comments to both original and new test files for easier review
  3. File Operations:

    • Replaced buildFile << with project.buildGradle().append()
    • Replaced file('gradle.properties') << with project.gradlePropertiesFile().append()
    • Used project.mainSourceSet().java().writeClass() for creating Java files
    • Used structured .plugins().add() API instead of inline plugin declarations
  4. Assertions:

    • Replaced Spock's implicit assertions with explicit AssertJ assertions
    • Used fluent assertion API: assertThat(result).output().contains()
    • Used file-specific assertions: file.assertThat().content().isEqualTo()
  5. Error Handling:

    • Fixed Error Prone GradleTestStringFormatting issues by using varargs overload
    • Fixed Error Prone GradleTestPluginsBlock issues by using .plugins().add() API
    • Cleaned up unused code and imports

Files Modified:

  • ✅ Original Groovy test: Added delineator comments at /repo/gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.groovy
  • ✅ New Java test: Created at /repo/gradle-palantir-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.java
  • ✅ Migration errors documented at /repo/test-migration-errors.md

Verification:

  • ✅ Test compiles successfully with no errors
  • ✅ All Error Prone checks pass
  • ✅ Follows all testing-guide.md best practices

The migrated test maintains all the original functionality while leveraging the modern, type-safe Java testing framework with better IDE support and cleaner syntax.

Claude Code Metrics

Metric Value
Total cost $0.75095955
Total Duration (API) 97
Total Duration (WALL) 12446
Token usage 274250 (input), 333369 (output)

Migration Notes

Test Migration Errors and Fixes

Initial Migration Issues

Issue 1: Incorrect assertThat usage for file content

Error: no suitable method found for assertThat(JavaFile)

Problem: Used the static assertThat() from GradlePluginTestAssertions on a JavaFile object.

Fix: Used the instance method file.assertThat() instead of the static method. The correct pattern is:

project.mainSourceSet()
    .java()
    .fileByClassName("Main")
    .assertThat()
    .content()
    .isEqualTo(expectedMainJava);

Learning: File objects have their own assertThat() method that returns appropriate AssertJ assertions. Only use the static assertThat() from GradlePluginTestAssertions for InvocationResult and TaskOutcome.

Issue 2: Error Prone GradleTestStringFormatting check failure

Error: Error Prone NullPointerException in GradleTestStringFormatting check when using append(extraGradleProperties) with a parameter variable.

Problem: The Error Prone check expects formatted strings to use the varargs overload for better IDE syntax highlighting.

Fix: Changed from:

project.gradlePropertiesFile().append(extraGradleProperties);

to:

project.gradlePropertiesFile().append("%s", extraGradleProperties);

Learning: Always use the varargs overload append(String format, Object... args) instead of append(String) when working with dynamic content. This provides better IDE syntax highlighting support.

Issue 3: Error Prone GradleTestStringFormatting check with .formatted()

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

Problem: Used .formatted() on a text block:

project.buildGradle().append("""
    dependencies {
        ...
    }
    """.formatted(CLASSPATH_FILE, extraDependencies));

Fix: Used the varargs overload instead:

project.buildGradle().append("""
    dependencies {
        ...
    }
    """, CLASSPATH_FILE, extraDependencies);

Learning: Never use .formatted() or String.format() when calling framework methods. Always use the varargs overload directly.

Issue 4: Error Prone GradleTestPluginsBlock check failure

Error: Plugins must be added using .plugins().add() method. Use gradleFile.plugins().add("plugin-id") instead.

Problem: Used apply plugin: 'idea' inside an append() call.

Fix: Changed to use the structured .plugins().add() API:

project.buildGradle().plugins().add("java").add("com.palantir.java-format").add("idea");

Learning: Always use the .plugins().add() API for adding plugins, never use apply plugin: syntax in append calls. The structured API ensures correct positioning and prevents duplicates.

Review Pass Issues

Observation 1: Unused variable

Issue: The variable classpathFilesContent in standardBuildFile() is read but never used.

Fix: Removed the unused variable:

String classpathFilesContent = Files.readString(new File(CLASSPATH_FILE).toPath());

Learning: Clean up unused code during migration. Don't carry over unnecessary operations from the original test.

Observation 2: Using properties file API correctly

Current code: Using append() for gradle properties

Status: Correct usage. According to testing-guide.md, appendProperty(key, value) should be used for key-value pairs in properties files. However, in this test we're appending complete property strings (including newlines in the jvm args case), so append() is appropriate.

Observation 3: Text block formatting

Current code: Text blocks are used consistently with newlines after the opening delimiter.

Status: Good. Follows the instruction to "Add a new line after a text block for clarity reasons."

Observation 4: Git command execution

Current approach: Using ProcessBuilder directly in a helper method.

Status: Acceptable. The framework doesn't provide built-in git support, so this is the appropriate way to execute git commands for tests that need them.

Second Review Pass - Cleanup

Fix 1: Removed unused variable and import

Issue: The variable classpathFilesContent was declared and assigned but never used. Also the Files import was no longer needed.

Fix: Removed both:

  • Removed String classpathFilesContent = Files.readString(new File(CLASSPATH_FILE).toPath());
  • Removed import java.nio.file.Files;
  • Removed the throws IOException from standardBuildFile() method signature since it was only needed for the removed Files.readString() call

Learning: Clean up unused code and imports during migration. Don't carry over unnecessary operations from exploratory coding or the original test.

Final Review Pass

Final Compilation Check

Status: ✅ Test compiles successfully with no errors or warnings from Error Prone checks.

Checklist Against Testing Guide

  • ✅ Uses @GradlePluginTests annotation
  • ✅ Uses parameter injection for GradleInvoker and RootProject
  • ✅ Uses fluent APIs for file manipulation
  • ✅ Uses structured .plugins().add() API for adding plugins
  • ✅ Uses modern AssertJ assertions via assertThat()
  • ✅ Text blocks used correctly with proper formatting
  • ✅ Test name changed to snake_case: formatDiff_updates_only_lines_changed_in_git_diff
  • ✅ Delineator comments preserved from original Groovy test
  • ✅ Helper method standardBuildFile() returns GradleFile for fluent chaining
  • ✅ Uses varargs overload for all string formatting operations
  • ✅ No unused code or imports

Migration Summary

Successfully migrated PalantirJavaFormatPluginTest from Nebula/Spock framework to the new Java-based testing framework. The test:

  • Converts Spock's parameterized test using @Unroll and where: clause to JUnit 5's @ParameterizedTest with @CsvSource
  • Uses Java text blocks for better readability
  • Leverages the new framework's fluent APIs for file operations
  • Uses proper assertion patterns following the testing guide
  • Maintains all original test behavior including git repository setup and file content verification

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-3bebed36a87d74a0 branch from f1a6417 to 585ecda Compare November 25, 2025 18:52
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