-
Notifications
You must be signed in to change notification settings - Fork 513
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
Add Java 17 to GHA setup #4659
Add Java 17 to GHA setup #4659
Conversation
66781e4
to
083d430
Compare
Codecov Report
@@ Coverage Diff @@
## main #4659 +/- ##
=======================================
Coverage 60.91% 60.91%
=======================================
Files 286 286
Lines 10457 10457
Branches 667 667
=======================================
Hits 6370 6370
Misses 4087 4087 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
dfe615a
to
79a6a8b
Compare
// In this case outer field is called "$cw" and it is hard to wrap it with proper exception | ||
// so we allow it to fail with NullPointerException | ||
e.getMessage should be(null) | ||
case _: NullPointerException => |
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.
Interestingly in Java 17, this now throws an NPE with the message Cannot throw exception because "null" is null
. cc @shnapz @RustedBones
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.
@clairemcginty will take a look. I think it's fine to skip this assertion, it wasn't really useful
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!
@@ -12,11 +12,11 @@ jobs: | |||
fetch-depth: 0 | |||
- name: cache SBT | |||
uses: coursier/cache-action@v6 | |||
- name: Java 11 setup | |||
- name: Java 17 setup |
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.
We can also save this for 0.13 milestone
d17432f
to
3540d04
Compare
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.
LGTM @clairemcginty
(fix #4597) testing GHA setup w/ Java 17.
See linked ticket for more info on why the JVM opts were added to both .jvmopts and to build.sbt: basically, the build.sbt options are ignored for tests where
fork := false
:Instead, opts are picked up from .jvmopts. Conversely, for forked tests, the JVM options don't seem to get passed from .jvmopts to the forked process 🤷♀️ so it seems like we need both to support both forked and non-forked tests.
Another option is just setting fork := true for all tests