Fix tests under Java 17 (Upgrade to Play 2.9 & update Guice)#94
Fix tests under Java 17 (Upgrade to Play 2.9 & update Guice)#94rhystmills merged 1 commit intomainfrom
Conversation
| organization := "com.gu", | ||
| scalaVersion := "2.12.18", | ||
| crossScalaVersions := Seq(scalaVersion.value, "2.13.12"), | ||
| scalaVersion := "2.13.12", |
There was a problem hiding this comment.
Dropping support for Scala 2.12, as it's no longer supported by Play, as of Play v2.9.
| "com.typesafe.play" %% "play" % playVersion, | ||
| "com.gu" %% "content-atom-model" % contentAtomVersion, | ||
| "org.scalatestplus.play" %% "scalatestplus-play" % "5.1.0" % Test, | ||
| "org.scalatestplus.play" %% "scalatestplus-play" % "6.0.1" % Test, |
There was a problem hiding this comment.
This is the recommended version of scalatestplus-play for Play 2.9:
48940ac to
422b7de
Compare
After #93 we found that unfortunately the Release workflow failed while running the tests: * The Release workflow uses Java 17 for all builds * The Atom Maker library uses Play with Guice for Dependency Injection * Guice gained Java 17 & 21 support with Guice v6: https://github.com/google/guice/wiki/Guice600 * Play only updated to Guice v6 (and gained general Java 17 support) with Play v2.9: playframework/playframework#11808 playframework/playframework@10ca54d#diff-3dc52110c1c1c453c2e9740ac58fe7e90d53121875a034ef3109c34ab030c29e
422b7de to
b4d55b3
Compare
|
@rtyley has published a preview version of this PR with release workflow run #8, based on commit b4d55b3: 2.0.0-PREVIEW.fix-tests-under-java-17.2024-01-18T1039.b4d55b3d Want to make another preview release?Click 'Run workflow' in the GitHub UI, specifying the fix-tests-under-java-17 branch, or use the GitHub CLI command: gh workflow run release.yml --ref fix-tests-under-java-17 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 |
These upgrades were prompted by guardian/atom-maker#94, but they're all a good idea anyway: * Play: 2.8 → 2.9 * Scala: 2.12 → 2.13 * Java: 8 → 11
|
The updated libraries have been released as 2.0.0, and this time, as desired, the tests did not fail during the release workflow: https://github.com/guardian/atom-maker/actions/runs/7899574542 |
This allows projects to specify, with an `asdf` (https://asdf-vm.com/)-formatted `.tool-versions` file, the Java version to be used by the workflow for building the project- `gha-scala-library-release-workflow` has always used a recent LTS version Java for the build (eg Java 17), and this has sometimes been _too recent_ (guardian/atom-maker#94) for some projects at the Guardian. We _want_ projects to update to Java 21 (see guardian/support-frontend#5792, guardian/scala-steward-public-repos#67 etc), but tying dozens of projects tightly to what `gha-scala-library-release-workflow` is using will make updating that version pretty hard. If, at some later point, _some_ projects want to experiment with Java 25, we shouldn't have to force all other projects to be compatible with that first. ## How to express the version of Java required... ### Configuration proliferation is a problem... One option would have been to simply add a new input parameter to the workflow, specifying the required Java version, but that has a downside: it proliferates the number of places in a project where the desired Java version is declared - and there are many in use at the Guardian, with no well-agreed canonical source-of-truth: * GHA `ci.yml`, in the [`java-version`](https://github.com/guardian/etag-caching/blob/7ecc04981f5a42a0f2ecb10631f28da571a49836/.github/workflows/ci.yml#L22) field of `actions/setup-java` - this has been my favourite in the past, as whatever CI runs with is usually pretty close to the truth * In sbt, the `scalacOptions` of `-target`, `-release`, `-java-output-version` (currently `-release` preferred, tho' once [support](scala/scala#10654) is pervasive, `-java-output-version` is probably best) and the `javacOptions` of `-target` & `-source` - these all effectively set lower-bounds on the version of Java supported, rather than guaranteeing a minimum upper bound of Java version the way CI does. * In apps running on Amigo AMI images; the Java version baked into the AMI, usually [referenced](https://github.com/guardian/mobile-apps-api/blob/3231e6bf064163c6d0e72c8dc862678c68eb0b62/mobile-fronts/conf/riff-raff.yaml#L10) by `riff-raff.yaml` * In AWS Lambdas; the cloudformation [`Runtime`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-function.html#cfn-lambda-function-runtime) parameter, often set [by CDK](https://github.com/guardian/mobile-save-for-later/blob/1ac12e4c0100edb976ebae9e2a9975ad2321e26e/cdk/lib/mobile-save-for-later.ts#L44) ### ...`asdf`'s `.tool-versions` flle offers some consolidation Benefits of using `.tool-versions`: * Developers will automatically get the right Java version if they have `asdf` installed * actions/setup-java#606 has added early support for reading the version of Java used in CI by `setup-java` from the `.tool-versions` flle - unfortunately, it's of limited use to us at the moment because of actions/setup-java#615, but it's a promising start _(`setup-java` also has support for `jEnv` files, but they are not commonly used at the Guardian)_ * Format of the file is simple enough that it can be easily understood and used, even if `asdf` is not in use - **including the file doesn't _force_ developers to use `asdf`** (I know some devs have been having some problems with it, and maybe there are/will be better alternatives out there), but it clearly documents the Java version to be used. This does mean that **all library repos need to add a `.tool-versions` file** before this PR is merged. Helpful error messaging has been added with this PR to handle cases where the file is missing or invalid: #### Only Java _major_ version is guaranteed Note that although `asdf` requires a fully-specified Java version (eg `21.0.3.9.1`) in the `.tool-versions` file, currently the workflow will only match the *major* version of Java specified in the file (eg `21`), and will _always_ use the AWS Corretto distribution of Java. This is due to [limitations](actions/setup-java#615) in [`actions/setup-java`](https://github.com/actions/setup-java).

What's changing?
Why?
After PR #93, installing
gha-scala-library-release-workflow, we found that unfortunately the release workflow failed with anInaccessibleObjectExceptionwhile running the tests (which in this project's normal CI used only Java 8 & 11 up till now):How to test?
The new
gha-scala-library-release-workflowadded in #93 made it easy to publish a pre-release version of this PR (see #94 (comment) - even though release of main branch currently isn't working due to this issue, the preview release of this branch fixing the problem does work!):This has been slotted into guardian/atom-workshop#349, alongside an upgrade to Play 2.9 & Scala 2.13 there, to check that this all works.
Note that the automated version-compatibility testing has also correctly decided that this change requires a 'major' version bump, from 1.4.0 to 2.0.0!
Consumers affected by this update
Code search for
atom-publisher-lib&atom-manager-playconfirms that at the Guardian we have only 4 applications using the libraries published by this repo, of which 2 are archived: