dubeejw closed pull request #3149: Cleanup test assets between unit tests using
afterEach
URL: https://github.com/apache/incubator-openwhisk/pull/3149
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/common/scala/src/main/scala/whisk/http/BasicHttpService.scala
b/common/scala/src/main/scala/whisk/http/BasicHttpService.scala
index 9f75d630d2..798bb1eff8 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 8dcfe2077b..273166d95e 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -126,6 +126,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 06e7efe669..c51e00a42f 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 934a607bda..917c4dbd6d 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 b3c6552f72..6001c9cd58 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 3696063942..ac5558be58 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 e4a02ff770..c36a7c1de7 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 e7af616a91..1370964ef8 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 5ae8167737..d2cac40438 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 26e789faf2..4803f25b60 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 f850f6b54e..abf9e9f615 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 55b2e69c6a..24c861fa8d 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()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
