-
Notifications
You must be signed in to change notification settings - Fork 2
Create the gradle/gradle-test-versions.yml file #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generate changelog in
|
✅ Successfully generated changelog entry!Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. 📋Changelog Preview✨ Features
|
1223d59 to
aeaf331
Compare
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Outdated
Show resolved
Hide resolved
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Outdated
Show resolved
Hide resolved
...e-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingExtension.java
Outdated
Show resolved
Hide resolved
...e-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingExtension.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Outdated
Show resolved
Hide resolved
...e-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingExtension.java
Outdated
Show resolved
Hide resolved
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Show resolved
Hide resolved
...e-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingExtension.java
Outdated
Show resolved
Hide resolved
.../src/test/groovy/com/palantir/gradle/plugintesting/PluginTestingPluginIntegrationSpec.groovy
Outdated
Show resolved
Hide resolved
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Show resolved
Hide resolved
fc9452d to
df298d9
Compare
| gradleTestUtils { | ||
| gradleVersions = ["${version}"] | ||
| } |
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.
Looking at this test, it makes no sense. There aren't any tests for the build under test to run.
Your changes also don't do anything:
- For every version in
version, it doesn't run the build under test with the parameterised gradle version, it runs the build under test with the default version of gradle. Then any tests that are run within the build under test, if they use the@GradlePluginTestingannotation, then those tests will run with different gradle versions. But there are no tests to run.
I would be surprised if this test actually did anything i.e. if the task was just NO-SOURCES.
I would probably delete the test, or set it up properly, and in any case, I don't think the change you're making here is necessary
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.
Hmm, the test does check the plugin logic run during configuration time for CC violations.
But no tasks are run, so the logic in test.doFirst isn't tested. I'll fix this in a FLUP PR?
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.
sounds good
.../src/test/groovy/com/palantir/gradle/plugintesting/PluginTestingPluginIntegrationSpec.groovy
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
| version << GradleTestVersions.gradleVersionsForTests | ||
| } | ||
|
|
||
| def 'use the test driver Gradle version when a version is not set: #version'() { |
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.
this is a much better example of what the ConfigurationCacheTest could be doing
| version << GradleTestVersions.gradleVersionsForTests | ||
| } | ||
|
|
||
| def 'use the test driver Gradle version when a version is not set: #version'() { |
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 you also need a test that the file does what it says i.e. can you run the build under test on a single gradle version, but set it up that the config file runs two versions. That a test you've written runs against two versions.
There's nothing here in this PR that does that. You've got deserialisation logic testing and that's it, there's nothing that indicates that you've wired things up correctly.
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.
Good idea, I've aded this test in PluginTestingPluginIntegrationSpec:
def 'use Gradle versions from both config file and extension'() {
given: 'gradle/gradle-test-versions.yml exists, and sets both major and extra versions'
...
and: 'gradleVersions is set in build script'
...
when: 'running GradlePluginTests'
...
then: 'test runs with versions from both config file and extension'
result.success
result.standardOutput.contains("Running with Gradle version: 8.14.2")
result.standardOutput.contains("Running with Gradle version: 8.8")
result.standardOutput.contains("Running with Gradle version: 7.6")
}
...plugin-testing/src/main/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfig.java
Outdated
Show resolved
Hide resolved
...e-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingExtension.java
Outdated
Show resolved
Hide resolved
.../src/test/groovy/com/palantir/gradle/plugintesting/PluginTestingPluginIntegrationSpec.groovy
Outdated
Show resolved
Hide resolved
...in-testing/src/test/java/com/palantir/gradle/plugintesting/GradleTestVersionsConfigTest.java
Outdated
Show resolved
Hide resolved
gradle-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingPlugin.java
Show resolved
Hide resolved
gradle-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingPlugin.java
Outdated
Show resolved
Hide resolved
gradle-plugin-testing/src/main/java/com/palantir/gradle/plugintesting/PluginTestingPlugin.java
Show resolved
Hide resolved
…testing/PluginTestingExtension.java Co-authored-by: Felix de Souza <[email protected]>
…in-testing into okelvin/yaml-config
|
👍 |
|
Released 0.38.0 |
|
FLUPS:
|
Before this PR
No way to excavate new Gradle versions for testing easily
After this PR
Introduce a new file
gradle/gradle-test-versions.yml:Add a section to the
READMEdetailing how gradleVersions is set. Refer to the README for details on how the config file is used.==COMMIT_MSG==
Create the gradle/gradle-test-versions.yml file
==COMMIT_MSG==
Possible downsides?