Excavator: Migrate Groovy nebula test PalantirJavaFormatIdeaPluginTest to the new Java Junit framework
#1480
+3,567
−68
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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/PalantirJavaFormatIdeaPluginTest.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! The migration is complete. Let me provide a summary of what was done:
Migration Summary
I've successfully migrated the test class from Nebula/Spock (Groovy) to the new Java-based testing framework. Here's what was completed:
Changes Made:
Added delineator comments to the original Groovy test for easier comparison during review (in
/repo/gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.groovy)Created the new Java test at
/repo/gradle-palantir-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.javawith:@GradlePluginTestsannotation instead of extending IntegrationTestKitSpecGradleInvokerandRootProjectplugins()API instead ofapply pluginstringsappendProperty()for gradle properties fileUncheckedIOExceptionVerified compilation - the test compiles successfully
Created error tracking file at
/repo/test-migration-errors.mddocumenting:Key Migration Decisions:
@Testmethods instead of using@ParameterizedTest, as this provides clearer test names and simpler logicThe migrated test follows all the best practices from the testing guide and compiles successfully!
Claude Code Metrics
Migration Notes
Test Migration Errors and Fixes
Initial Migration Pass
Error 1: RuntimeException instead of UncheckedIOException
Issue: Used
RuntimeExceptionwhen wrappingIOExceptioninreadNativeImageFile()method.Error Message:
Fix: Changed from
throw new RuntimeException("Failed to read native image file", e);tothrow new UncheckedIOException("Failed to read native image file", e);Source: Error Prone check caught this during compilation.
Error 2: Using append() instead of structured plugins() API
Issue: Attempted to use
apply plugin: 'idea'within anappend()call instead of using the structured plugins API.Error Message:
Fix: Changed from:
To:
Source: Error Prone check from the testing framework. Also mentioned in testing-guide.md under "Plugin Management".
Error 3: Parameterized test complexity
Issue: Initially tried to use JUnit's
@ParameterizedTestwith@CsvSourceto replicate Spock's where clause, but this caused issues with string handling and Error Prone checks.Fix: Split the test into two separate methods:
idea_configures_xml_files()- tests without native formatteridea_configures_xml_files_with_native_formatter()- tests with native formatterThis approach is cleaner and more explicit than trying to force parameterization for this use case.
Source: Trial and error during initial compilation attempts.
Review Pass 1
Review Item 1: Text Blocks
Issue: Need to ensure text blocks have blank lines after them for clarity.
Status: ✓ Text blocks properly formatted with blank lines in both test methods.
Review Item 2: Use of String.format
Issue: Need to verify the String.format syntax is used correctly with the append() method.
Status: ✓ Using varargs format correctly:
.append("""...""", NATIVE_CONFIG)Review Item 3: Assertion Chaining
Issue: Could potentially chain assertions when checking multiple things on XML parsing results.
Status: Current approach is acceptable. The assertions are separate because they check different boolean flags with different descriptive messages. Chaining wouldn't provide a benefit here.
Review Item 4: File Path Construction
Issue: Using
.path()to get Path object when we might be able to use framework file assertions directly.Status: Current approach is necessary because we need to parse XML files using standard Java XML parsing APIs. The framework doesn't provide XML-specific assertions, so using
.path()to get the Path object for XML parsing is appropriate.Review Pass 2
No additional issues found. The test:
To enable or disable this check, please contact the maintainers of Excavator.