[GitHub] [incubator-openwhisk] falkzoll commented on issue #4450: Fix test cases to also work with nodejs:10 as default nodejs runtime.

[GitBox <gi...@apache.org>]

falkzoll commented on issue #4450: Fix test cases to also work with nodejs:10 as default nodejs runtime.
URL: https://github.com/apache/incubator-openwhisk/pull/4450#issuecomment-488656336
 
 
   From the functional point of view, this PR is now ready for merge. All tests are successful and the testcases also work when nodejs:10 is the default nodejs runtime.
   
   Most comments have been addressed.
   Except of moving the check of the exec annotation into a subroutine. This is the check:
   ```
         .convertTo[Seq[JsObject]]
         .find(_.fields("key").convertTo[String] == "exec")
         .map(_.fields("value"))
         .map(exec => { exec.convertTo[String] should startWith("nodejs:") })
         .getOrElse(fail())
   ```
   The checks (at 7 places) are in multiple classes so we would need something outside to them. I tried to find a good place for such a common subroutine for the tests, but I was not really successful. But I am not that deep in openwhisk, yet, and my Scala skill is not at expert level... yet :-). 
   Therefore hints and tricks are highly welcome :-).
   
   Depending on how important it is to merge this PR right now... I could also follow up on the move of this exec annotation check into a common subroutine in a separate PR?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services