Skip to content

Fix pjf crashing when we force realization of the spotlessJava task#1319

Closed
kelvinou01 wants to merge 1 commit into
developfrom
okelvin/spotlessJava-realized
Closed

Fix pjf crashing when we force realization of the spotlessJava task#1319
kelvinou01 wants to merge 1 commit into
developfrom
okelvin/spotlessJava-realized

Conversation

@kelvinou01

Copy link
Copy Markdown
Contributor

Before this PR

After this PR

==COMMIT_MSG==
==COMMIT_MSG==

Possible downsides?

@changelog-app

changelog-app Bot commented Jun 12, 2025

Copy link
Copy Markdown

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Fix pjf crashing when we force realization of the spotlessJava task

Check the box to generate changelog(s)

  • Generate changelog entry

import java.util.stream.Stream

/**
* IntegrationTestKitSpec currently [loads more than what it needs into the classpath](https://github.com/nebula-plugins/nebula-test/blob/c5d3af9004898276bde5c68da492c6b0b4c5facc/src/main/groovy/nebula/test/IntegrationTestKitBase.groovy#L136)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not valid javadoc (yet - it is in java 23). Can you use the proper links and what not. Not doing a proper review, was just curious what was going on

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep gotcha.

Comment on lines +61 to +62
GradlewExecutionResult result = new GradlewExecutionResult(process.exitValue(), output)
return result

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return directly

Comment on lines +90 to +94
ProcessBuilder processBuilder = new ProcessBuilder()
.command(arguments)
.directory(projectDir)
.redirectErrorStream(true)
return processBuilder

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return directly

* This classpath only contains the dependencies required by the plugin, as well as the plugin itself.
* This means no more eager loading of Formatters onto the classpath, no more complaints from PalantirJavaFormatStep
*/
class GradlewExecutor {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure this is used by the existing spotless test

}

private static Iterable<File> getBuildPluginClasspathInjector() {
return getPluginClasspathInjector(Path.of("../gradle-palantir-java-format/build/pluginUnderTestMetadata/plugin-under-test-metadata.properties"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try and get this from a system property, so you're inherently relying the on the structure of your build. Then in your gradle file, ensure that the task that will run this test, has it as a system property.

If you were to make the change to nebula, I imagine this is what you'd want to do. i.e. if the system property is there, then use that, if not then fallback to whatever mechanism they currently have.

@kelvinou01 kelvinou01 closed this Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants