root: Upgrade test library versions in preparation for 2.13 support #527
Conversation
|
|
|
JVM 11 failure in the build seems like twitter/finagle#832 |
| @@ -1,6 +1,6 @@ | |||
| package com.twitter.inject | |||
|
|
|||
| import org.mockito.Matchers | |||
| import org.mockito.ArgumentMatchers | |||
There was a problem hiding this comment.
Can you also upgrade the mockito version? Matchers are not deprecated in the version we depend on currently.
There was a problem hiding this comment.
Updated to the latest, 3.3.3.
| @@ -2,7 +2,7 @@ package com.twitter.inject | |||
|
|
|||
| import org.junit.runner.RunWith | |||
| import org.scalatest.FunSuite | |||
| import org.scalatest.junit.JUnitRunner | |||
| import org.scalatestplus.junit.JUnitRunner | |||
There was a problem hiding this comment.
I think we can instead just remove the RunWith and JunitRunner.
Here's a similar approach: twitter/util@ad9ccd0#diff-114c8b055e627e0231beb718ccbd9255. It looks like finatra test all extend this Test, so maybe only this one place.
There was a problem hiding this comment.
I did some test internally and surprised how challenging it could be to upgrade test deps versions. My theory is Finatra provides all these test suites to applications and exports the test deps versions, it could be breaking changes for all the applications. cc @cacoco can you confirm?
Since this is blocking @chrisbenincasa your other PRs, what do you think only updating the ones blocking 213 for now and file an issue for the rest so we can tackle those along the way?
| val scalaTest = "3.0.8" | ||
| val scalaTest = "3.1.1" | ||
| val scalaTestPlusJunit = "1.0.0-M2" | ||
| val scalaTestPlusScalacheck = "3.1.0.0-RC2" |
There was a problem hiding this comment.
the junit one is not necessary anymore, since we removed the RunWith annotation. the scalacheck one is still necessary due to a change between scalatest 3.0 and 3.1 that moved 3rd party integrations to separate packages.
| @@ -137,6 +139,8 @@ lazy val baseSettings = Seq( | |||
| "org.mockito" % "mockito-core" % versions.mockito % Test, | |||
| "org.scalacheck" %% "scalacheck" % versions.scalaCheck % Test, | |||
| "org.scalatest" %% "scalatest" % versions.scalaTest % Test, | |||
| "org.scalatestplus" %% "scalatestplus-junit" % versions.scalaTestPlusJunit % Test, | |||
| "org.scalatestplus" %% "scalacheck-1-14" % versions.scalaTestPlusScalacheck % Test, | |||
There was a problem hiding this comment.
explained in the comment above - removed the junit one.
@yufangong all of our versions should track what is in the monorepo 3rdparty, trying to diverge will likely not be possible. We should confirm these versions are in sync. |
Problem Several libraries used for testing were using older versions that were not cross-compiled for Scala 2.13. Solution Upgrade the relevant libraries to latest versions supporting Scala 2.13 and add new libraries necessary due to deprecations in the existing libraries. Result Base testing libraries are now compatiable with a Scala 2.13 cross builder. Building for Scala 2.12 and 2.11 still works as expected.
|
Reverted the Mockito change based on @yufangong's comment since it's not strictly blocking for Scala 2.13. |
|
Hi @chrisbenincasa, I apologize for the delay. After some investigation, our assumption is spec2 (mockito) upgrades are a bit challenging at this time, as our internal monorepo is still on an earlier version, Finatra changes will break internal usages. We are currently working on that, I will keep you posted when we can get unblocked. Sorry! |
|
Hey @yufangong, any updates? |
|
Hi, @chrisbenincasa, we now have some folks are actively working on upgrading spec2/scalatest/mockito internally to unblock us here, I don't have an estimation (or, two or three weeks) but feel optimized it should be sorted out soon. |
|
@chrisbenincasa some news! we were finally able to conduct the scalatest update. we're going to start looking at specs next. sorry again for the delay. |
|
We're going to be removing the specs2 dependency soon and will replicate the behavior with mockito-scala. This should be (hopefully) completed in the next few weeks in time for our August release (will likely not make the July release). |
|
Good news! Thanks for the update. |
|
@chrisbenincasa good news, I believe we are unblocked! @mosesn and @cacoco made this happen. |
|
@yufangong that's great! I've rebased all of the other PRs off of this one and straight onto develop. |
Problem
Several libraries used for testing were using older versions that were
not cross-compiled for Scala 2.13.
Solution
Upgrade the relevant libraries to latest versions supporting Scala 2.13
and add new libraries necessary due to deprecations in the existing
libraries.
Result
Base testing libraries are now compatiable with a Scala 2.13 cross
builder. Building for Scala 2.12 and 2.11 still works as expected.