-
Notifications
You must be signed in to change notification settings - Fork 241
Scala native 0.5 #1363
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
Scala native 0.5 #1363
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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