Skip to content

Test api calls #746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 22, 2018
Merged

Test api calls #746

merged 7 commits into from
Mar 22, 2018

Conversation

jjlljj
Copy link
Contributor

@jjlljj jjlljj commented Mar 21, 2018

Question Response
Version? v1.4.1
Devices tested? iPhone 7...
Bug fix? no
New feature? no
Includes tests? yes
All Tests pass? yes
Related ticket? #597

Screenshots

No visual changes

Description

Added API call testing directory in __tests___/tests/ . Added API test index.js file. Added tests for v3 methods: call, parameters, count, and delete.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.8%) to 47.041% when pulling 2b424cc on soytjan:test-api-calls into 986120c on gitpoint:master.

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

This is great, thank you @jjlljj @etcetera8 @soytjan

Deferring to @machour for a second glance since he's been modifying our API logic - but all in all more test coverage around our API wrappers is always a good idea :)

"contributions": [
"code",
"test"
]
Copy link
Member

Choose a reason for hiding this comment

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

Welcome to the family everyone 😊

@machour machour merged commit d267258 into gitpoint:master Mar 22, 2018
@machour
Copy link
Member

machour commented Mar 22, 2018

Nice team work!

@machour
Copy link
Member

machour commented Mar 23, 2018

Just a tiny little thingy that may interest you: When upgrading to last jest version, I had to remove the await keyword from the second test, cause expect() needed a promise, not its resolved value:

f6d2246

@andrewda andrewda mentioned this pull request Dec 1, 2018
63 tasks
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.

5 participants