Migration from specs2 to scalatest (#273)

* Migration from specs2 to scalatest

* Added changelog note

CHANGELOG.md

@@ -6,6 +6,14 @@
 Updated to Play `2.9.0` and dropped `Scala 2.12` since it was discontinued from the Play framework.
 Minimal supported Java version is now `11`. [#265](https://github.com/KarelCemus/play-redis/pull/265)
 
+Migrated from test framework `specs2` to `scalatest` because specs2 is not friendly to
+cross-compilation between Scala 2.13 and Scala 3. Test are uses `testcontainers` to run
+standalone, cluster or sentinel instance. However, current redis connector is not friendly
+and there are several significant limitations in the implementation, therefore the tests
+for cluster are using fixed port mapping and tests for sentinel are disabled since it seems
+that sentinel implementation is not fully reliable, therefore sentinel is not officially
+supported at this moment. [#273](https://github.com/KarelCemus/play-redis/pull/273)
+
 ### [:link: 2.7.0](https://github.com/KarelCemus/play-redis/tree/2.7.0)
 
 SET command supports milliseconds, previous versions used seconds [#247](https://github.com/KarelCemus/play-redis/issues/247)
@@ -30,17 +38,17 @@ Note: This should not be a breaking change since it was not possible to properly
 the value in Java without encountering the exception.
 
 Introduced another source `aws-cluster`, which is a cluster with nodes defined by DNS record. For example,
-Amazon AWS uses this kind of cluster definition. Therefore this type of a cluster resolves 
+Amazon AWS uses this kind of cluster definition. Therefore this type of a cluster resolves
 a domain main to extract all nodes. See [#221](https://github.com/KarelCemus/play-redis/pull/221) for more details.
 
 ### [:link: 2.5.0](https://github.com/KarelCemus/play-redis/tree/2.5.0)
 
-Added `expiresIn(key: String): Option[Duration]` implementing PTTL 
+Added `expiresIn(key: String): Option[Duration]` implementing PTTL
 command to get expiration of the given key. [#204](https://github.com/KarelCemus/play-redis/pull/204)
 
 Introduced asynchronous implementation of advanced Java API for redis cache. The API
 wraps the Scala version thus provides slightly worse performance and deals with
-the lack of `classTag` in Play's API design. **This API implementation is experimental 
+the lack of `classTag` in Play's API design. **This API implementation is experimental
 and may change in future.** Feedback will be welcome. [#206](https://github.com/KarelCemus/play-redis/issues/206)
 
 Added `getFields(fields: String*)` and `getFields(fields: Iterable[String])` into `RedisMap` API
@@ -55,7 +63,7 @@ Cross-compiled with Scala 2.13 [#211](https://github.com/KarelCemus/play-redis/i
 
 Update to Play `2.7.0` [#202](https://github.com/KarelCemus/play-redis/pull/202)
 
-Added `getAll[T: ClassTag](keys: Iterable[String]): Result[Seq[Option[T]]]` into `AbstractCacheApi` 
+Added `getAll[T: ClassTag](keys: Iterable[String]): Result[Seq[Option[T]]]` into `AbstractCacheApi`
 in order to also accept collections aside vararg. [#194](https://github.com/KarelCemus/play-redis/pull/194)
 
 Fixed `getOrElse` method in Synchronous API with non-empty cache prefix. [#196](https://github.com/KarelCemus/play-redis/pull/196)
@@ -71,7 +79,7 @@ Returned keys are automatically unprefixed. [#184](https://github.com/KarelCemus
 
 Support of plain arrays in JavaRedis [#176](https://github.com/KarelCemus/play-redis/pull/176).
 
-Connection timeout introduced in [#147](https://github.com/KarelCemus/play-redis/issues/147) 
+Connection timeout introduced in [#147](https://github.com/KarelCemus/play-redis/issues/147)
 is now configurable and can be disabled [#174](https://github.com/KarelCemus/play-redis/pull/174).
 
 Removed deprecations introduced in [2.0.0](https://github.com/KarelCemus/play-redis/tree/2.0.0)

build.sbt

@@ -21,11 +21,12 @@ libraryDependencies ++= Seq(
   // redis connector
   "io.github.rediscala" %% "rediscala" % "1.14.0-akka",
   // test framework with mockito extension
-  "org.specs2" %% "specs2-mock" % "4.20.3" % Test,
+  "org.scalatest" %% "scalatest" % "3.2.17" % Test,
+  "org.scalamock" %% "scalamock" % "5.2.0" % Test,
   // test module for play framework
   "com.typesafe.play" %% "play-test" % playVersion.value % Test,
   // to run integration tests
-  "com.dimafeng" %% "testcontainers-scala-core" % "0.41.0" % Test
+  "com.dimafeng" %% "testcontainers-scala-core" % "0.41.2" % Test
 )
 
 resolvers ++= Seq(
@@ -40,3 +41,5 @@ enablePlugins(CustomReleasePlugin)
 
 // exclude from tests coverage
 coverageExcludedFiles := ".*exceptions.*"
+
+Test / test := (Test / testOnly).toTask(" * -- -l \"org.scalatest.Ignore\"").value

src/main/scala/play/api/cache/redis/CacheApi.scala

@@ -27,7 +27,7 @@ private[redis] trait AbstractCacheApi[Result[_]] {
     * @param key cache storage keys
     * @return stored record, Some if exists, otherwise None
     */
-  def getAll[T: ClassTag](key: String*): Result[Seq[Option[T]]] = getAll(key)
+  final def getAll[T: ClassTag](key: String*): Result[Seq[Option[T]]] = getAll(key)
 
   /**
     * Retrieve the values of all specified keys from the cache.

src/main/scala/play/api/cache/redis/configuration/RedisHost.scala

@@ -46,6 +46,7 @@ object RedisHost extends ConfigLoader[RedisHost] {
     host = config.getString(path / "host"),
     port = config.getInt(path / "port"),
     database = config.getOption(path / "database", _.getInt),
+    username = config.getOption(path / "username", _.getString),
     password = config.getOption(path / "password", _.getString)
   )
 

src/main/scala/play/api/cache/redis/configuration/RedisInstance.scala

@@ -71,7 +71,7 @@ trait RedisStandalone extends RedisInstance with RedisHost {
 
 object RedisStandalone {
 
-  def apply(name: String, host: RedisHost, settings: RedisSettings) =
+  def apply(name: String, host: RedisHost, settings: RedisSettings): RedisStandalone =
     create(name, host, settings)
 
   @inline

src/main/scala/play/api/cache/redis/connector/RedisCommands.scala

@@ -1,13 +1,10 @@
 package play.api.cache.redis.connector
 
 import javax.inject._
-
 import scala.concurrent.Future
-
 import play.api.Logger
 import play.api.cache.redis.configuration._
 import play.api.inject.ApplicationLifecycle
-
 import akka.actor.ActorSystem
 import redis.{RedisClient => RedisStandaloneClient, RedisCluster => RedisClusterClient, _}
 
@@ -17,7 +14,7 @@ import redis.{RedisClient => RedisStandaloneClient, RedisCluster => RedisCluster
   */
 private[connector] class RedisCommandsProvider(instance: RedisInstance)(implicit system: ActorSystem, lifecycle: ApplicationLifecycle) extends Provider[RedisCommands] {
 
-  lazy val get = instance match {
+  lazy val get: RedisCommands = instance match {
     case cluster: RedisCluster       => new RedisCommandsCluster(cluster).get
     case standalone: RedisStandalone => new RedisCommandsStandalone(standalone).get
     case sentinel: RedisSentinel     => new RedisCommandsSentinel(sentinel).get
@@ -115,7 +112,7 @@ private[connector] class RedisCommandsCluster(configuration: RedisCluster)(impli
   }
 
   // $COVERAGE-OFF$
-  def start() = {
+  def start(): Unit = {
     def servers = nodes.map {
       case RedisHost(host, port, Some(database), _, _) => s" $host:$port?database=$database"
       case RedisHost(host, port, None, _, _)           => s" $host:$port"
@@ -126,7 +123,7 @@ private[connector] class RedisCommandsCluster(configuration: RedisCluster)(impli
 
   def stop(): Future[Unit] = Future successful {
     log.info("Stopping the redis cluster cache actor ...")
-    client.stop()
+    Option(client).foreach(_.stop())
     log.info("Redis cluster cache stopped.")
   }
   // $COVERAGE-ON$

src/main/scala/play/api/cache/redis/connector/RedisConnectorProvider.scala

@@ -12,7 +12,7 @@ import akka.actor.ActorSystem
   */
 private[redis] class RedisConnectorProvider(instance: RedisInstance, serializer: AkkaSerializer)(implicit system: ActorSystem, lifecycle: ApplicationLifecycle, runtime: RedisRuntime) extends Provider[RedisConnector] {
 
-  private lazy val commands = new RedisCommandsProvider(instance).get
+  private[connector] lazy val commands = new RedisCommandsProvider(instance).get
 
   lazy val get = new RedisConnectorImpl(serializer, commands)
 }

src/main/scala/play/api/cache/redis/impl/InvocationPolicy.scala

@@ -15,9 +15,14 @@ sealed trait InvocationPolicy {
 }
 
 object EagerInvocation extends InvocationPolicy {
-  def invoke[T](f: => Future[Any], thenReturn: T)(implicit context: ExecutionContext) = { f; Future successful thenReturn }
+  override def invoke[T](f: => Future[Any], thenReturn: T)(implicit context: ExecutionContext): Future[T] = {
+    f
+    Future successful thenReturn
+  }
 }
 
 object LazyInvocation extends InvocationPolicy {
-  def invoke[T](f: => Future[Any], thenReturn: T)(implicit context: ExecutionContext) = f.map(_ => thenReturn)
+  override def invoke[T](f: => Future[Any], thenReturn: T)(implicit context: ExecutionContext): Future[T] = {
+    f.map(_ => thenReturn)
+  }
 }

src/main/scala/play/api/cache/redis/impl/RedisCache.scala

@@ -66,9 +66,8 @@ private[impl] class RedisCache[Result[_]](redis: RedisConnector, builder: Builde
     doMatching(pattern).recoverWithDefault(Seq.empty[String])
   }
 
-  def getOrElse[T: ClassTag](key: String, expiration: Duration)(orElse: => T) = key.prefixed { key =>
+  def getOrElse[T: ClassTag](key: String, expiration: Duration)(orElse: => T) =
     getOrFuture(key, expiration)(orElse.toFuture).recoverWithDefault(orElse)
-  }
 
   def getOrFuture[T: ClassTag](key: String, expiration: Duration)(orElse: => Future[T]): Future[T] = key.prefixed { key =>
     redis.get[T](key).flatMap {

src/main/scala/play/api/cache/redis/impl/RedisListJavaImpl.scala

@@ -10,6 +10,17 @@ class RedisListJavaImpl[Elem](internal: RedisList[Elem, Future])(implicit runtim
 
   def This = this
 
+  private lazy val modifier: AsyncRedisList.AsyncRedisListModification[Elem] = newModifier()
+  private lazy val viewer: AsyncRedisList.AsyncRedisListView[Elem] = newViewer()
+
+  protected def newViewer(): AsyncRedisList.AsyncRedisListView[Elem] = {
+    new AsyncRedisListViewJavaImpl(internal.view)
+  }
+
+  protected def newModifier(): AsyncRedisList.AsyncRedisListModification[Elem] = {
+    new AsyncRedisListModificationJavaImpl(internal.modify)
+  }
+
   def prepend(element: Elem): CompletionStage[AsyncRedisList[Elem]] = {
     async { implicit context =>
       internal.prepend(element).map(_ => this)
@@ -70,13 +81,9 @@ class RedisListJavaImpl[Elem](internal: RedisList[Elem, Future])(implicit runtim
     }
   }
 
-  def view(): AsyncRedisList.AsyncRedisListView[Elem] = {
-    new AsyncRedisListViewJavaImpl(internal.view)
-  }
+  def view(): AsyncRedisList.AsyncRedisListView[Elem] = viewer
 
-  def modify(): AsyncRedisList.AsyncRedisListModification[Elem] = {
-    new AsyncRedisListModificationJavaImpl(internal.modify)
-  }
+  def modify(): AsyncRedisList.AsyncRedisListModification[Elem] = modifier
 
   private class AsyncRedisListViewJavaImpl(view: internal.RedisListView) extends AsyncRedisList.AsyncRedisListView[Elem] {
 

src/test/resources/reference.conf

@@ -18,5 +18,5 @@ akka {
     warn-about-java-serializer-usage = off
   }
 
-  loggers = ["play.api.cache.redis.logging.RedisLogger"]
+  loggers = ["play.api.cache.redis.test.RedisLogger"]
 }

src/test/scala/play/api/cache/redis/ExpirationSpec.scala

@@ -1,6 +1,6 @@
 package play.api.cache.redis
 
-import org.specs2.mutable.Specification
+import play.api.cache.redis.test._
 
 import java.time.Instant
 import java.util.Date
@@ -9,7 +9,7 @@ import scala.concurrent.duration._
 /**
   * <p>This specification tests expiration conversion</p>
   */
-class ExpirationSpec extends Specification {
+class ExpirationSpec extends UnitSpec {
 
   "Expiration" should {
 
@@ -20,12 +20,16 @@ class ExpirationSpec extends Specification {
     val expirationTo = expiration + 1.second
 
     "from java.util.Date" in {
-      new Date(expireAt.toEpochMilli).asExpiration must beBetween(expirationFrom, expirationTo)
+      val expiration = new Date(expireAt.toEpochMilli).asExpiration
+      expiration mustBe >=(expirationFrom)
+      expiration mustBe <=(expirationTo)
     }
 
     "from java.time.LocalDateTime" in {
       import java.time._
-      LocalDateTime.ofInstant(expireAt, ZoneId.systemDefault()).asExpiration must beBetween(expirationFrom, expirationTo)
+      val expiration = LocalDateTime.ofInstant(expireAt, ZoneId.systemDefault()).asExpiration
+      expiration mustBe >=(expirationFrom)
+      expiration mustBe <=(expirationTo)
     }
   }
 }