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 can be viewed here

🤖 🤖 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 done:

Migration Summary

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

Key Changes:

  1. Language and Framework:

    • Migrated from Groovy/Spock to Java/JUnit 5
    • Changed from IntegrationSpec base class to @GradlePluginTests annotation
    • Replaced Spock's parameterized testing with the framework's automatic multi-version testing
  2. Test Structure:

    • Converted void setup() to @BeforeEach void setup()
    • Used parameter injection for RootProject, MavenRepo, SubProject, and GradleInvoker
    • Changed test name from '#gradleVersionNumber can consume recommended product dependencies project'() to can_consume_recommended_product_dependencies_project()
  3. Maven Repository:

    • Replaced generateMavenRepo() with MavenRepo parameter injection
    • Used repo.publish(MavenArtifact.of(...)) for publishing artifacts
  4. Plugin Management:

    • Used structured plugins().add() API instead of apply plugin strings
    • Added external plugin com.palantir.sls-packaging:gradle-sls-packaging:7.31.0 to gradlePluginForTesting configuration in build.gradle
  5. Build Execution:

    • Changed runTasks() to gradle.withArgs().buildsSuccessfully()
  6. Assertions:

    • Replaced Spock's task outcome collection with individual task assertions
    • Used assertThat(result).task(":taskPath").succeeded() for each task
  7. Conditional Test Execution:

    • Replaced Spock's @IgnoreIf with JUnit 5's assumeFalse() using TestInfo to check for Gradle 9
  8. Delineator Comments:

    • Added review delineator comments to both the original Groovy file and the new Java file as requested

Files Modified:

  • ✅ Added delineators to: /repo/src/test/groovy/com/palantir/gradle/versions/SlsPackagingCompatibilityIntegrationSpec.groovy
  • ✅ Created new test: /repo/src/test/java/com/palantir/gradle/versions/SlsPackagingCompatibilityIntegrationTest.java
  • ✅ Updated: /repo/build.gradle (added gradlePluginForTesting dependency)
  • ✅ Created: /repo/test-migration-errors.md (documentation of errors and fixes)

The new test compiles successfully and follows all best practices from the testing guide.

Claude Code Metrics

Metric Value
Total cost $1.4599948499999997
Total Duration (API) 62
Total Duration (WALL) 17512
Token usage 429502 (input), 443488 (output)

Migration Notes

Test Migration Errors and Fixes

First Pass - Initial Migration

Error 1: Method signature for addWithoutApply

Issue: Used addWithoutApply("com.palantir.sls-java-service-distribution", "7.31.0") which doesn't accept a version parameter.

Fix:

  • Read the Plugins.java source code and discovered that addWithoutApply only accepts a plugin ID, not a version.
  • According to the testing guide, external plugins with versions should be added to gradlePluginForTesting configuration in build.gradle.
  • Added gradlePluginForTesting 'com.palantir.sls-packaging:gradle-sls-packaging:7.31.0' to build.gradle dependencies.
  • Changed code to use addWithoutApply("com.palantir.sls-java-service-distribution") without version.

Error 2: No method to get tasks by outcome

Issue: Used assertThat(wroteLocks).tasks(TaskOutcome.SUCCESS) which doesn't exist in the new framework.

Fix:

  • Read InvocationResult.java and InvocationResultAssert.java and found that there's no method to get all tasks with a specific outcome.
  • The new framework requires checking each task individually.
  • Changed to multiple individual assertions: assertThat(wroteLocks).task(":api:compileRecommendedProductDependencies").succeeded()
  • This is more verbose but provides clearer failure messages.

Error 3: Error Prone check for plugin blocks

Issue: Error Prone check GradleTestPluginsBlock was triggered when using append() with plugin declarations.

Fix:

  • Used the structured plugins() API instead of raw string append for all plugin declarations.
  • Changed apply plugin: 'java' to buildGradle().plugins().add("java")
  • This ensures proper plugin block positioning and prevents duplicate entries.

Second Pass - Review Against testing-guide.md

Issue 1: Unused imports

Issue: The test had unused imports java.util.Set and org.gradle.testkit.runner.TaskOutcome.

Fix:

  • Removed these imports as they were left over from the initial migration when I was trying to use tasks(TaskOutcome.SUCCESS) method that doesn't exist.

Review Summary

The migrated test follows all the best practices from the testing-guide.md:

  • ✅ Uses @GradlePluginTests annotation
  • ✅ Uses parameter injection for GradleInvoker, RootProject, MavenRepo, and SubProject
  • ✅ Uses @BeforeEach for setup
  • ✅ Uses MavenRepo.publish() with MavenArtifact.of()
  • ✅ Uses the structured plugins() API for all plugin declarations
  • ✅ Uses buildsSuccessfully() instead of .build()
  • ✅ Uses assertThat() from GradlePluginTestAssertions
  • ✅ Uses propertiesFile().appendProperty() for versions.props
  • ✅ Uses text blocks for multi-line strings
  • ✅ Test name follows snake_case convention
  • ✅ Keeps comments from the original Groovy test
  • ✅ External plugin added to gradlePluginForTesting configuration
  • ✅ Uses JUnit 5 Assumptions for conditional test execution

Multi-version testing notes

  • The original test used Spock's where: block with gradleVersionNumber << GRADLE_VERSIONS for parameterized testing.
  • The new framework automatically runs tests against multiple Gradle versions configured in gradleTestUtils.gradleVersions in build.gradle.
  • For conditional execution based on Gradle version, used JUnit 5's assumeFalse with TestInfo to check the display name (which includes the Gradle version).
  • This is a reasonable approach given that GradleVersion cannot be directly injected as a test parameter in the current framework.

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-36debc64a3cf516f branch from 59d9473 to 602cca0 Compare November 24, 2025 10:53
writeVersionsLocks
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.

4 participants