search

twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 406 forks source link

root: Upgrade test library versions in preparation for 2.13 support #527

Closed chrisbenincasa closed 4 years ago

chrisbenincasa commented 4 years ago

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.

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

chrisbenincasa commented 4 years ago

JVM 11 failure in the build seems like https://github.com/twitter/finagle/issues/832

cacoco commented 4 years ago

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?

@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.

chrisbenincasa commented 4 years ago

Reverted the Mockito change based on @yufangong's comment since it's not strictly blocking for Scala 2.13.

yufangong commented 4 years ago

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!

chrisbenincasa commented 4 years ago

Hey @yufangong, any updates?

yufangong commented 4 years ago

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.

mosesn commented 4 years ago

@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.

cacoco commented 4 years ago

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).

chrisbenincasa commented 4 years ago

Good news! Thanks for the update.

yufangong commented 4 years ago

@chrisbenincasa good news, I believe we are unblocked! @mosesn and @cacoco made this happen. The test dependencies are covered at https://github.com/twitter/finatra/commit/fbb7b5357f32f15b86c7a70f16804411c17bef7f, so we can probably close this PR. And when you get a chance, do you mind rebasing other PRs on the newest code?

chrisbenincasa commented 4 years ago

@yufangong that's great! I've rebased all of the other PRs off of this one and straight onto develop.