Drop Scala 2.12 and Play 2.7, begin partial support for Scala 3 #318
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JSON objects have unordered keys, so there's no 'correct' order for the fields to appear in when the JSON is converted into a String. This means it's a bad idea to write tests that assert that JSON should match a specific string - because the string may change with new versions of the underlying software libraries, eg with changes to Java or Scala versions. This particular 'should serialize territories' test in `facia-scala-client` works at the moment, but spontaneously fails under Scala 3 with the changes in #287 : https://github.com/guardian/facia-scala-client/actions/runs/9894534871/job/27332773020#step:5:2147 ``` [info] - should serialize territories *** FAILED *** [info] "...nt-content=true"},"t[argetedTerritory":"NZ","uneditable":true,"type":"news/most-popular]"}" was not equal to "...nt-content=true"},"t[ype":"news/most-popular","uneditable":true,"targetedTerritory":"NZ]"}" (ConfigSpec.scala:58) ``` A quick fix is to not make an assertion against a _string_, but to make an assertion against the JSON _type_ (in our case, `JsValue`) which has proper equality rules, and does not mind about _ordering_ of the fields.
The version of the mocking library Mockito that we're currently using does not support Java 21, and so the tests currently fail under Java 21 with `java.lang.NoClassDefFoundError` ``` [info] com.gu.facia.api.contentapi.ContentApiTest *** ABORTED *** [info] java.lang.NoClassDefFoundError: Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3 [info] at org.mockito.internal.creation.cglib.ClassImposterizer.createProxyClass(ClassImposterizer.java:95) [info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:57) [info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:49) [info] at org.mockito.internal.creation.cglib.CglibMockMaker.createMock(CglibMockMaker.java:24) [info] at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:33) [info] at org.mockito.internal.MockitoCore.mock(MockitoCore.java:59) [info] at org.mockito.Mockito.mock(Mockito.java:1285) [info] at org.mockito.Mockito.mock(Mockito.java:1163) [info] at org.scalatestplus.mockito.MockitoSugar.mock(MockitoSugar.scala:73) [info] at org.scalatestplus.mockito.MockitoSugar.mock$(MockitoSugar.scala:72) [info] ... [info] Cause: java.lang.ExceptionInInitializerError: Exception java.lang.ExceptionInInitializerError [in thread "pool-7-thread-11"] [info] at org.mockito.cglib.core.KeyFactory$Generator.generateClass(KeyFactory.java:167) [info] at org.mockito.cglib.core.DefaultGeneratorStrategy.generate(DefaultGeneratorStrategy.java:25) [info] at org.mockito.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:217) [info] at org.mockito.cglib.core.KeyFactory$Generator.create(KeyFactory.java:145) [info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:117) [info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:109) [info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:105) [info] at org.mockito.cglib.proxy.Enhancer.<clinit>(Enhancer.java:70) [info] at org.mockito.internal.creation.cglib.ClassImposterizer.createProxyClass(ClassImposterizer.java:95) [info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:57) [info] ... ```
rtyley
changed the title
rtyley
force-pushed
the
drop-scala-2.12-and-play-27
branch
from
001eb33
to
7607e05
Compare
rtyley
commented
Jul 24, 2024
| organization := "com.gu", | ||
| resolvers ++= Resolver.sonatypeOssRepos("releases"), | ||
| scalaVersion := "2.13.14", | ||
| crossScalaVersions := Seq(scalaVersion.value) ++ (if (supportScala3) Seq("3.3.3") else Seq.empty), |
There was a problem hiding this comment.
Note that Scala 2.12 is no longer listed here, and for artifacts that can support it (ie the facia-json artifacts) we now compile to Scala 3 as well.
rtyley
force-pushed
the
drop-scala-2.12-and-play-27
branch
from
7607e05
to
2f129bb
Compare
rtyley
commented
Jul 24, 2024
| java corretto-21.0.3.9.1 | ||
| java corretto-11.0.23.9.1 |
There was a problem hiding this comment.
The version of Mockito that we're currently using does not support Java 21, and so the tests currently fail under Java 21 with java.lang.NoClassDefFoundError
[info] com.gu.facia.api.contentapi.ContentApiTest *** ABORTED ***
[info] java.lang.NoClassDefFoundError: Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[info] at org.mockito.internal.creation.cglib.ClassImposterizer.createProxyClass(ClassImposterizer.java:95)
[info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:57)
[info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:49)
[info] at org.mockito.internal.creation.cglib.CglibMockMaker.createMock(CglibMockMaker.java:24)
[info] at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:33)
[info] at org.mockito.internal.MockitoCore.mock(MockitoCore.java:59)
[info] at org.mockito.Mockito.mock(Mockito.java:1285)
[info] at org.mockito.Mockito.mock(Mockito.java:1163)
[info] at org.scalatestplus.mockito.MockitoSugar.mock(MockitoSugar.scala:73)
[info] at org.scalatestplus.mockito.MockitoSugar.mock$(MockitoSugar.scala:72)
[info] ...
[info] Cause: java.lang.ExceptionInInitializerError: Exception java.lang.ExceptionInInitializerError [in thread "pool-7-thread-11"]
[info] at org.mockito.cglib.core.KeyFactory$Generator.generateClass(KeyFactory.java:167)
[info] at org.mockito.cglib.core.DefaultGeneratorStrategy.generate(DefaultGeneratorStrategy.java:25)
[info] at org.mockito.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:217)
[info] at org.mockito.cglib.core.KeyFactory$Generator.create(KeyFactory.java:145)
[info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:117)
[info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:109)
[info] at org.mockito.cglib.core.KeyFactory.create(KeyFactory.java:105)
[info] at org.mockito.cglib.proxy.Enhancer.<clinit>(Enhancer.java:70)
[info] at org.mockito.internal.creation.cglib.ClassImposterizer.createProxyClass(ClassImposterizer.java:95)
[info] at org.mockito.internal.creation.cglib.ClassImposterizer.imposterise(ClassImposterizer.java:57)
[info] ...
|
@rtyley has published a preview version of this PR with release workflow run #23, based on commit 2f129bb: 8.0.1-PREVIEW.drop-scala-212-and-play-27.2024-07-24T1450.2f129bb2 Want to make another preview release?Click 'Run workflow' in the GitHub UI, specifying the drop-scala-2.12-and-play-27 branch, or use the GitHub CLI command: gh workflow run release.yml --ref drop-scala-2.12-and-play-27 Want to make a full release after this PR is merged?Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command: gh workflow run release.yml |
Fixes #317, meaning that PRs like #287 can use Scala 2.13 features. Also dropping Play 2.7 - Play 2.8 was end-of-life (EOL) on 31st May 2024, so dropping support for Play 2.7 seems very reasonable! Running the tests under Scala 3 required commit 3690941, fixing a test so that it doesn't rely on JSON object's field order. All 7 consumers of the facia-scala-client library are now on Scala 2.13, and Play 2.8 or above, so they do not pose blockers to dropping Scala 2.12 and Play 2.7: * guardian/ophan - Play 3.0.4 * guardian/frontend - Play 3.0.4 * guardian/apple-news - Play 3.0.2 * guardian/editors-picks-uploader - AWS Lambda, doesn't use Play * guardian/facia-tool - Play 3.0.2 * guardian/story-packages - Play 2.8.19 * guardian/mobile-apps-api - Play 3.0.3
rtyley
force-pushed
the
drop-scala-2.12-and-play-27
branch
from
2f129bb
to
3aa8aa1
Compare
|
@rtyley has published a preview version of this PR with release workflow run #24, based on commit 3aa8aa1: 8.0.1-PREVIEW.drop-scala-212-and-play-27.2024-07-24T1515.3aa8aa15 Want to make another preview release?Click 'Run workflow' in the GitHub UI, specifying the drop-scala-2.12-and-play-27 branch, or use the GitHub CLI command: gh workflow run release.yml --ref drop-scala-2.12-and-play-27 Want to make a full release after this PR is merged?Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command: gh workflow run release.yml |
JamieB-gu
approved these changes
Jul 24, 2024
| scalaVersion := "2.13.14", | ||
| crossScalaVersions := Seq(scalaVersion.value) ++ (if (supportScala3) Seq("3.3.3") else Seq.empty), | ||
| scalacOptions := Seq( | ||
| "-release:8", |
There was a problem hiding this comment.
release:11 has become release:8?
There was a problem hiding this comment.
Yep, I'm afraid so- guardian/story-packages is still running on Java 8:
rtyley
added a commit
that referenced
this pull request
Aug 2, 2024
After #318 dropped support for Scala 2.12, release 8.0.1 correctly no longer published Scala 2.12 artifacts, but unfortunately the 'root' project (which does not have any code!) was still defaulting to using Scala 2.12, because that is the default sbt uses if no `scalaVersion` is set. The next time we tried to do a release (#319 (comment)) the automated-compatibility checking failed, because it saw that one project was using Scala 2.12, and so it attempted to download all the non-existent Scala 2.12 artifacts for release 8.0.1 - and failed. To fix this, we can simply ensure that all modules in this sbt build default to Scala 2.13, not 2.12 - ie particularly, the 'root' module!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #317, meaning that PRs like #287 can use Scala 2.13 features. Also dropping Play 2.7- Play 2.8 was end-of-life (EOL) on 31st May 2024, so dropping support for Play 2.7 seems very reasonable!
Running the tests under Scala 3 required 3690941, fixing a test so that it doesn't rely on JSON object's field order.
Consumers of
facia-scala-clientAll 7 consumers of the
facia-scala-clientlibrary are now on Scala 2.13 (last one to upgrade was guardian/story-packages#195!), and Play 2.8 or above, so they do not pose blockers to dropping Scala 2.12 and Play 2.7: