Rewrite wsk basic tests #3103

houshengbo · 2017-12-13T19:59:36Z

This rewrites the test cases in WskBasicTests by implementing them in REST interface.

markusthoemmes · 2017-12-14T07:38:46Z

Please provide a description about what this PR is supposed to solve and what's the intended change.

markusthoemmes

Please fix mutability aspects and some parsing which could make the tests clearer.

markusthoemmes · 2017-12-14T08:58:03Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+    stdout should include regex (""""key":"a"""")
+    stdout should include regex (""""value":"A"""")
+    stdout should include regex (""""publish":true""")
+    stdout should include regex (""""version":"0.0.2"""")


Can we parse the JSON response of the API and then assert on the parsed values? That would remove those whitespace issues alltogether.

markusthoemmes · 2017-12-14T08:59:06Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+      result.getField("version") shouldBe "0.0.1"
+      result.getFieldJsValue("publish").toString shouldBe "false"
+      result.getFieldJsValue("binding").toString shouldBe "{}"
+      result.getField("invalid") shouldBe ""


Above tests should work like this!

markusthoemmes · 2017-12-14T08:59:26Z

tests/src/test/scala/common/rest/WskRest.scala

+        case (value) =>
+          val fullName = fullEntityName(value)
+          paramsList :+= JsString(fullName)
+      }
    }


See #2996 for comments on these parts.

dubee

@houshengbo, what's the status of this PR?

rabbah · 2018-01-12T17:47:13Z

tests/src/test/scala/system/basic/WskBasicTests.scala

-      ActivationResult.serdes.read(removeCLIHeader(stderr).parseJson).response.result shouldBe Some {
-        JsObject("error" -> JsObject("msg" -> "failed activation on purpose".toJson))
-      }
+      val result = wsk.action.invoke(name, blocking = true, expectedExitCode = 502)


502? Why not use the akka code?

rabbah · 2018-01-12T17:47:47Z

tests/src/test/scala/system/basic/WskBasicTests.scala

-    stdout should include regex (""""version": "0.0.2"""")
-    wsk.pkg.list().stdout should include(name)
+    val pack = wsk.pkg.get(name)
+    pack.getFieldJsValue("publish").toString shouldBe "true"


you can remove toString and use shouldBe JsBoolean(true) also fyi.

rabbah · 2018-01-12T17:48:28Z

tests/src/test/scala/system/basic/WskBasicTests.scala

-      val stderr = wsk.pkg.create(name, expectedExitCode = CONFLICT).stderr
-      stderr should include regex (s"""Unable to create package '$name': resource already exists \\(code \\d+\\)""")
+      val stderr = wsk.pkg.create(name, expectedExitCode = Conflict.intValue).stderr
+      stderr should include regex (s"""resource already exists""")


many cases of s"""xxx""" dont need string interpolation, and can just be "xxx".

rabbah · 2018-01-12T17:49:11Z

tests/src/test/scala/system/basic/WskBasicTests.scala

-      .stdout should include regex (""""count": 3""")
+    val result = wsk.action
+      .invoke(name, Map("payload" -> "one two three".toJson), blocking = true, result = true)
+    result.stdout should include regex (""""count":3""")


this check is weaker now - it should be:
result.stdout.parseJson.asJsObject shouldBe JsObject("count" -> JsNumber(3))

rabbah · 2018-01-12T17:51:31Z

tests/src/test/scala/system/basic/WskBasicTests.scala

  }

  it should "list entities in default namespace" in {
    // use a fresh wsk props instance that is guaranteed to use
    // the default namespace
-    wsk.namespace.get(expectedExitCode = SUCCESS_EXIT)(WskProps()).stdout should include("default")
+    val result = wsk.namespace.get()(WskProps()).stdout
+    result should include("actions")


how about parsing the output and checking that it's a JsObject that includes these four fields, and each value is an JsArray.

I will remove this test case, since get is not supported by REST implementation.

rabbah · 2018-01-12T17:52:33Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+
+  it should "not list entities with an invalid namespace" in {
+    val namespace = "fakeNamespace"
+    val stderr = wsk.namespace.get(Some(s"/${namespace}"), expectedExitCode = Forbidden.intValue).stderr


use action list as this will no longer work soon.

I will remove this test case, since get is not supported by REST implementation.

rabbah

See comments.

rabbah · 2018-01-17T14:52:43Z

@markusthoemmes can you update your review?

houshengbo · 2018-01-17T15:26:17Z

Relaunched a PG: Playground3 1691.

rabbah · 2018-01-23T03:25:10Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+      result.getField("name") shouldBe name
+      result.getField("version") shouldBe "0.0.1"
+      result.getFieldJsValue("publish") shouldBe JsBoolean(false)
+      result.getFieldJsValue("binding").toString shouldBe "{}"


result.getFieldJsValue("binding") shouldBe JsObject()?

rabbah · 2018-01-23T03:27:10Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+      result.getFieldJsValue("annotations").toString shouldBe "[]"
+      result.getFieldJsValue("parameters") shouldBe JsArray(
+        JsObject("key" -> JsString("payload"), "value" -> JsString("test")))
+      result.getFieldJsValue("limits").toString shouldBe "{}"


i think you can replace all the {} checks with JsObject()

requested changes already made.

This PR rewrites WskBasicTests.

houshengbo force-pushed the rewrite-WskBasicTests branch 2 times, most recently from ac1835d to 44808c8 Compare December 13, 2017 20:43

houshengbo added review Review for this PR has been requested and yet needs to be done. cli labels Dec 13, 2017

houshengbo force-pushed the rewrite-WskBasicTests branch from 44808c8 to 06f9236 Compare December 13, 2017 22:41

markusthoemmes previously requested changes Dec 14, 2017

View reviewed changes

houshengbo force-pushed the rewrite-WskBasicTests branch 4 times, most recently from 49f5f94 to 7c02dff Compare December 18, 2017 20:13

houshengbo force-pushed the rewrite-WskBasicTests branch from 7c02dff to 2b7eee0 Compare January 2, 2018 18:24

rabbah mentioned this pull request Jan 11, 2018

Handle namespace API listing on the client. apache/openwhisk-cli#191

Merged

dubee reviewed Jan 11, 2018

View reviewed changes

rabbah self-requested a review January 11, 2018 02:00

houshengbo force-pushed the rewrite-WskBasicTests branch from 2b7eee0 to 67b2f51 Compare January 11, 2018 16:05

rabbah reviewed Jan 12, 2018

View reviewed changes

rabbah requested changes Jan 12, 2018

View reviewed changes

rabbah self-assigned this Jan 12, 2018

houshengbo force-pushed the rewrite-WskBasicTests branch 4 times, most recently from eed4114 to c03dbd3 Compare January 12, 2018 21:56

rabbah approved these changes Jan 14, 2018

View reviewed changes

houshengbo force-pushed the rewrite-WskBasicTests branch 2 times, most recently from 599e390 to 63e9aff Compare January 17, 2018 21:11

rabbah reviewed Jan 23, 2018

View reviewed changes

rabbah approved these changes Jan 23, 2018

View reviewed changes

Vincent Hou added 2 commits January 23, 2018 11:16

Reimplementation of WskBasicTests in REST

18f5436

This PR rewrites WskBasicTests.

Use the Akka code, remove the get action for namespace in REST

1e84f61

houshengbo force-pushed the rewrite-WskBasicTests branch from 63e9aff to 1e84f61 Compare January 23, 2018 16:20

rabbah merged commit 469c5a5 into apache:master Jan 24, 2018

BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019

Rewrite wsk basic tests for the REST API (apache#3103)

07e99f4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite wsk basic tests #3103

Rewrite wsk basic tests #3103

houshengbo commented Dec 13, 2017 •

edited

Loading

markusthoemmes commented Dec 14, 2017

markusthoemmes left a comment

markusthoemmes Dec 14, 2017

markusthoemmes Dec 14, 2017

houshengbo Dec 19, 2017

markusthoemmes Dec 14, 2017

houshengbo Dec 18, 2017

dubee left a comment

rabbah Jan 12, 2018

houshengbo Jan 12, 2018

rabbah Jan 12, 2018

houshengbo Jan 12, 2018

rabbah Jan 12, 2018 •

edited

Loading

houshengbo Jan 12, 2018

rabbah Jan 12, 2018 •

edited

Loading

houshengbo Jan 12, 2018

rabbah Jan 12, 2018

houshengbo Jan 12, 2018

rabbah Jan 12, 2018

houshengbo Jan 12, 2018 •

edited

Loading

rabbah left a comment

rabbah commented Jan 17, 2018

houshengbo commented Jan 17, 2018

rabbah Jan 23, 2018 •

edited

Loading

rabbah Jan 23, 2018

Rewrite wsk basic tests #3103

Rewrite wsk basic tests #3103

Conversation

houshengbo commented Dec 13, 2017 • edited Loading

markusthoemmes commented Dec 14, 2017

markusthoemmes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dubee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rabbah Jan 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rabbah Jan 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

houshengbo Jan 12, 2018 • edited Loading

Choose a reason for hiding this comment

rabbah left a comment

Choose a reason for hiding this comment

rabbah commented Jan 17, 2018

houshengbo commented Jan 17, 2018

rabbah Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

houshengbo commented Dec 13, 2017 •

edited

Loading

rabbah Jan 12, 2018 •

edited

Loading

rabbah Jan 12, 2018 •

edited

Loading

houshengbo Jan 12, 2018 •

edited

Loading

rabbah Jan 23, 2018 •

edited

Loading