Run recent JDK tests on JDK 21 #834
Conversation
Added test for null case (covering uber#831) and changed JDK 17 to JDK 21 for unit testing
Changed module name
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #834 +/- ##
=========================================
Coverage 86.75% 86.75%
Complexity 1870 1870
=========================================
Files 74 74
Lines 6178 6178
Branches 1202 1202
=========================================
Hits 5360 5360
Misses 407 407
Partials 411 411 ☔ View full report in Codecov by Sentry. |
| @@ -174,4 +175,30 @@ public void testSwitchExprLambda() { | |||
| "}") | |||
| .doTest(); | |||
| } | |||
|
|
|||
| @Ignore | |||
There was a problem hiding this comment.
Please add a comment indicating we are ignoring because of a crash in the dataflow library
| " a,", | ||
| " b,", | ||
| " }", | ||
| " // NOTE: we should report a bug here for nullableEnum but cannot do so until", |
There was a problem hiding this comment.
Actually thinking more this is not true; if there is case null then there is no NPE. Can you confirm?
There was a problem hiding this comment.
case null is crashing the code right now, so I can't really check. But without case null it should report a NPE, which it doesn't. I could probably have two switch statements one with null and one without and then get this documented.
There was a problem hiding this comment.
I'm saying that NullAway should not report an NPE here, because there will be no NPE if case null is present. See here. You only get an NPE if there is no case null case.
There was a problem hiding this comment.
That's correct, I misunderstood your comment. I added the comment for NullAway not reporting without null, as we discussed previously, but in this context i can either omit it entirely or mention that it only applies when null case isn't defined.
There was a problem hiding this comment.
Yeah, I misunderstood before. I went ahead and deleted the comment.
We need some version of JDK 21 installed now (after #834) for the snapshot job to succeed, even though it's not used. Take the opportunity to simplify other usage of `setup-java`.
Added test for null case in switch statements (covering #831) and changed JDK 17 to JDK 21 for testing recent language features