Skip to content

Conversation

@upkarlidder
Copy link
Contributor

@blumareks can you please review and let me know if I am missing any tests? I added the following

  • function should not return null
  • function should return JsonObject type
  • function should return { greeting: "Hello World!"} when called with empty args
  • function should return { greeting: "Hello upkar!"} when called with {name: "upkar"}

Copy link
Contributor

@blumareks blumareks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidderupk I think we need to add quotes in both fields - at least this is what I get from Swift:

Activation ID:
fb8041e02b484c4a8041e02b48ac4a40
Results:
{
  "message": "Hello World!"
}
Logs:
[]
  • in addition there might be a timeout test condition of 60 seconds. So in other words if you send something and there is no response in 60 sec (or less) it means the service is down.

} else {
result = "Hello " + nameArg.getAsString();
}
response.addProperty("greeting", result);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blumareks I do have quotes. I forgot to add in the comments :).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@upkarlidder
Copy link
Contributor Author

Thanks @blumareks

For in addition there might be a timeout test condition of 60 seconds.: is this a unit test that I should add in the test class? What will this look like? This is a limit set by OpenWhisk.

@upkarlidder
Copy link
Contributor Author

@blumareks , can you please review and merge? Thank you 🙏

Copy link
Contributor

@blumareks blumareks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed - looks good!

Copy link
Contributor

@blumareks blumareks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@blumareks
Copy link
Contributor

blumareks commented Jan 9, 2020

@lidderupk I reviewed and accepted. Please merge. I do not have a button here for that... At least I do not see it. I know what it is. I do not have WRITE ACCESS:
Screen Shot 2020-01-09 at 8 11 15 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants