Excavator: Migrate Groovy nebula test TestReportFormattingPluginIntegrationSpec to the new Java Junit framework
#870
+2,478
−55
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/TestReportFormattingPluginIntegrationSpec.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 create a summary of what was accomplished:
Summary
I have successfully migrated the test class
TestReportFormattingPluginIntegrationSpec.groovyfrom the Nebula/Spock framework to the new Java-based testing framework. Here's what was completed:Files Created/Modified:
Original Groovy Test (
gradle-witchcraft-logging/src/test/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationSpec.groovy):New Java Test (
gradle-witchcraft-logging/src/test/java/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationTest.java):@GradlePluginTestsannotation@AdditionallyRunWithGradle({"7.6.4", "8.8"})for multi-version testingformats_test_report_stdout_and_stderrGradleInvokerandRootProjectMigration Documentation (
test-migration-errors.md):Key Migration Changes:
@GradlePluginTestswhere:clause →@AdditionallyRunWithGradleannotationapplyPlugin(TestReportFormattingPlugin)→.plugins().add("com.palantir.witchcraft-logging-testreport")runTasksSuccessfully()→gradle.withArgs().buildsSuccessfully()runTasksWithFailure()→gradle.withArgs().buildsWithFailure().assertThat().content()writeUnitTest()→rootProject.testSourceSet().java().writeClass()Verification:
✅ The migrated test compiles successfully with
./gradlew :gradle-witchcraft-logging:compileTestJavaAll tests have been migrated following the testing guide's best practices, and the code is ready for review.
Claude Code Metrics
Migration Notes
Test Migration Errors and Fixes
Migration: TestReportFormattingPluginIntegrationSpec → TestReportFormattingPluginIntegrationTest
Initial Attempt - Wrong Package Names for Annotations
Error:
Root Cause:
I initially assumed the annotation classes were in the
com.palantir.gradle.testing.annotationpackage based on common convention, but they are actually incom.palantir.gradle.testing.junitpackage.Fix:
Changed imports from:
to:
How I Found the Fix:
Used
findcommand to locate the actual annotation class files in the framework:find /tmp/gradle-plugin-testing/gradle-plugin-testing-junit/src/main/java -name "GradlePluginTests.java"Initial Attempt - Missing AssertJ Import for String Assertions
Error:
Root Cause:
The
GradlePluginTestAssertions.assertThat()method is specifically for testing framework types (InvocationResult, TaskOutcome), not for general assertions like strings.Fix:
Added AssertJ import:
How I Found the Fix:
The testing guide shows that string assertions should use standard AssertJ methods. However, I later realized a better approach is to use the framework's fluent file assertion API.
Second Pass - Using Fluent File Assertion API
Change:
Instead of reading the file content into a String variable and asserting on it:
Changed to use the framework's fluent API:
Reason:
This is more idiomatic and follows the testing guide's recommendation (lines 703-707). It also removes the need for an intermediate variable and the AssertJ static import.
How I Found the Fix:
Reviewed the testing guide section on "File Assertions" which shows this pattern as the preferred approach.
Key Learnings
Always check actual package names: Don't assume package structures - use
findorgrepto locate classes in the framework source.Prefer framework-provided fluent APIs: The testing framework provides specialized fluent assertion methods (like
.assertThat().content()) that are more idiomatic than extracting values and using general assertion libraries.Read the testing guide thoroughly: The guide has specific sections on best practices for assertions, file operations, etc. Following these patterns from the start saves refactoring time.
Multi-version testing: The
@AdditionallyRunWithGradleannotation makes it easy to test across multiple Gradle versions. Just add the annotation at the class level with an array of version strings.Final Review Notes
After completing the migration and reviewing the testing guide:
Plugin application: The old test used
applyPlugin(TestReportFormattingPlugin)(a Nebula helper that applies a plugin by class reference). In the new framework, we use the plugin ID string"com.palantir.witchcraft-logging-testreport"with.plugins().add(). This is the correct approach for plugins defined in the same project.Multi-version testing strategy: The original test had
GRADLE_VERSIONS = ["7.6.4", "8.8", GradleVersion.current().getVersion()]with a Spockwhere:clause. In the new framework:@AdditionallyRunWithGradle({"7.6.4", "8.8"})adds these versions to the test matrixTest structure: The test correctly follows the framework patterns:
@BeforeEachneeded since the test is self-containedGradleInvokerandRootProjectJava source handling: The test Java class content is kept as a static final String constant (matching the original pattern), then passed to
writeClass()which automatically creates it in the correct location.All tests compile successfully and follow framework best practices.
To enable or disable this check, please contact the maintainers of Excavator.