Skip to content

Conversation

@svc-excavator-bot
Copy link
Collaborator

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/GradleJdkToolchainsIntegrationTest.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 brief summary of what was accomplished:

Summary

I have successfully migrated the test class GradleJdkToolchainsIntegrationTest from the old Nebula/Groovy testing framework to the new Java-based JUnit 5 testing framework. Here's what was completed:

Key Changes Made:

  1. Added delineator comments to the original Groovy test file for easier review comparison

  2. Migrated the test to Java with the following transformations:

    • Changed from Groovy/Spock to Java/JUnit 5
    • Applied @GradlePluginTests annotation instead of extending IntegrationSpec
    • Used parameter injection for GradleInvoker and RootProject
    • Converted Spock's where: blocks (multi-version testing is now automatic)
    • Changed test names from Spock format to snake_case
    • Used fluent assertion APIs from the new framework
    • Created helper methods to replace Groovy variables (e.g., standardBuildFileForJdk17Toolchain())
  3. Handled special requirements:

    • This test uniquely needs to run ./gradlew as a subprocess with custom GRADLE_USER_HOME to verify JDK setup
    • Kept custom execution methods (runGradlewTasksSuccessfully(), etc.) since the framework's TestKit runs in-process
    • Created workingDir inside the test project instead of using @TempDir (per Error Prone guidance)
  4. Fixed all compilation errors:

    • Copied package-private constants from GradleJdkTestUtils
    • Fixed API differences (.path().toFile() instead of .toFile())
    • Changed exception checking from .exception() to .output() assertions
    • Applied all Error Prone fixes:
      • Used UncheckedIOException instead of RuntimeException
      • Properly closed Files.list() streams with try-with-resources
      • Used varargs overloads instead of .formatted()
      • Replaced var with explicit types
      • Suppressed GradleTestPluginsBlock check where apply plugin: is required in buildscript blocks
  5. Test compiles successfully with all Error Prone checks passing

  6. Documented all errors in test-migration-errors.md for future reference

The migrated test maintains all functionality from the original while following the new framework's conventions and best practices.

Claude Code Metrics

Metric Value
Total cost $3.0640292
Total Duration (API) 57
Total Duration (WALL) 32345
Token usage 760626 (input), 798770 (output)

Migration Notes

Test Migration Errors and Resolutions

This document tracks errors encountered during the migration of GradleJdkToolchainsIntegrationTest from Groovy/Nebula to Java/JUnit 5, and how they were resolved.

First Compilation Pass

Error 1: Package-private constants in GradleJdkTestUtils

Error: SIMPLIFIED_JDK_11_VERSION has private access in GradleJdkTestUtils

Cause: In Groovy, static fields without an access modifier are package-private, not public. The Java test couldn't access these constants.

Resolution: Copied the constants directly into the test class as private static final fields.

Error 2: No toFile() method on ArbitraryFile

Error: cannot find symbol: method toFile()

Cause: ArbitraryFile is a record with a path() method, not a toFile() method.

Resolution: Changed .toFile() to .path().toFile() for all file references.

Error 3: No exception() method on InvocationResult

Error: cannot find symbol: method exception()

Cause: The new framework's InvocationResult doesn't have an exception() method. Build failures are communicated through output text.

Resolution: Changed from Throwables.getRootCause(failingCheck.exception()).getMessage() to checking the output text: failingCheck.output().contains("...")

Second Compilation Pass: Error Prone Checks

Error 4: GradleTestTemporaryFile

Error: Avoid using temporary directories or files in Gradle tests

Cause: Used @TempDir for workingDir, which Error Prone flags as hard to debug.

Resolution: Created workingDir inside the test project using a @BeforeEach method:

@BeforeEach
void setupWorkingDir(RootProject rootProject) throws IOException {
    workingDir = Files.createDirectories(rootProject.path().resolve("test-gradle-user-home"));
}

Error 5: GradleTestStringFormatting

Error: Use the varargs overload of methods to get syntax highlighting

Cause: Used .formatted() on text blocks instead of varargs parameters.

Resolution: Changed from:

.append("""...""".formatted(arg1, arg2))

to:

.append("""...""", arg1, arg2)

Error 6: PreferUncheckedIoException

Error: Prefer UncheckedIOException or SafeUncheckedIoException when wrapping IOException

Cause: Wrapped IOExceptions in generic RuntimeException.

Resolution: Changed throw new RuntimeException("...", e) to throw new UncheckedIOException("...", e) for all IOException wrapping.

Error 7: StreamResourceLeak

Error: Streams that encapsulate a closeable resource should be closed using try-with-resources

Cause: Multiple calls to Files.list() without try-with-resources.

Resolution: Created a helper method that properly closes streams:

private java.util.Set<String> getJdkDirectoryNames(Path jdksPath) throws IOException {
    try (Stream<Path> paths = Files.list(jdksPath)) {
        return paths.map(it -> it.getFileName().toString()).collect(Collectors.toSet());
    }
}

For one case that needed allMatch(), used try-with-resources inline.

Error 8: VarUsage

Error: The 'var' keyword results in illegible code in most cases and should not be used

Cause: Used var for SubProject declarations.

Resolution: Changed var subprojectLib21 = ... to explicit type com.palantir.gradle.testing.project.SubProject subprojectLib21 = ...

Error 9: GradleTestPluginsBlock

Error: Plugins must be added using .plugins().add() method

Cause: Used apply plugin: syntax in buildscript blocks within prepend() calls.

Resolution: Since these are in buildscript blocks with manual classpath injection (not regular plugin blocks), suppressed the check:

@SuppressWarnings("GradleTestPluginsBlock") // buildscript blocks require apply plugin syntax
private void applyJdksPlugins(RootProject rootProject) { ... }

Framework Differences Discovered

Custom Gradlew Execution

This test is unique in that it needs to run ./gradlew as a separate process with a custom GRADLE_USER_HOME to verify JDK setup works correctly. The new testing framework uses Gradle TestKit which runs in-process. To handle this:

  1. Kept custom methods: runGradlewTasksSuccessfully(), runGradlewTasksWithFailure(), runGradlewTasks(), and getProcessBuilder()
  2. These methods execute gradlew as a subprocess using ProcessBuilder
  3. The workingDir is set as the GRADLE_USER_HOME environment variable

Multi-version Testing

The old Groovy test used Spock's data tables with where: gradleVersionNumber << GRADLE_TEST_VERSIONS. The new framework handles multi-version testing automatically through the @GradlePluginTests annotation, so individual test methods don't need parameters.

Second Review Pass

After reviewing the testing-guide.md and the migrated test:

  1. Delineator comments preserved: All // ***DELINEATOR FOR REVIEW: comments were preserved from the Groovy test to assist human reviewers.

  2. Helper methods for build file setup: Created standardBuildFileForJdk17Toolchain() method to replace the pattern from the Groovy test, returning GradleFile for fluent chaining.

  3. Proper exception handling: All IOException wrapping uses UncheckedIOException as required by Error Prone.

  4. Resource management: All streams from Files.list() are properly closed.

  5. Explicit types: No use of var keyword to maintain code readability.

Summary

The migration was successful after addressing compilation errors related to:

  • Framework API differences (no toFile(), no exception())
  • Access modifiers in Groovy vs Java
  • Error Prone best practices (resource management, exception handling, var usage)
  • Special handling for tests that need subprocess execution of gradlew

The migrated test maintains all the functionality of the original while following the new framework's conventions and Error Prone checks.

To enable or disable this check, please contact the maintainers of Excavator.

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