You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by du...@apache.org on 2018/01/05 18:50:07 UTC
[incubator-openwhisk] branch master updated: Cleanup test assets
between unit tests using afterEach (#3149)
This is an automated email from the ASF dual-hosted git repository.
dubeejw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new 3c83deb Cleanup test assets between unit tests using afterEach (#3149)
3c83deb is described below
commit 3c83debc2713c8af7ba5545b6135c7498fe566e3
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Fri Jan 5 13:50:04 2018 -0500
Cleanup test assets between unit tests using afterEach (#3149)
* Replace after with afterEach for cleaning up test assets. (No semantic change intended)
* Add test for listing empty activations.
* Reject mutually inclusive parameters.
---
.../main/scala/whisk/http/BasicHttpService.scala | 23 ++---
.../src/main/scala/whisk/http/ErrorResponse.scala | 1 +
.../scala/whisk/core/controller/Activations.scala | 5 +-
tests/src/test/scala/common/WskActorSystem.scala | 2 +-
.../test/scala/services/KafkaConnectorTests.scala | 2 +-
.../core/controller/test/ActionsApiTests.scala | 12 ++-
.../core/controller/test/ActivationsApiTests.scala | 110 +++++++++++++--------
.../controller/test/ControllerTestCommon.scala | 8 +-
.../core/database/test/CacheConcurrencyTests.scala | 8 +-
.../database/test/CouchDbRestClientTests.scala | 4 +-
.../whisk/core/entity/test/DatastoreTests.scala | 14 +--
.../scala/whisk/core/entity/test/ViewTests.scala | 13 +--
12 files changed, 119 insertions(+), 83 deletions(-)
diff --git a/common/scala/src/main/scala/whisk/http/BasicHttpService.scala b/common/scala/src/main/scala/whisk/http/BasicHttpService.scala
index 9f75d63..798bb1e 100644
--- a/common/scala/src/main/scala/whisk/http/BasicHttpService.scala
+++ b/common/scala/src/main/scala/whisk/http/BasicHttpService.scala
@@ -44,16 +44,6 @@ import whisk.common.MetricEmitter
*/
trait BasicHttpService extends Directives with TransactionCounter {
- /** Rejection handler to terminate connection on a bad request. Delegates to Akka handler. */
- implicit def customRejectionHandler(implicit transid: TransactionId) = {
- RejectionHandler.default.mapRejectionResponse {
- case res @ HttpResponse(_, _, ent: HttpEntity.Strict, _) =>
- val error = ErrorResponse(ent.data.utf8String, transid).toJson
- res.copy(entity = HttpEntity(ContentTypes.`application/json`, error.compactPrint))
- case x => x
- }
- }
-
/**
* Gets the routes implemented by the HTTP service.
*
@@ -87,7 +77,7 @@ trait BasicHttpService extends Directives with TransactionCounter {
assignId { implicit transid =>
DebuggingDirectives.logRequest(logRequestInfo _) {
DebuggingDirectives.logRequestResult(logResponseInfo _) {
- handleRejections(customRejectionHandler) {
+ handleRejections(BasicHttpService.customRejectionHandler) {
prioritizeRejections {
toStrictEntity(30.seconds) {
routes
@@ -151,4 +141,15 @@ object BasicHttpService {
Await.result(actorSystem.whenTerminated, 30.seconds)
}
}
+
+ /** Rejection handler to terminate connection on a bad request. Delegates to Akka handler. */
+ def customRejectionHandler(implicit transid: TransactionId) = {
+ RejectionHandler.default.mapRejectionResponse {
+ case res @ HttpResponse(_, _, ent: HttpEntity.Strict, _) =>
+ val error = ErrorResponse(ent.data.utf8String, transid).toJson
+ res.copy(entity = HttpEntity(ContentTypes.`application/json`, error.compactPrint))
+ case x => x
+ }
+ }
+
}
diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index 733c8db..c6ee4ae 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -124,6 +124,7 @@ object Messages {
val abnormalInitialization = "The action did not initialize and exited unexpectedly."
val abnormalRun = "The action did not produce a valid response and exited unexpectedly."
val memoryExhausted = "The action exhausted its memory and was aborted."
+ val docsNotAllowedWithCount = "The parameter 'docs' is not permitted with 'count'."
def badNameFilter(value: String) = s"Parameter may be a 'simple' name or 'package-name/simple' name: $value"
def badEpoch(value: String) = s"Parameter is not a valid value for epoch seconds: $value"
diff --git a/core/controller/src/main/scala/whisk/core/controller/Activations.scala b/core/controller/src/main/scala/whisk/core/controller/Activations.scala
index 06e7efe..c51e00a 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Activations.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Activations.scala
@@ -148,10 +148,11 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
'name.as[Option[EntityPath]] ?,
'since.as[Instant] ?,
'upto.as[Instant] ?) { (skip, limit, count, docs, name, since, upto) =>
+ val invalidDocs = count && docs
val cappedLimit = if (limit == 0) WhiskActivationsApi.maxActivationLimit else limit
// regardless of limit, cap at maxActivationLimit (200) records, client must paginate
- if (cappedLimit <= WhiskActivationsApi.maxActivationLimit) {
+ if (!invalidDocs && cappedLimit <= WhiskActivationsApi.maxActivationLimit) {
val activations = name.flatten match {
case Some(action) =>
WhiskActivation.listActivationsMatchingName(
@@ -179,6 +180,8 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
listEntities {
activations map (_.fold((js) => js, (wa) => wa.map(_.toExtendedJson)))
}
+ } else if (invalidDocs) {
+ terminate(BadRequest, Messages.docsNotAllowedWithCount)
} else {
terminate(BadRequest, Messages.maxActivationLimitExceeded(limit, WhiskActivationsApi.maxActivationLimit))
}
diff --git a/tests/src/test/scala/common/WskActorSystem.scala b/tests/src/test/scala/common/WskActorSystem.scala
index 934a607..917c4db 100644
--- a/tests/src/test/scala/common/WskActorSystem.scala
+++ b/tests/src/test/scala/common/WskActorSystem.scala
@@ -40,7 +40,7 @@ trait WskActorSystem extends BeforeAndAfterAll {
implicit def executionContext: ExecutionContext = actorSystem.dispatcher
- override def afterAll() {
+ override def afterAll() = {
try {
Await.result(Http().shutdownAllConnectionPools(), 30.seconds)
} finally {
diff --git a/tests/src/test/scala/services/KafkaConnectorTests.scala b/tests/src/test/scala/services/KafkaConnectorTests.scala
index b3c6552..6001c9c 100644
--- a/tests/src/test/scala/services/KafkaConnectorTests.scala
+++ b/tests/src/test/scala/services/KafkaConnectorTests.scala
@@ -59,7 +59,7 @@ class KafkaConnectorTests extends FlatSpec with Matchers with WskActorSystem wit
sessionTimeout = sessionTimeout,
maxPollInterval = maxPollInterval)
- override def afterAll() {
+ override def afterAll() = {
producer.close()
consumer.close()
super.afterAll()
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
index 3696063..ac5558b 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -65,6 +65,14 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
val parametersLimit = Parameters.sizeLimit
//// GET /actions
+ it should "return empty list when no actions exist" in {
+ implicit val tid = transid()
+ Get(collectionPath) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ responseAs[List[JsObject]] shouldBe 'empty
+ }
+ }
+
it should "list actions by default namespace" in {
implicit val tid = transid()
val actions = (1 to 2).map { i =>
@@ -72,7 +80,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
}.toList
actions foreach { put(entityStore, _) }
waitOnView(entityStore, WhiskAction, namespace, 2)
- Get(s"$collectionPath") ~> Route.seal(routes(creds)) ~> check {
+ Get(collectionPath) ~> Route.seal(routes(creds)) ~> check {
status should be(OK)
val response = responseAs[List[JsObject]]
actions.length should be(response.length)
@@ -125,7 +133,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
it should "list should reject request with post" in {
implicit val tid = transid()
- Post(s"$collectionPath") ~> Route.seal(routes(creds)) ~> check {
+ Post(collectionPath) ~> Route.seal(routes(creds)) ~> check {
status should be(MethodNotAllowed)
}
}
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
index e4a02ff..c36a7c1 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
@@ -126,6 +126,16 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
}
//// GET /activations?docs=true
+ it should "return empty list when no activations exist" in {
+ implicit val tid = transid()
+ whisk.utils.retry { // retry because view will be stale from previous test and result in null doc fields
+ Get(s"$collectionPath?docs=true") ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ responseAs[List[JsObject]] shouldBe 'empty
+ }
+ }
+ }
+
it should "get full activation by namespace" in {
implicit val tid = transid()
// create two sets of activation records, and check that only one set is served back
@@ -186,7 +196,13 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
val since = now.plusSeconds(10)
val upto = now.plusSeconds(30)
implicit val activations = Seq(
- WhiskActivation(namespace, actionName, creds.subject, ActivationId(), start = now.plusSeconds(9), end = now),
+ WhiskActivation(
+ namespace,
+ actionName,
+ creds.subject,
+ ActivationId(),
+ start = now.plusSeconds(9),
+ end = now.plusSeconds(9)),
WhiskActivation(
namespace,
actionName,
@@ -207,61 +223,64 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
creds.subject,
ActivationId(),
start = now.plusSeconds(31),
- end = now.plusSeconds(20)),
+ end = now.plusSeconds(31)),
WhiskActivation(
namespace,
actionName,
creds.subject,
ActivationId(),
start = now.plusSeconds(30),
- end = now.plusSeconds(20))) // should match
+ end = now.plusSeconds(30))) // should match
activations foreach { put(activationStore, _) }
waitOnView(activationStore, namespace.root, activations.length, WhiskActivation.view)
- // get between two time stamps
- whisk.utils.retry {
- Get(s"$collectionPath?docs=true&since=${since.toEpochMilli}&upto=${upto.toEpochMilli}") ~> Route.seal(
- routes(creds)) ~> check {
- status should be(OK)
- val response = responseAs[List[JsObject]]
- val expected = activations filter {
- case e =>
- (e.start.equals(since) || e.start.equals(upto) || (e.start.isAfter(since) && e.start.isBefore(upto)))
+ { // get between two time stamps
+ val filter = s"since=${since.toEpochMilli}&upto=${upto.toEpochMilli}"
+ val expected = activations.filter { e =>
+ (e.start.equals(since) || e.start.equals(upto) || (e.start.isAfter(since) && e.start.isBefore(upto)))
+ }
+
+ whisk.utils.retry {
+ Get(s"$collectionPath?docs=true&$filter") ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ val response = responseAs[List[JsObject]]
+ expected.length should be(response.length)
+ expected forall { a =>
+ response contains a.toExtendedJson
+ } should be(true)
}
- expected.length should be(response.length)
- expected forall { a =>
- response contains a.toExtendedJson
- } should be(true)
}
}
- // get 'upto' with no defined since value should return all activation 'upto'
- whisk.utils.retry {
- Get(s"$collectionPath?docs=true&upto=${upto.toEpochMilli}") ~> Route.seal(routes(creds)) ~> check {
- status should be(OK)
- val response = responseAs[List[JsObject]]
- val expected = activations filter {
- case e => e.start.equals(upto) || e.start.isBefore(upto)
+ { // get 'upto' with no defined since value should return all activation 'upto'
+ val expected = activations.filter(e => e.start.equals(upto) || e.start.isBefore(upto))
+ val filter = s"upto=${upto.toEpochMilli}"
+
+ whisk.utils.retry {
+ Get(s"$collectionPath?docs=true&$filter") ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ val response = responseAs[List[JsObject]]
+ expected.length should be(response.length)
+ expected forall { a =>
+ response contains a.toExtendedJson
+ } should be(true)
}
- expected.length should be(response.length)
- expected forall { a =>
- response contains a.toExtendedJson
- } should be(true)
}
}
- // get 'since' with no defined upto value should return all activation 'since'
- whisk.utils.retry {
- Get(s"$collectionPath?docs=true&since=${since.toEpochMilli}") ~> Route.seal(routes(creds)) ~> check {
- status should be(OK)
- val response = responseAs[List[JsObject]]
- val expected = activations filter {
- case e => e.start.equals(since) || e.start.isAfter(since)
+ { // get 'since' with no defined upto value should return all activation 'since'
+ whisk.utils.retry {
+ val expected = activations.filter(e => e.start.equals(since) || e.start.isAfter(since))
+ val filter = s"since=${since.toEpochMilli}"
+
+ Get(s"$collectionPath?docs=true&$filter") ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ val response = responseAs[List[JsObject]]
+ expected.length should be(response.length)
+ expected forall { a =>
+ response contains a.toExtendedJson
+ } should be(true)
}
- expected.length should be(response.length)
- expected forall { a =>
- response contains a.toExtendedJson
- } should be(true)
}
}
}
@@ -345,15 +364,24 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
}
}
+ it should "reject invalid query parameter combinations" in {
+ implicit val tid = transid()
+ whisk.utils.retry { // retry because view will be stale from previous test and result in null doc fields
+ Get(s"$collectionPath?docs=true&count=true") ~> Route.seal(routes(creds)) ~> check {
+ status should be(BadRequest)
+ responseAs[ErrorResponse].error shouldBe Messages.docsNotAllowedWithCount
+ }
+ }
+ }
+
it should "reject activation list when limit is greater than maximum allowed value" in {
implicit val tid = transid()
val exceededMaxLimit = WhiskActivationsApi.maxActivationLimit + 1
val response = Get(s"$collectionPath?limit=$exceededMaxLimit") ~> Route.seal(routes(creds)) ~> check {
- val response = responseAs[String]
- response should include {
+ status should be(BadRequest)
+ responseAs[ErrorResponse].error shouldBe {
Messages.maxActivationLimitExceeded(exceededMaxLimit, WhiskActivationsApi.maxActivationLimit)
}
- status should be(BadRequest)
}
}
diff --git a/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala b/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
index e7af616..1370964 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
@@ -21,7 +21,7 @@ import scala.concurrent.{Await, Future}
import scala.concurrent.ExecutionContext
import scala.concurrent.duration.{DurationInt, FiniteDuration}
import scala.language.postfixOps
-import org.scalatest.BeforeAndAfter
+import org.scalatest.BeforeAndAfterEach
import org.scalatest.BeforeAndAfterAll
import org.scalatest.FlatSpec
import org.scalatest.Matchers
@@ -48,7 +48,7 @@ import whisk.spi.SpiLoader
protected trait ControllerTestCommon
extends FlatSpec
- with BeforeAndAfter
+ with BeforeAndAfterEach
with BeforeAndAfterAll
with ScalatestRouteTest
with Matchers
@@ -149,11 +149,11 @@ protected trait ControllerTestCommon
val NAMESPACES = Collection(Collection.NAMESPACES)
val PACKAGES = Collection(Collection.PACKAGES)
- after {
+ override def afterEach() = {
cleanup()
}
- override def afterAll() {
+ override def afterAll() = {
println("Shutting down db connections");
entityStore.shutdown()
activationStore.shutdown()
diff --git a/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala b/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
index 5ae8167..d2cac40 100644
--- a/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
+++ b/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
@@ -22,7 +22,7 @@ import scala.concurrent.duration.DurationInt
import scala.concurrent.forkjoin.ForkJoinPool
import org.junit.runner.RunWith
-import org.scalatest.BeforeAndAfter
+import org.scalatest.BeforeAndAfterEach
import org.scalatest.FlatSpec
import org.scalatest.junit.JUnitRunner
@@ -38,7 +38,7 @@ import whisk.common.TransactionId
import whisk.utils.retry
@RunWith(classOf[JUnitRunner])
-class CacheConcurrencyTests extends FlatSpec with WskTestHelpers with BeforeAndAfter {
+class CacheConcurrencyTests extends FlatSpec with WskTestHelpers with BeforeAndAfterEach {
println(s"Running tests on # proc: ${Runtime.getRuntime.availableProcessors()}")
@@ -58,13 +58,13 @@ class CacheConcurrencyTests extends FlatSpec with WskTestHelpers with BeforeAndA
withClue(s"$phase: failed for $name") { (name, block(name)) }
}
- before {
+ override def beforeEach() = {
run("pre-test sanitize") { name =>
wsk.action.sanitize(name)
}
}
- after {
+ override def afterEach() = {
run("post-test sanitize") { name =>
wsk.action.sanitize(name)
}
diff --git a/tests/src/test/scala/whisk/core/database/test/CouchDbRestClientTests.scala b/tests/src/test/scala/whisk/core/database/test/CouchDbRestClientTests.scala
index 26e789f..4803f25 100644
--- a/tests/src/test/scala/whisk/core/database/test/CouchDbRestClientTests.scala
+++ b/tests/src/test/scala/whisk/core/database/test/CouchDbRestClientTests.scala
@@ -72,14 +72,14 @@ class CouchDbRestClientTests
config.dbPassword,
dbName)
- override def beforeAll() {
+ override def beforeAll() = {
super.beforeAll()
whenReady(client.createDb()) { r =>
assert(r.isRight)
}
}
- override def afterAll() {
+ override def afterAll() = {
whenReady(client.deleteDb()) { r =>
assert(r.isRight)
}
diff --git a/tests/src/test/scala/whisk/core/entity/test/DatastoreTests.scala b/tests/src/test/scala/whisk/core/entity/test/DatastoreTests.scala
index f850f6b..abf9e9f 100644
--- a/tests/src/test/scala/whisk/core/entity/test/DatastoreTests.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/DatastoreTests.scala
@@ -23,7 +23,7 @@ import scala.Vector
import scala.concurrent.Await
import org.junit.runner.RunWith
-import org.scalatest.BeforeAndAfter
+import org.scalatest.BeforeAndAfterEach
import org.scalatest.BeforeAndAfterAll
import org.scalatest.FlatSpec
import org.scalatest.junit.JUnitRunner
@@ -41,7 +41,7 @@ import whisk.core.entity._
@RunWith(classOf[JUnitRunner])
class DatastoreTests
extends FlatSpec
- with BeforeAndAfter
+ with BeforeAndAfterEach
with BeforeAndAfterAll
with WskActorSystem
with DbUtils
@@ -56,7 +56,11 @@ class DatastoreTests
implicit val cacheUpdateNotifier: Option[CacheChangeNotification] = None
- override def afterAll() {
+ override def afterEach() = {
+ cleanup()
+ }
+
+ override def afterAll() = {
println("Shutting down store connections")
datastore.shutdown()
authstore.shutdown()
@@ -71,10 +75,6 @@ class DatastoreTests
def afullname(implicit namespace: EntityPath, name: String) = FullyQualifiedEntityName(namespace, EntityName(name))
- after {
- cleanup()
- }
-
behavior of "Datastore"
it should "CRD action blackbox" in {
diff --git a/tests/src/test/scala/whisk/core/entity/test/ViewTests.scala b/tests/src/test/scala/whisk/core/entity/test/ViewTests.scala
index 55b2e69..24c861f 100644
--- a/tests/src/test/scala/whisk/core/entity/test/ViewTests.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/ViewTests.scala
@@ -22,14 +22,9 @@ import java.time.Instant
import scala.concurrent.Await
import scala.language.postfixOps
-
import org.junit.runner.RunWith
-import org.scalatest.BeforeAndAfter
-import org.scalatest.BeforeAndAfterAll
-import org.scalatest.FlatSpec
-import org.scalatest.Matchers
+import org.scalatest._
import org.scalatest.junit.JUnitRunner
-
import akka.stream.ActorMaterializer
import common.StreamLogging
import common.WskActorSystem
@@ -44,7 +39,7 @@ import whisk.core.entity.WhiskEntityQueries._
@RunWith(classOf[JUnitRunner])
class ViewTests
extends FlatSpec
- with BeforeAndAfter
+ with BeforeAndAfterEach
with BeforeAndAfterAll
with Matchers
with DbUtils
@@ -75,11 +70,11 @@ class ViewTests
val entityStore = WhiskEntityStore.datastore(config)
val activationStore = WhiskActivationStore.datastore(config)
- after {
+ override def afterEach = {
cleanup()
}
- override def afterAll() {
+ override def afterAll() = {
println("Shutting down store connections")
entityStore.shutdown()
activationStore.shutdown()
--
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].