SpotlessApply uses by default the Java formatter #1243
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| type: fix | ||
| fix: | ||
| description: SpotlessApply uses Java 21 version | ||
| links: | ||
| - https://github.com/palantir/palantir-java-format/pull/1243 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,11 @@ package com.palantir.javaformat.gradle | |
| import nebula.test.IntegrationTestKitSpec | ||
| import spock.lang.Unroll | ||
|
|
||
| import java.nio.charset.StandardCharsets | ||
| import java.nio.file.Path | ||
| import java.util.stream.Collectors | ||
| import java.util.stream.Stream | ||
|
|
||
| class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec { | ||
| /** ./gradlew writeImplClasspath generates this file. */ | ||
| private static final CLASSPATH_FILE = new File("build/impl.classpath").absolutePath | ||
|
|
@@ -26,20 +31,61 @@ class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec { | |
|
|
||
|
|
||
| @Unroll | ||
| def "formats with spotless when spotless is applied"(String extraGradleProperties, String javaVersion, String expectedOutput) { | ||
| def extraDependencies = extraGradleProperties.isEmpty() ? "" : NATIVE_CONFIG | ||
| settingsFile << """ | ||
| buildscript { | ||
| repositories { | ||
| mavenCentral() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } } | ||
| gradlePluginPortal() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } } | ||
| } | ||
| dependencies { | ||
| classpath 'com.palantir.gradle.jdks:gradle-jdks-settings:0.62.0' | ||
| } | ||
| } | ||
| apply plugin: 'com.palantir.jdks.settings' | ||
| """.stripIndent(true) | ||
|
|
||
| buildFile << """ | ||
| buildscript { | ||
| repositories { | ||
| mavenCentral() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } } | ||
| gradlePluginPortal() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } } | ||
| } | ||
| dependencies { | ||
| classpath 'com.palantir.baseline:gradle-baseline-java:6.21.0' | ||
| classpath 'com.palantir.gradle.jdks:gradle-jdks:0.62.0' | ||
| classpath 'com.palantir.gradle.jdkslatest:gradle-jdks-latest:0.17.0' | ||
| classpath files(FILES) | ||
| } | ||
| } | ||
|
|
||
| // The 'com.diffplug.spotless:spotless-plugin-gradle' dependency is already added by palantir-java-format | ||
| plugins { | ||
| id 'java' | ||
| } | ||
|
|
||
| apply plugin: 'com.palantir.java-format' | ||
| apply plugin: 'com.palantir.baseline-java-versions' | ||
| apply plugin: 'com.palantir.jdks' | ||
| apply plugin: 'com.palantir.jdks.latest' | ||
|
|
||
| dependencies { | ||
| palantirJavaFormat files(file("${CLASSPATH_FILE}").text.split(':')) | ||
| EXTRA_CONFIGURATION | ||
| } | ||
|
|
||
| javaVersions { | ||
| libraryTarget = JAVA_VERSION | ||
|
There was a problem hiding this comment. Can we just string interpolate here? |
||
| } | ||
|
|
||
| jdks { | ||
| daemonTarget = JAVA_VERSION | ||
| } | ||
|
|
||
| """.replace("FILES", getBuildPluginClasspathInjector().join(",")) | ||
| .replace("JAVA_VERSION", javaVersion) | ||
| .replace("EXTRA_CONFIGURATION", extraDependencies).stripIndent() | ||
|
|
||
| // Add jvm args to allow spotless and formatter gradle plugins to run with Java 16+ | ||
| file('gradle.properties') << """ | ||
|
|
@@ -48,6 +94,7 @@ class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec { | |
| --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \ | ||
| --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \ | ||
| --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED | ||
| palantir.jdk.setup.enabled=true | ||
| """.stripIndent() | ||
| file('gradle.properties') << extraGradleProperties | ||
|
|
||
|
|
@@ -56,19 +103,58 @@ class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec { | |
| """.stripIndent() | ||
|
|
||
| file('src/main/java/Main.java').text = invalidJavaFile | ||
| runTasks('wrapper') | ||
|
|
||
| when: | ||
| def result = runGradlewTasks('spotlessApply', '--info') | ||
|
|
||
| then: | ||
| result.contains(expectedOutput) | ||
| file('src/main/java/Main.java').text == validJavaFile | ||
|
|
||
| where: | ||
| extraGradleProperties | javaVersion | expectedOutput | ||
| "" | 21 | "Using the Java-based formatter" | ||
| "palantir.native.formatter=true" | 21 | "Using the Java-based formatter" | ||
| "palantir.native.formatter=true" | 17 | "Using the native-image formatter" | ||
|
|
||
| } | ||
|
|
||
| private static Iterable<File> getBuildPluginClasspathInjector() { | ||
|
There was a problem hiding this comment. should I move this to https://github.com/palantir/gradle-plugin-testing ? gradle-jdks needs to do the same thing in order to run on multiple Java versions. There was a problem hiding this comment. I don't understand what it's supposed to do? There was a problem hiding this comment. I suppose what I don't understand is why you need this at all? I thought nebula does this for you? There was a problem hiding this comment. I need to test how the task works against Java 17 and Java 21. So, I am using the I have the same setup in There was a problem hiding this comment. oh, I thought the tooling api invoked gradlew, interesting! you could move it to gradle-plugin-testing, it's a bit niche, but I don't see why not. There was a problem hiding this comment. I think a nicer way to do this, is probably to reuse some of the stuff in Nebula, it makes your life a lot easier I think:
I think the current approach - I also mentioned something similar in the testing plugin PRs - detracts from the test by adding a bunch of cruft that's not actually important to know for the test, but is unfortunately required for the test to pass. There was a problem hiding this comment. It also means, that you can hide all of this behind a There was a problem hiding this comment. An aside: I think you could probably ship this method + |
||
| return getPluginClasspathInjector(Path.of("../gradle-palantir-java-format/build/pluginUnderTestMetadata/plugin-under-test-metadata.properties")) | ||
| } | ||
|
|
||
| private static Iterable<File> getPluginClasspathInjector(Path path) { | ||
| File propertiesFile = path.toFile() | ||
| Properties properties = new Properties() | ||
| propertiesFile.withInputStream { inputStream -> | ||
| properties.load(inputStream) | ||
| } | ||
| String classpath = properties.getProperty('implementation-classpath') | ||
| return classpath.split(File.pathSeparator).collect { "'" + it + "'" } | ||
| } | ||
|
|
||
| private String runGradlewTasks(String... tasks) { | ||
| ProcessBuilder processBuilder = getProcessBuilder(tasks) | ||
| Process process = processBuilder.start() | ||
| String output = readAllInput(process.getInputStream()) | ||
| return output | ||
| } | ||
|
|
||
| public static String readAllInput(InputStream inputStream) { | ||
| try (Stream<String> lines = | ||
| new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8)).lines()) { | ||
| return lines.collect(Collectors.joining("\n")); | ||
| } | ||
| } | ||
|
|
||
| private ProcessBuilder getProcessBuilder(String... tasks) { | ||
| List<String> arguments = ["./gradlew"] | ||
| Arrays.asList(tasks).forEach(arguments::add) | ||
| ProcessBuilder processBuilder = new ProcessBuilder() | ||
| .command(arguments) | ||
| .directory(projectDir).redirectErrorStream(true) | ||
| return processBuilder | ||
| } | ||
|
|
||
| def validJavaFile = '''\ | ||
|
|
||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a tad confusing, in both cases you're spinning up a new process.
Correct me if I've misunderstood, but I believe you want something along the lines of:
?