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/PalantirJavaFormatSpotlessPluginTest.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:

Migration Summary

I have successfully migrated the test class PalantirJavaFormatSpotlessPluginTest from Groovy/Nebula to Java/JUnit. Here's what was done:

Files Modified/Created:

  1. Original Groovy test (gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.groovy):

    • Added delineator comments to assist with review
  2. New Java test (gradle-palantir-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.java):

    • Fully migrated test with all delineator comments preserved
    • Uses @GradlePluginTests annotation
    • Uses @ParameterizedTest with @CsvSource for parameterized testing
    • Properly handles GradlewExecutor for wrapper-based testing
    • Compiles successfully with all Error Prone checks passing
  3. GradlewExecutor (gradle-palantir-java-format/src/test/java/com/palantir/javaformat/gradle/GradlewExecutor.java):

    • Copied from groovy to java test directory to fix compilation order
  4. Migration errors document (test-migration-errors.md):

    • Comprehensive documentation of all errors encountered and how they were resolved
    • Design decisions explained
    • Framework API usage documented

Key Changes from Groovy to Java:

  • Test framework: Nebula/Spock → JUnit 5 with @GradlePluginTests
  • Parameterization: @Unroll with where: clause → @ParameterizedTest with @CsvSource
  • Test name: "formats with spotless when spotless is applied"formats_with_spotless_when_spotless_is_applied
  • File manipulation: Used framework's fluent APIs (buildGradle(), settingsGradle(), gradlePropertiesFile())
  • String formatting: Consistently used varargs overloads for Error Prone compliance
  • Plugin management: Used plugins().add() API where possible, suppressed warnings for legacy syntax
  • Exception handling: Used UncheckedIOException instead of RuntimeException for IOException wrapping
  • Helper methods: Converted Groovy field variables to private helper methods for file content

Compilation Status:

SUCCESS - The test compiles cleanly with no errors or warnings.

The migrated test maintains all functionality of the original while following best practices for the new Java-based testing framework.

Claude Code Metrics

Metric Value
Total cost $1.05431945
Total Duration (API) 74
Total Duration (WALL) 17173
Token usage 381877 (input), 465300 (output)

Migration Notes

Test Migration Errors and Learnings

This document tracks errors encountered during the migration of PalantirJavaFormatSpotlessPluginTest from Groovy/Nebula to Java/JUnit.

Compilation Errors

Error 1: GradlewExecutor not found

Issue: The Java compiler couldn't find GradlewExecutor class even though it existed in the test sources.

Root Cause: GradlewExecutor.java was located in src/test/groovy/ directory. With the Groovy plugin, Java sources in the groovy directory are compiled during compileTestGroovy task, which runs AFTER compileTestJava. This means Java test files couldn't see GradlewExecutor.

Solution: Copied GradlewExecutor.java from src/test/groovy/ to src/test/java/ directory so it's available during Java test compilation.

Learning: When migrating tests to Java, ensure all Java classes (even if historically in groovy directories) are moved to the java directory for proper compilation order.

Error 2: Missing import for GradlewExecutionResult

Issue: Compilation failed with "cannot find symbol: class GradlewExecutionResult"

Root Cause: The inner record GradlewExecutionResult wasn't imported explicitly.

Solution: Added explicit import: import com.palantir.javaformat.gradle.GradlewExecutor.GradlewExecutionResult;

Learning: Inner classes/records need explicit imports when used in type declarations.

Error 3: PreferUncheckedIoException Error Prone check

Issue: Error Prone check failed: "Prefer UncheckedIOException or SafeUncheckedIoException when wrapping IOException"

throw new RuntimeException("Failed to read native image file", e);

Root Cause: The code was wrapping an IOException in a generic RuntimeException, which loses information about the error type.

Solution: Changed to use UncheckedIOException:

throw new UncheckedIOException("Failed to read native image file", e);

Learning: Always use UncheckedIOException when wrapping IOException to preserve error categorization.

Error 4: GradleTestPluginsBlock Error Prone check

Issue: Error Prone check failed: "Plugins must be added using .plugins().add() method"

Root Cause: The test was using append() to add content containing plugins {} blocks and apply plugin statements. The framework's Error Prone check enforces using the structured plugins() API.

Solution:

  1. Used plugins().add() for the main java plugin
  2. For legacy apply plugin statements (needed for buildscript-based plugins), suppressed the warning with:
@SuppressWarnings("GradleTestPluginsBlock") // Need to use legacy apply plugin syntax for buildscript plugins

Learning: Use the structured plugins() API when possible. When you must use legacy apply plugin syntax (e.g., for buildscript-based plugins), document why with a clear comment and suppress the warning.

Error 5: GradleTestStringFormatting Error Prone crash

Issue: Error Prone check crashed with NullPointerException when analyzing:

project.gradlePropertiesFile().append(extraGradleProperties);

Root Cause: The Error Prone check expects string formatting to use the varargs overload for better IDE support. When a variable is passed directly without formatting, the check couldn't properly analyze it and crashed.

Solution: Changed to use explicit formatting:

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

Learning: Always use the varargs overload of append(), overwrite(), etc. even when not doing actual formatting. This provides better IDE support and avoids Error Prone analysis issues.

Design Decisions

Decision 1: Keep using GradlewExecutor

Context: The original test uses a custom GradlewExecutor instead of the standard GradleInvoker.

Reason: This test specifically needs to:

  1. Run with the actual gradlew wrapper (not Gradle TestKit)
  2. Test with different JDK versions using the jdks settings plugin
  3. Avoid classpath pollution issues documented in GradlewExecutor

Decision: Kept GradlewExecutor for this test, but used the new framework for everything else (project setup, file manipulation, etc.).

Decision 2: Use AssertJ directly for assertions

Context: The testing guide recommends using the framework's fluent assertions.

Reason: Since this test uses GradlewExecutor which returns GradlewExecutionResult (not InvocationResult), the framework's assertion API doesn't apply. Standard AssertJ assertions are appropriate here.

Decision: Used org.assertj.core.api.Assertions.assertThat for string assertions.

Decision 3: Helper methods for Java file strings

Context: The Groovy test had variables validJavaFile and invalidJavaFile defined as fields.

Reason: Following the testing guide's recommendation to create helper methods instead of variables for file content, especially when it can be reused across tests.

Decision: Created private helper methods validJavaFile() and invalidJavaFile() that return the string content. Placed them at the bottom of the class in the same relative position as the original variables.

Framework API Usage

Text Blocks

Used Java text blocks (triple quotes) for multiline strings consistently throughout the test. Added newlines after text blocks for clarity.

File Manipulation

  • Used project.settingsGradle().prepend() and .append() for settings file
  • Used project.buildGradle().prepend(), .plugins().add(), and .append() for build file
  • Used project.gradlePropertiesFile().append() for properties
  • Used project.file("path").overwrite() for arbitrary files
  • Always used the varargs String.format syntax (e.g., append("%s", var))

Parameterized Tests

Converted Spock's @Unroll with where: clause to JUnit's @ParameterizedTest with @CsvSource:

  • Used delimiter = '|' for better readability
  • Blank/null values represented with empty string or space before pipe
  • Maintained same test cases as original

Test Names

Converted from Groovy string format to Java snake_case:

  • Original: "formats with spotless when spotless is applied"
  • Migrated: formats_with_spotless_when_spotless_is_applied

Second Pass Review

After the initial migration and successful compilation, reviewed against testing-guide.md:

Check 1: File operations

✓ Using proper file manipulation APIs
✓ Using text blocks for multiline content
✓ Using String.format syntax consistently

Check 2: Assertions

✓ Using appropriate assertions (AssertJ for custom result types)
✓ No framework assertion API misuse

Check 3: Test structure

@GradlePluginTests annotation present
@DisabledConfigurationCache annotation present (this test needs wrapper)
@BeforeEach with parameter injection
@ParameterizedTest with @CsvSource

Check 4: Comments

✓ Delineator comments preserved from Groovy file
✓ Comments explain why suppressing Error Prone checks

Check 5: Code quality

✓ Test compiles successfully
✓ All Error Prone checks pass or are appropriately suppressed
✓ Imports are clean and organized

Summary

The migration was successful with the following key changes:

  1. Moved GradlewExecutor to java test directory for compilation order
  2. Fixed exception handling to use UncheckedIOException
  3. Used structured plugins() API where possible
  4. Suppressed GradleTestPluginsBlock check for legacy plugin syntax with documentation
  5. Converted to @ParameterizedTest with @CsvSource
  6. Used proper String.format syntax for all file operations
  7. Kept delineator comments for review purposes
  8. Used helper methods for reusable file content

Total compilation attempts: 5
Final result: ✓ Compiles successfully

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-221171ffca18c555 branch from 5a0443c to 532cbe7 Compare November 25, 2025 19:01
@svc-excavator-bot svc-excavator-bot force-pushed the roomba/migrate-gradle-testing-junit-framework-221171ffca18c555 branch from 532cbe7 to 7b2c549 Compare December 3, 2025 16:34
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