Merge pull request #157 from KarelCemus/tests

Revamped tests

CHANGELOG.md

@@ -12,7 +12,7 @@ Standalone client now fails eagerly when the connection to redis is not
 established. This is to avoid long timeout while the rediscala is trying
 to reconnect. [#147](https://github.com/KarelCemus/play-redis/issues/147)
 
-Deprecated`timeout` property and replaced by `sync-timeout` with the identical
+Deprecated `timeout` property and replaced by `sync-timeout` with the identical
 meaning and use. Will be removed by 2.2.0. [#154](https://github.com/KarelCemus/play-redis/issues/154)
 
 Introduced **optional** `redis-timeout` property indicating timeout on redis queries.
@@ -22,6 +22,8 @@ See [the configuration]() for more details. [#154](https://github.com/KarelCemus
 
 Rediscala bumped to 1.8.3 and subsequently Akka bumped to 2.5.6 [#150](https://github.com/KarelCemus/play-redis/issues/150).
 
+Revamped tests, reduced their number but increased value and code coverage [#108](https://github.com/KarelCemus/play-redis/issues/108)
+
 #### Removal of `@Named` and introduction of `@NamedCache`
 
 Named caches now uses `@NamedCache` instead of `@Named` to be consistent with Play's EhCache and 

build.sbt

@@ -15,14 +15,12 @@ scalaVersion := "2.12.5"
 
 crossScalaVersions := Seq( "2.11.12", scalaVersion.value )
 
-val playVersion = "2.6.12"
+val playVersion = "2.6.13"
 
 val connectorVersion = "1.8.3"
 
 val specs2Version = "4.0.3"
 
-parallelExecution in Test := false
-
 libraryDependencies ++= Seq(
   // play framework cache API
   "com.typesafe.play" %% "play-cache" % playVersion % Provided,
@@ -31,7 +29,9 @@ libraryDependencies ++= Seq(
   // test framework
   "org.specs2" %% "specs2-core" % specs2Version % Test,
   // test module for play framework
-  "com.typesafe.play" %% "play-specs2" % playVersion % Test
+  "com.typesafe.play" %% "play-specs2" % playVersion % Test,
+  // mockito framework
+  "org.mockito" % "mockito-core" % "2.18.0"
 )
 
 resolvers ++= Seq(
@@ -71,3 +71,6 @@ publishTo := {
   if (isSnapshot.value) Some(Opts.resolver.sonatypeSnapshots)
   else Some( Opts.resolver.sonatypeStaging )
 }
+
+// exclude from tests coverage
+coverageExcludedFiles := ".*exceptions.*"

project/build.properties

@@ -1 +1 @@
-sbt.version=1.1.1
+sbt.version=1.1.2

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

@@ -16,9 +16,6 @@ import play.api.inject._
   */
 trait RecoveryPolicy {
 
-  /** name of the policy used for internal purposes */
-  def name: String = this.getClass.getSimpleName
-
   /** When a failure occurs, this method handles it. It may re-run it, return default value,
     * log it or propagate the exception.
     *
@@ -30,7 +27,12 @@ trait RecoveryPolicy {
     */
   def recoverFrom[ T ]( rerun: => Future[ T ], default: => Future[ T ], failure: RedisException ): Future[ T ]
 
+  // $COVERAGE-OFF$
+  /** name of the policy used for internal purposes */
+  def name: String = this.getClass.getSimpleName
+
   override def toString = s"RecoveryPolicy($name)"
+  // $COVERAGE-ON$
 }
 
 /** Abstract recovery policy provides a general helpers for
@@ -47,8 +49,8 @@ trait Reports extends RecoveryPolicy {
   protected def message( failure: RedisException ): String = failure match {
     case TimeoutException( cause ) => s"Command execution timed out."
     case SerializationException( key, message, cause ) => s"$message for key '$key'."
-    case ExecutionFailedException( Some( key ), command, cause ) => s"Command $command for key '$key' failed."
-    case ExecutionFailedException( None, command, cause ) => s"Command $command failed."
+    case ExecutionFailedException( Some( key ), command, statement, cause ) => s"Command $command for key '$key' failed."
+    case ExecutionFailedException( None, command, statement, cause ) => s"Command $command failed."
     case UnexpectedResponseException( Some( key ), command ) => s"Command $command for key '$key' returned unexpected response."
     case UnexpectedResponseException( None, command ) => s"Command $command returned unexpected response."
   }
@@ -98,7 +100,7 @@ trait FailThrough extends RecoveryPolicy {
 
   override def recoverFrom[ T ]( rerun: => Future[ T ], default: => Future[ T ], failure: RedisException ): Future[ T ] = {
     // fail through
-    throw failure
+    Future.failed( failure )
   }
 }
 
@@ -159,6 +161,8 @@ trait RecoveryPolicyResolver {
   def resolve: PartialFunction[ String, RecoveryPolicy ]
 }
 
+// $COVERAGE-OFF$
+
 class RecoveryPolicyResolverImpl extends RecoveryPolicyResolver {
   val resolve: PartialFunction[ String, RecoveryPolicy ] = {
     case "log-and-fail" => new LogAndFailPolicy
@@ -187,3 +191,5 @@ class RecoveryPolicyResolverGuice @Inject( )( injector: Injector ) extends Recov
     case name => injector instanceOf bind[ RecoveryPolicy ].qualifiedWith( name )
   }
 }
+
+// $COVERAGE-ON$

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

@@ -34,10 +34,6 @@ trait RedisCacheComponents
 
   private lazy val akkaSerializer: connector.AkkaSerializer = new connector.AkkaSerializerProvider().get
 
-  private def hasInstances = configuration.underlying.hasPath( "play.cache.redis.instances" )
-
-  private def defaultCache = configuration.underlying.getString( "play.cache.redis.default-cache" )
-
   private lazy val manager = configuration.get( "play.cache.redis" )( play.api.cache.redis.configuration.RedisInstanceManager )
 
   /** translates the cache name into the configuration  */

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

@@ -0,0 +1,14 @@
+package play.api.cache.redis.configuration
+
+/**
+  * @author Karel Cemus
+  */
+private[ configuration ] object Equals {
+
+  // $COVERAGE-OFF$
+  @inline
+  def check[ T ]( a: T, b: T )( property: ( T => Any )* ): Boolean = {
+    property.forall( property => property( a ) == property( b ) )
+  }
+  // $COVERAGE-ON$
+}

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

@@ -22,10 +22,6 @@ private[ configuration ] object RedisConfigLoader {
     def /( suffix: String ): String = if ( path == "" ) suffix else s"$path.$suffix"
   }
 
-  implicit class FallbackValue[ T ]( val value: T ) extends AnyVal {
-    def asFallback = ( _: String ) => value
-  }
-
   def required( path: String ) = throw new IllegalStateException( s"Configuration key '$path' is missing." )
 }
 
@@ -39,10 +35,6 @@ private[ configuration ] object RedisConfigLoader {
   */
 private[ configuration ] trait RedisConfigLoader[ T ] { outer =>
 
-  implicit final def loader( implicit defaults: RedisSettings ) = new ConfigLoader[ T ] {
-    def load( config: Config, path: String ) = outer.load( config, path )
-  }
-
   def load( config: Config, path: String )( implicit defaults: RedisSettings ): T
 }
 
@@ -56,9 +48,5 @@ private[ configuration ] trait RedisConfigLoader[ T ] { outer =>
   */
 private[ configuration ] trait RedisConfigInstanceLoader[ T ] { outer =>
 
-  final def loader( name: String )( implicit defaults: RedisSettings ) = new ConfigLoader[ T ] {
-    def load( config: Config, path: String ) = outer.load( config, path = path, name = name )
-  }
-
   def load( config: Config, path: String, name: String )( implicit defaults: RedisSettings ): T
 }

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

@@ -18,11 +18,12 @@ trait RedisHost {
   def database: Option[ Int ]
   /** when enabled security, this returns password for the AUTH command */
   def password: Option[ String ]
+  // $COVERAGE-OFF$
   /** trait-specific equals */
   override def equals( obj: scala.Any ) = equalsAsHost( obj )
   /** trait-specific equals, invokable from children */
   protected def equalsAsHost( obj: scala.Any ) = obj match {
-    case that: RedisHost => this.host == that.host && this.port == that.port && this.database == that.database && this.password == that.password
+    case that: RedisHost => Equals.check( this, that )( _.host, _.port, _.database, _.password )
     case _ => false
   }
   /** to string */
@@ -32,6 +33,7 @@ trait RedisHost {
     case ( None, Some( database ) ) => s"redis://$host:$port?db=$database"
     case ( None, None ) => s"redis://$host:$port"
   }
+  // $COVERAGE-ON$
 }
 
 
@@ -72,8 +74,11 @@ object RedisHost extends ConfigLoader[ RedisHost ] {
     val password = _password
   }
 
-  def unapply( host: RedisHost ): Option[ (String, Int, Option[ Int ], Option[ String ]) ] =
+  // $COVERAGE-OFF$
+  def unapply( host: RedisHost ): Option[ (String, Int, Option[ Int ], Option[ String ]) ] = {
     Some( (host.host, host.port, host.database, host.password) )
+  }
+  // $COVERAGE-ON$
 }
 
 /**

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

@@ -9,13 +9,15 @@ package play.api.cache.redis.configuration
 sealed trait RedisInstance extends RedisSettings {
   /** name of the redis instance */
   def name: String
+  // $COVERAGE-OFF$
   /** trait-specific equals */
   override def equals( obj: scala.Any ) = equalsAsInstance( obj )
   /** trait-specific equals, invokable from children */
   protected def equalsAsInstance( obj: scala.Any ) = obj match {
     case that: RedisInstance => this.name == that.name && equalsAsSettings( that )
     case _ => false
   }
+  // $COVERAGE-ON$
 }
 
 /**
@@ -27,13 +29,15 @@ sealed trait RedisInstance extends RedisSettings {
 trait RedisCluster extends RedisInstance {
   /** nodes definition when cluster is defined */
   def nodes: List[ RedisHost ]
+  // $COVERAGE-OFF$
   /** trait-specific equals */
   override def equals( obj: scala.Any ) = obj match {
     case that: RedisCluster => equalsAsInstance( that ) && this.nodes == that.nodes
     case _ => false
   }
   /** to string */
   override def toString = s"Cluster[${ nodes mkString "," }]"
+  // $COVERAGE-ON$
 }
 
 object RedisCluster {
@@ -57,6 +61,7 @@ object RedisCluster {
   * @author Karel Cemus
   */
 trait RedisStandalone extends RedisInstance with RedisHost {
+  // $COVERAGE-OFF$
   /** trait-specific equals */
   override def equals( obj: scala.Any ) = obj match {
     case that: RedisStandalone => equalsAsInstance( that ) && equalsAsHost( that )
@@ -67,6 +72,7 @@ trait RedisStandalone extends RedisInstance with RedisHost {
     case Some( database ) => s"Standalone($name@$host:$port?db=$database)"
     case None => s"Standalone($name@$host:$port)"
   }
+  // $COVERAGE-ON$
 }
 
 object RedisStandalone {

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

@@ -36,6 +36,13 @@ trait RedisInstanceManager extends Traversable[ RedisInstanceProvider ] {
 
   /** traverse all binders */
   def foreach[ U ]( f: RedisInstanceProvider => U ) = caches.view.flatMap( instanceOfOption ).foreach( f )
+
+  // $COVERAGE-OFF$
+  override def equals( obj: scala.Any ) = obj match {
+    case that: RedisInstanceManager => Equals.check( this, that )( _.caches, _.defaultInstance, _.toSet )
+    case _ => false
+  }
+  // $COVERAGE-ON$
 }
 
 private[ redis ] object RedisInstanceManager extends ConfigLoader[ RedisInstanceManager ] {

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

@@ -16,13 +16,35 @@ sealed trait RedisInstanceProvider extends Any {
   def resolved( implicit resolver: RedisInstanceResolver ): RedisInstance
 }
 
-class ResolvedRedisInstance( val instance: RedisInstance ) extends AnyVal with RedisInstanceProvider {
+class ResolvedRedisInstance( val instance: RedisInstance ) extends RedisInstanceProvider {
   def name: String = instance.name
   def resolved( implicit resolver: RedisInstanceResolver ) = instance
+
+  // $COVERAGE-OFF$
+  override def equals( obj: scala.Any ) = obj match {
+    case that: ResolvedRedisInstance => this.name == that.name && this.instance == that.instance
+    case _ => false
+  }
+
+  override def hashCode( ) = name.hashCode
+
+  override def toString = s"ResolvedRedisInstance($name@$instance)"
+  // $COVERAGE-ON$
 }
 
-class UnresolvedRedisInstance( val name: String ) extends AnyVal with RedisInstanceProvider {
+class UnresolvedRedisInstance( val name: String ) extends RedisInstanceProvider {
   def resolved( implicit resolver: RedisInstanceResolver ) = resolver resolve name
+
+  // $COVERAGE-OFF$
+  override def equals( obj: scala.Any ) = obj match {
+    case that: UnresolvedRedisInstance => this.name == that.name
+    case _ => false
+  }
+
+  override def hashCode( ) = name.hashCode
+
+  override def toString = s"UnresolvedRedisInstance($name)"
+  // $COVERAGE-ON$
 }
 
 private[ configuration ] object RedisInstanceProvider extends RedisConfigInstanceLoader[ RedisInstanceProvider ] {
@@ -39,7 +61,7 @@ private[ configuration ] object RedisInstanceProvider extends RedisConfigInstanc
       // supplied custom configuration
       case "custom" => RedisInstanceCustom
       // found but unrecognized
-      case other => invalidConfiguration( config.getConfig( path / "source" ).origin(), other )
+      case other => invalidConfiguration( config.getValue( path / "source" ).origin(), other )
     }
   }.load( config, path, name )