markusthoemmes commented on a change in pull request #2577: Support multiple 
response header values in raw web actions
URL: 
https://github.com/apache/incubator-openwhisk/pull/2577#discussion_r131519162
 
 

 ##########
 File path: 
tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
 ##########
 @@ -1127,6 +1127,27 @@ trait WebActionsApiTests extends ControllerTestCommon 
with BeforeAndAfterEach wi
                 }
         }
 
+        it should s"support multiple values for headers (auth? 
${creds.isDefined})" in {
+            implicit val tid = transid()
+
+            Seq(s"$systemId/proxy/export_c.http").
+                foreach { path =>
+                    invocationsAllowed += 1
+                    actionResult = Some(
+                        JsObject(
+                            "headers" -> JsObject(
+                                "Set-Cookie" -> JsArray(JsString("a=b"), 
JsString("c=d; Path = /")))))
+
+                    Options(s"$testRoutePath/$path") ~> 
sealRoute(routes(creds)) ~> check {
+                        withClue(headers) {
+                            headers.length shouldBe 2
+                            headers(0).toString shouldBe "Set-Cookie: a=b"
+                            headers(1).toString shouldBe "Set-Cookie: c=d; 
Path = /"
+                        }
 
 Review comment:
   Using the extensive matchers of ScalaTest, we can write this in a more 
speaking way and even preserve great error messages in case a test fails. We 
also should preserve typesafety here and not rely on the string representation 
of the headers:
   
   Assuming you want to keep the order stable (which I think is not necessary 
here) you could write:
   
   ```scala
   headers shouldBe List(RawHeader("Set-Cookie", "a=b"), 
RawHeader("Set-Cookie", "c=d; Path = /"))
   ```
   
   This code would fail with appropriate error messages if the list doesn't 
comply to the test, and automatically print both lists to be checked thus 
removing the need for `withClue` and a specific length check.
   
   As I think we shouldn't check for a specific order here, we could use 
`contains allOf` like so:
   
   ```scala
   headers should contain allOf (
       RawHeader("Set-Cookie", "a=b"),
       RawHeader("Set-Cookie", "c=d; Path = /")
   )
   ```
   
   Externalizing the cookie values and the headername (maybe even strongly 
typing them with `scala.http.HttpHeaders.Set-Cookie` and 
`scala.http.HttpCookie`) is valuable here as well.
 
----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to