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 <rod...@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" <commits@openwhisk.apache.org>'].