Improve test coverage

RB_ID=757326

build.sbt

@@ -318,7 +318,7 @@ lazy val jackson = project.
     )
   ).
   dependsOn(
-    injectServer % "test->test",
+    injectApp % "test->test",
     utils
   )
 

inject/inject-core/src/test/scala/com/twitter/inject/tests/TwitterModuleTest.scala

@@ -3,7 +3,7 @@ package com.twitter.inject.tests
 import com.google.inject.{Key, Guice}
 import com.twitter.finatra.tests.Prod
 import com.twitter.inject.{Injector, Test}
-import com.twitter.inject.tests.module.{ComplexServiceFactory, DoEverythingModule}
+import com.twitter.inject.tests.module.{ClassToConvert, ComplexServiceFactory, DoEverythingModule}
 
 class TwitterModuleTest extends Test {
 
@@ -27,6 +27,10 @@ class TwitterModuleTest extends Test {
     injector.instance[String, Prod] should be("prod string")
   }
 
+  "type conversion" in {
+    injector.instance[ClassToConvert](name = "name") should be(ClassToConvert("Steve"))
+  }
+
   def assertServiceFactory(complexServiceFactory: ComplexServiceFactory): Unit = {
     complexServiceFactory.create("foo").execute should equal("done foo")
   }

inject/inject-core/src/test/scala/com/twitter/inject/tests/module/DoEverythingModule.scala

@@ -1,10 +1,12 @@
 package com.twitter.inject.tests.module
 
-import com.google.inject.Provides
 import com.google.inject.name.{Named, Names}
+import com.google.inject.spi.TypeConverter
+import com.google.inject.{Provides, TypeLiteral}
 import com.twitter.conversions.time._
 import com.twitter.finatra.tests.Prod
 import com.twitter.inject.{Injector, TwitterModule}
+import java.util.Properties
 
 object DoEverythingModule extends TwitterModule {
 
@@ -26,6 +28,18 @@ object DoEverythingModule extends TwitterModule {
     val multiBinder = createMultiBinder[MultiService]
     multiBinder.addBinding.to[OneMultiService]
     multiBinder.addBinding.to[TwoMultiService]
+
+    addTypeConvertor[ClassToConvert](
+      new TypeConverter {
+        override def convert(s: String, typeLiteral: TypeLiteral[_]): AnyRef = {
+          ClassToConvert(s)
+        }
+      })
+
+
+    val properties = new Properties()
+    properties.setProperty("name", "Steve")
+    Names.bindProperties(binder(), properties)
   }
 
   override def singletonStartup(injector: Injector) {
@@ -42,3 +56,6 @@ object DoEverythingModule extends TwitterModule {
     "named"
   }
 }
+
+case class ClassToConvert(
+  name: String)

inject/inject-server/src/main/scala/com/twitter/server/internal/FinagleBuildRevision.scala

@@ -21,7 +21,7 @@ object FinagleBuildRevision extends Logging {
 
   /* Private */
 
-  private def convertBuildRevision(revision: String): Long = {
+  private[server] def convertBuildRevision(revision: String): Long = {
     try {
       revision.take(10).toU64Long
     } catch {

inject/inject-server/src/test/scala/BUILD

@@ -4,7 +4,9 @@ junit_tests(name='scala',
     'finatra/inject/inject-server',
     'finatra/inject/inject-server:test-deps'
   ],
-  sources=globs('com/twitter/inject/server/tests/*.scala'),
+  sources=globs(
+    'com/twitter/server/tests/*.scala',
+    'com/twitter/inject/server/tests/*.scala'),
   resources=[
     'finatra/inject/inject-server/src/test/resources'
   ]

inject/inject-server/src/test/scala/com/twitter/inject/server/tests/PortUtilsTest.scala

@@ -0,0 +1,46 @@
+package com.twitter.inject.server.tests
+
+import com.twitter.finagle.builder.Server
+import com.twitter.inject.Test
+import com.twitter.inject.server.PortUtils
+import com.twitter.util.Awaitable.CanAwait
+import com.twitter.util.{Time, Duration}
+import java.net.InetSocketAddress
+
+class PortUtilsTest extends Test {
+
+  "PortUtils" should {
+
+    "getPort for Server" in {
+      PortUtils.getPort(new Server {
+        /**
+         * The address to which this server is bound.
+         */
+        override def boundAddress = new InetSocketAddress(9999)
+
+        override protected def closeServer(deadline: Time) = ???
+
+        /**
+         * Support for `Await.result`. The use of the implicit permit is an
+         * access control mechanism: only `Await.result` may call this
+         * method.
+         */
+        override def result(timeout: Duration)(implicit permit: CanAwait) = ???
+
+        /**
+         * Is this Awaitable ready? In other words: would calling
+         * [[com.twitter.util.Awaitable.ready Awaitable.ready]] block?
+         */
+        override def isReady(implicit permit: CanAwait) = ???
+
+        /**
+         * Support for `Await.ready`. The use of the implicit permit is an
+         * access control mechanism: only `Await.ready` may call this
+         * method.
+         */
+        override def ready(timeout: Duration)(implicit permit: CanAwait) = ???
+      }) should be(9999)
+    }
+  }
+
+}

inject/inject-server/src/test/scala/com/twitter/server/tests/FinagleBuildRevisionTest.scala

@@ -0,0 +1,22 @@
+package com.twitter.server.tests
+
+import com.twitter.inject.Test
+import com.twitter.server.internal.FinagleBuildRevision
+import org.apache.commons.lang.RandomStringUtils
+
+class FinagleBuildRevisionTest extends Test {
+
+  val revision = "e748e2fe89da63923e525b71f82f398b18c9a60c"
+
+  "build revision" should {
+
+    "parse revision" in {
+      FinagleBuildRevision.convertBuildRevision(revision) should be(993360281225L)
+    }
+
+    "fail revision parsing" in {
+      FinagleBuildRevision.convertBuildRevision(RandomStringUtils.randomAlphanumeric(20)) should be(-1L)
+    }
+  }
+
+}

jackson/src/main/scala/com/twitter/finatra/json/internal/caseclass/validation/validators/SizeValidator.scala

@@ -25,7 +25,7 @@ object SizeValidator {
       case traversableValue: Traversable[_] =>
         traversableValue.size
       case str: String =>
-        str.size
+        str.length
       case _ =>
         throw new IllegalArgumentException("Class [%s] is not supported" format value.getClass)
     }
@@ -49,7 +49,7 @@ class SizeValidator(
     val size = value match {
       case arrayValue: Array[_] => arrayValue.length
       case traversableValue: Traversable[_] => traversableValue.size
-      case str: String => str.size
+      case str: String => str.length
       case _ =>
         throw new IllegalArgumentException("Class [%s] is not supported" format value.getClass)
     }

jackson/src/test/scala/BUILD

@@ -4,6 +4,7 @@ junit_tests(name='scala',
     'finatra/jackson/src/main/scala',
     'finatra/jackson/src/main/java',
     'finatra/jackson/src/test/scala:test-deps',
+    'finatra/inject/inject-app:test-deps',
     'finatra/slf4j',
     'util/util-core'
   ],

jackson/src/test/scala/com/twitter/finatra/tests/json/FinatraObjectMapperTest.scala

@@ -3,13 +3,16 @@ package com.twitter.finatra.tests.json
 import com.fasterxml.jackson.databind.node.{IntNode, TreeTraversingParser}
 import com.fasterxml.jackson.databind.{JsonMappingException, JsonNode}
 import com.twitter.finagle.http.{Request, Response}
+import com.twitter.finatra.annotations.CamelCaseMapper
 import com.twitter.finatra.conversions.time._
 import com.twitter.finatra.json.internal.caseclass.exceptions.{CaseClassMappingException, CaseClassValidationException, JsonInjectionNotSupportedException, RequestFieldInjectionNotSupportedException}
+import com.twitter.finatra.json.modules.FinatraJacksonModule
 import com.twitter.finatra.json.{FinatraObjectMapper, JsonDiff}
 import com.twitter.finatra.tests.json.internal.Obj.NestedCaseClassInObject
 import com.twitter.finatra.tests.json.internal._
 import com.twitter.finatra.tests.json.internal.internal.{SimplePersonInPackageObjectWithoutConstructorParams, SimplePersonInPackageObject}
 import com.twitter.inject.Logging
+import com.twitter.inject.app.TestInjector
 import com.twitter.io.Buf
 import java.io.{ByteArrayInputStream, ByteArrayOutputStream}
 import org.joda.time.{DateTime, DateTimeZone}
@@ -24,6 +27,9 @@ class FinatraObjectMapperTest extends FeatureSpec with Matchers with Logging {
 
   /* Class under test */
   val mapper = FinatraObjectMapper.create()
+  /* Test Injector */
+  val injector = TestInjector(
+    FinatraJacksonModule)
 
   feature("simple tests") {
 
@@ -1031,6 +1037,14 @@ class FinatraObjectMapperTest extends FeatureSpec with Matchers with Logging {
     }
   }
 
+  feature("Support camel case mapper") {
+    scenario("camel case object") {
+      val camelCaseObjectMapper = injector.instance[FinatraObjectMapper, CamelCaseMapper]
+
+      camelCaseObjectMapper.parse[Map[String, String]]("""{"firstName": "Bob"}""") should equal(Map("firstName" -> "Bob"))
+    }
+  }
+
   private def assertJsonParse[T: Manifest](json: String, withErrors: Seq[String]) = {
     if (withErrors.nonEmpty) {
       val e1 = intercept[CaseClassMappingException] {

jackson/src/test/scala/com/twitter/finatra/tests/json/internal/caseclass/validation/validators/SizeValidatorTest.scala

@@ -3,11 +3,13 @@ package com.twitter.finatra.tests.json.internal.caseclass.validation.validators
 import com.twitter.finatra.json.internal.caseclass.validation.validators.SizeValidator
 import com.twitter.finatra.validation.ValidationResult._
 import com.twitter.finatra.validation.{ErrorCode, Size, ValidationResult, ValidatorTest}
+import org.apache.commons.lang.RandomStringUtils
 
 
 case class SizeArrayExample(@Size(min = 1, max = 5) sizeValue: Array[Int])
 case class SizeSeqExample(@Size(min = 1, max = 5) sizeValue: Array[Int])
 case class SizeInvalidTypeExample(@Size(min = 1, max = 5) sizeValue: Int)
+case class SizeStringExample(@Size(min = 1, max = 140) sizeValue: String)
 
 class SizeValidatorTest extends ValidatorTest {
 
@@ -39,6 +41,19 @@ class SizeValidatorTest extends ValidatorTest {
       validate[SizeArrayExample](value) should equal(Valid)
     }
 
+    "pass validation for string type" in {
+      val value = RandomStringUtils.randomNumeric(140)
+      validate[SizeStringExample](value) should equal(Valid)
+    }
+
+    "fail validation for string type" in {
+      val value = RandomStringUtils.randomNumeric(141)
+      validate[SizeStringExample](value) should equal(
+        Invalid(
+          errorMessage(value, minValue = 1, maxValue = 140),
+          ErrorCode.SizeOutOfRange(141, 1, 140)))
+    }
+
     "fail validation for too few seq type" in {
       val value = Seq()
       validate[SizeArrayExample](value) should equal(

slf4j/src/test/scala/com/twitter/finatra/logging/integration/PooledServer.scala

@@ -4,13 +4,14 @@ import com.twitter.finagle.http.{Request, Response}
 import com.twitter.finatra.http.HttpServer
 import com.twitter.finatra.http.filters.CommonFilters
 import com.twitter.finatra.http.routing.HttpRouter
-import com.twitter.finatra.logging.filter.LoggingMDCFilter
+import com.twitter.finatra.logging.filter.{TraceIdMDCFilter, LoggingMDCFilter}
 
 class PooledServer extends HttpServer {
 
   override def configureHttp(router: HttpRouter) {
     router.
       filter[LoggingMDCFilter[Request, Response]].
+      filter[TraceIdMDCFilter[Request, Response]].
       filter[CommonFilters].
       add[PooledController]
   }

utils/src/test/scala/com/twitter/finatra/tests/exceptions/ExternalServiceExceptionMatcherTest.scala

@@ -4,14 +4,15 @@ import com.twitter.conversions.time._
 import com.twitter.finagle._
 import com.twitter.finatra.exceptions.ExternalServiceExceptionMatcher
 import com.twitter.inject.Test
+import com.twitter.util.Duration
 import java.io.IOException
 import java.net.ConnectException
 import org.apache.thrift.transport.TTransportException
 
 class ExternalServiceExceptionMatcherTest extends Test {
 
   "match external exceptions" in {
-    val externalExceptions = Seq(
+    val externalExceptions: Seq[Exception] = Seq(
       new RequestException(),
       new ApiException(),
       new ChannelWriteException(new Exception),
@@ -20,9 +21,17 @@ class ExternalServiceExceptionMatcherTest extends Test {
       new ChannelClosedException(),
       new InterruptedException(),
       new TTransportException(),
-      new ConnectException())
+      new ConnectException(),
+      new TimeoutException() {
+        override protected val timeout: Duration = 5.seconds
+        override protected def explanation: String = "timeout!"
+      })
 
     externalExceptions forall ExternalServiceExceptionMatcher.apply should equal(true)
+
+    externalExceptions forall { e =>
+      ExternalServiceExceptionMatcher.unapply(e).isDefined
+    } should equal(true)
   }
 
   "not match non-external exceptions" in {
@@ -32,6 +41,9 @@ class ExternalServiceExceptionMatcherTest extends Test {
       new NullPointerException)
 
     nonExternalExceptions forall ExternalServiceExceptionMatcher.apply should equal(false)
-  }
 
+    nonExternalExceptions forall { e =>
+      ExternalServiceExceptionMatcher.unapply(e).isDefined
+    } should equal(false)
+  }
 }