Skip to content
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

Ensure spock 2/nebula-test 10 tests actually run #1929

Merged
merged 8 commits into from
Oct 1, 2021

Conversation

CRogers
Copy link
Contributor

@CRogers CRogers commented Sep 30, 2021

Before this PR

Nebula test 10 upgraded Spock to version 2, which is required for Gradle 7/Groovy 3 support.

Spock 2 has a breaking change that means it no longer runs using JUnit 4, but instead uses the JUnit Platform. In Gradle, you need to opt into the JUnit Platform by setting some configuration:

test {
    useJUnitPlatform()
}

If you don't do this, your tests will be silently not be run 😢 . For many of our repos that use nebula-test, they got upgraded to 10 automatically and the tests just stopped running.

After this PR

==COMMIT_MSG==
Ensure nebula-test 10/Spock 2 test run by automatically setting useJUnitPlatform() on test tasks. Add extra verification to the checkJUnitDependencies task for nebula-test 10/Spock 2 tests.
==COMMIT_MSG==

Possible downsides?

@CRogers CRogers requested a review from fawind September 30, 2021 14:21
@changelog-app
Copy link

changelog-app bot commented Sep 30, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Ensure nebula-test 10/Spock 2 test run by automatically setting useJUnitPlatform() on test tasks. Add extra verification to the checkJUnitDependencies task for nebula-test 10/Spock 2 tests.

Check the box to generate changelog(s)

  • Generate changelog entry

project.getConfigurations()
.getByName(sourceSet.getRuntimeClasspathConfigurationName())
.getIncoming()
.afterResolve(deps -> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we need to distinguish between Spock 1 and Spock 2, we need to actually resolve the configuration to get a version number. It's actually quite tricky to resolve here, as it fails foul of the checks we have to prevent resolving configuration at configuration time, which destroys perf.

Instead, we wait for this configuration to resolved, which will happen before the test is run, by the checkJUnitDependencies task if nothing else...

(This would be so much easier if useJUnitPlatform was a lazy property)

Copy link
Contributor

Choose a reason for hiding this comment

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

I was initially thinking that it might be sufficient to just go through the explicitly declared deps which would not require resolution. However most of the time Spock is brought in as a transitive and so you have to do resolution :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially did that and checked for nebula-test too, but until resolution happens you don't know the version number, so can't decide whether or not to do the right thing

@CRogers CRogers marked this pull request as ready for review September 30, 2021 17:16
@policy-bot policy-bot bot requested a review from tpetracca September 30, 2021 17:16
import spock.lang.Unroll

@Unroll
class BaselineTestingIntegrationTest extends AbstractPluginTest {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fun fact, this homemade AbstractPluginTest framework from 2017 wasted nearly 2 hours of my time because it doesn't run changed code in the test from IntelliJ unless you first recompile the code through gradle 😱 😱 😱 😱 😱 😱 😱

Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

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

Scary stuff but looks good, just one smaller comment.

project.getTasks().register("checkJUnitDependencies", CheckJUnitDependencies.class);

project.getConvention()
.getPlugin(JavaPluginConvention.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the JavaPluginExtension as conventions are deprecated and to be removed with Gradle 8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but Gradle 6 doesn't have the getSourceSets method we need 118cb97

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I remember encountering this before. I guess Gradle 7 is the bridge version then at with Gradle 8 we have to cut support for Gradle 6. Fine for now then!

@bulldozer-bot bulldozer-bot bot merged commit b08bf40 into develop Oct 1, 2021
@bulldozer-bot bulldozer-bot bot deleted the callumr/ensure-nebula-spock-tests-run branch October 1, 2021 15:36
@svc-autorelease
Copy link
Collaborator

Released 4.27.0

bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Oct 1, 2021
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 4.26.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | CheckExplicitSourceCompatibilityTask accepts toolchains as a mechanism to specify a release version | palantir/gradle-baseline#1915 |


## 4.27.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Ensure nebula-test 10/Spock 2 test run by automatically setting `useJUnitPlatform()` on test tasks. Add extra verification to the `checkJUnitDependencies` task for nebula-test 10/Spock 2 tests. | palantir/gradle-baseline#1929 |



To enable or disable this check, please contact the maintainers of Excavator.
This was referenced Oct 1, 2021
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.

None yet

4 participants