-
Notifications
You must be signed in to change notification settings - Fork 29
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
Update dependencies and improve build tool testing infrastructure, add JDK 21 #692
Conversation
We introduce the concept of Tool, which has restrictions on minimum/maximum JDK version it can run under. We also use external JDK version to conditionally ignore tests.
@@ -14,10 +14,10 @@ jobs: | |||
# NOTE(olafurpg) Windows is not enabled because it times out due to reasons I don't understand. | |||
# os: [windows-latest, ubuntu-latest] | |||
os: [ubuntu-latest] | |||
java: [8, 11, 17] | |||
java: [8, 11, 17, 21] |
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.
Add JDK 21 to the build matrix - that's what triggered the rest of the changes.
build.sbt
Outdated
val coursier = "2.1.9" | ||
val scalaXml = "2.1.0" | ||
val bsp = "2.0.0-M13" | ||
val moped = "0.1.11" | ||
val moped = "0.1.11+2-c9893ac3-SNAPSHOT" | ||
val gradle = "7.0" | ||
val scala213 = "2.13.10" | ||
val scala212 = "2.12.17" | ||
val scala213 = "2.13.13" | ||
val scala212 = "2.12.19" | ||
val scala211 = "2.11.12" | ||
val scala3 = "3.2.2" | ||
val metals = "0.11.11" | ||
val scalameta = "4.8.1" | ||
val scala3 = "3.3.3" | ||
val metals = "1.2.2" | ||
val scalameta = "4.9.3" | ||
val semanticdbKotlinc = "0.4.0" | ||
val testcontainers = "0.39.3" | ||
val requests = "0.6.5" | ||
val requests = "0.8.0" |
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.
Various dependency upgrades
lazy val minimized21 = project | ||
.in(file("tests/minimized/.j21")) | ||
.settings( | ||
javaOnlySettings, | ||
minimizedSettings, | ||
javaToolchainVersion := "21", | ||
javacOptions ++= javacModuleOptions | ||
) | ||
.dependsOn(agent, javacPlugin) | ||
.disablePlugins(JavaFormatterPlugin) |
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.
Make sure our test java sources are compiled under java 21 as well
scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ScipBuildTool.scala
Show resolved
Hide resolved
private val PrintJavaVersion = """ | ||
public class PrintJavaVersion { | ||
public static void main(String[] args) { | ||
System.out.print(System.getProperty("java.version")); | ||
} | ||
} |
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.
This is a more reliable way of getting the version out of Java, because parsing java -version
is more difficult due to variability in how that information is presented depending on version and vendor.
Logic and mechanism were lifted from sourcegraph/sbt-sourcegraph#78
|
||
version | ||
.map(parseJavaVersion) | ||
.getOrElse(sys.error("Failed to detect external JDK version")) |
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.
It's fine to hard die here, because an external JVM has to be available for all the build tool tests (gradle, maven, sbt won't start without it)
extraArguments: List[String] = Nil | ||
extraArguments: List[String] = Nil, | ||
gradleVersions: List[Tool.Gradle] = Nil, | ||
tools: List[Tool] = Nil |
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.
This extra argument allows further specifying compilers/buildtools/etc. that are used as part of the test, which can affect the success under particular JDK.
E.g. if a Gradle build can run under JDK 21, but uses a Scala 2.12.12 compiler, it won't be able to complete because JDK 21 support requires at least Scala 2.12.18
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 👍🏻
Summary:
Tool
- which adds the ability to add JDK version ranges, so that the tests are ignored automatically on JDK versions that the tools don't supportTest plan