Conversation
e663498 to
abd89ae
Compare
Enable tests with JDK 21.
abd89ae to
4e3d7e7
Compare
dd29762 to
7b64378
Compare
Use lower precision for native platform for NRootSuite
7b64378 to
291fb1f
Compare
|
Thanks for the PR! See this other PR that explains why we are struggling to make this upgrade. |
| val value = x // Explicitly define. Avoids failures on certain JDKs | ||
| val closure = () => { capturedValues += value; () } | ||
| b1 += closure |
There was a problem hiding this comment.
Are you sure this is related to JDK version? This is actually due to a change in Scala 3.
There was a problem hiding this comment.
Interesting. It was definitely passing on older JDK's. Unless I am very confused and was running on scala 2.13!
Edit: Sorry, you are correct. I had been running those tests on Scala 2. Hadn't used the +test in that instance
There was a problem hiding this comment.
Anyway I believe this implementation is functionally equivalent no? Replaced the while loop with a cfor loop for b2. Can revert that.
There was a problem hiding this comment.
To clarify, this is test code, not implementation code. You can consider test code to be representing user code. So if we have to change our test code because the behavior of the implementation changed, that means our users will have to change their code as well. That's not good 😅 when users upgrade, they could be inadvertently introducing bugs.
There was a problem hiding this comment.
@armanbilge For CforSuite I was getting similar failures to #1343 on current versions of Java 8 only. Tested with an old JDK 8 and it wasn't an issue. My guess is there is some change in Java 8's closure expansion? After a bunch of trial and error explicitly declaring a local val seems to fix it.
There was a problem hiding this comment.
For NRootSuite getting Munit test timeouts running on CI for Native platform. (Running on local workstation is passing). Munit's test timeout setting cannot be changed for scala-native platform. Reduced the precision for Native platform tests. If this is acceptible?
|
Closing this as a duplicate of #1343 |
Build plugins updates:
Resolves #1342