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

Upgrade mockito now that the minimum Java is 11 on CI #247

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,7 @@ dependencies {
testImplementation("com.github.tomakehurst:wiremock:2.27.2")
testImplementation("ru.lanwen.wiremock:wiremock-junit5:1.3.1")
testImplementation("org.assertj:assertj-core:3.24.2")
// This cannot be updated to 5.x as it requires Java 11,
// but we are running CI on Java 8 in .github/workflows/java-versions.yml.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(btw, shouldn't Java 8 CI still exist considering the target bytecode is 8?)

Copy link
Contributor

Choose a reason for hiding this comment

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

We've dropped the Java 8 CI builds to speed up the process after #171 is applied. Java <11 is unsupported for building 2.0.0+ (and with that MR merged it will effectively be enforced), so I'm not sure, if we need it. We probably could have some "smoke tests" trying to execute Gradle build with Java 8 for the artifacts build and "released" with JDK 17, but it is also not supported, so we might wait for people to complaint about any runtime regression in their projects. WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it would make sense to build using 17, and and target bytecode 8. Best of both worlds and the effort should be really low. Tests can still run on 11 or 17. I just queued up a similar PR in Mockito: TWiStErRob/mockito#1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyway, for now since 8 is in a "dropped" state, I think this PR can go ahead. It should only affect unit tests. I guess compatTests don't use mocks?

testImplementation("org.mockito:mockito-junit-jupiter:4.11.0")
testImplementation("org.mockito:mockito-junit-jupiter:5.4.0")
testImplementation("com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0")
}

Expand Down