Add a JDK11 and JDK12 build to the Travis matrix #1953
Conversation
|
Ah, right. sbt. |
|
🙃 |
|
Sure enough we're failing on JDK11! I'm taking a look. |
Given Oracle's recent licensing changes, we probably want to switch to using OpenJDK since a *lot* of folks will be switching to using OpenJDK
When running in jdk11, the lines method returns a java Stream. linesIterator appears to return the Scala iterator we expect
|
Couldn't repro the mongodb record failure locally so I'm guessing that was transient. Fixed the compile issues in webkit. |
We're experiencing a stack overflow in Travis when running the mongo field spec on JDK11. It seems like it's a specs2 issue so I'm going to attempt a version bump to see if that fixes the issue.
|
Holy crap the build is green |
farmdawgnation
changed the title
|
Ok, JDK 12 is now also in the mix. JDK 13 is fresh off the presses but we'll give it a bit before we try that out. |
| sudo: false | ||
|
|
||
| scala: | ||
| - 2.11.11 |
There was a problem hiding this comment.
Looks like 2.11.12 is a thing?
There was a problem hiding this comment.
I'm happy to version bump that later but this has been languishing awhile, let's not hold it up.
| @@ -153,7 +153,7 @@ class HtmlNormalizerSpec extends Specification with XmlMatchers with Mockito { | |||
|
|
|||
| js.toJsCmd must be matching("""(?s)\Qlift.onEvent("lift-event-js-\E[^"]+\Q","event",function(event) {doStuff;}); | |||
| |lift.onEvent("hello","event",function(event) {doStuff2;}); | |||
| |lift.onEvent("lift-event-js-\E[^"]+\Q","event",function(event) {doStuff3;});\E""".stripMargin('|').lines.mkString("\n").r | |||
| |lift.onEvent("lift-event-js-\E[^"]+\Q","event",function(event) {doStuff3;});\E""".stripMargin('|').linesIterator.mkString("\n").r | |||
There was a problem hiding this comment.
This turns out to be because of this guy: scala/bug#11125
A note from that issue:
note that it isn't safe to rely on the compiler to catch bad uses of .lines, you want to search your source code and find all of them
yes, often the compiler will catch it, e.g. s.lines.toVector fails to compile because Java's Stream doesn't have toVector
but more devious is something like s.lines.filter(...) == .... Java's Stream has filter, and == isn't type-safe, so if you're lucky you'll get a "are unrelated: they will most likely never compare equal" warning ... if you're lucky... and if you're lucky enough to spot the warning in your code's big pile of warnings it already generates that you tune out now...
I guess a Scalafix rewrite could help here, and wouldn't have false positives if you are using .lines method on something other than a String
There was a problem hiding this comment.
Oh I just realized this probably means we've got to hunt all uses of lines down. ffffff
There was a problem hiding this comment.
It feels like I can pick this particular issue up in the 2.13 PR, since I think that'll catch issues? Not sure.
| scalaVersion in ThisBuild := "2.12.6" | ||
| crossScalaVersions in ThisBuild := Seq("2.12.6", "2.11.11") | ||
| scalaVersion in ThisBuild := "2.12.8" | ||
| crossScalaVersions in ThisBuild := Seq("2.12.8", "2.11.11") |
There was a problem hiding this comment.
I'm happy to version bump that later but this has been languishing awhile, let's not hold it up.
Given that JDK 11 is the new LTS release of the JDK, we should ensure our tests pass in it. The runtime bytecode is guaranteed to be compatible, but there are some changes to the default classpath that could affect our behavior.