Skip to content
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

tests(misc): add unit tests #237

Merged
merged 2 commits into from
Aug 15, 2019
Merged

tests(misc): add unit tests #237

merged 2 commits into from
Aug 15, 2019

Conversation

callenmacphee
Copy link
Contributor

Add unit tests to improve test coverage.

Improvements

  • Adds unit tests.

@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

Copy link
Contributor

@paulineribeyre paulineribeyre left a comment

Choose a reason for hiding this comment

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

good idea testing these, but what do you think about creating a single test test_index_list_invalid_param: where you test all of these:
"limit must be an integer", "limit must be between 0 and 1024", "size must be an integer" and "size must be > 0"
Also i think these are duplicates so we should remove one. i think the actual min limit is 0 https://github.com/uc-cdis/indexd/blob/2.0.1/indexd/index/blueprint.py#L67 https://github.com/uc-cdis/indexd/blob/2.0.1/indexd/index/blueprint.py#L112

tests/test_client.py Outdated Show resolved Hide resolved
tests/test_client.py Outdated Show resolved Hide resolved
Copy link
Contributor

@paulineribeyre paulineribeyre left a comment

Choose a reason for hiding this comment

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

awesome :)

@callenmacphee callenmacphee merged commit f134c3a into master Aug 15, 2019
@callenmacphee callenmacphee deleted the test/improve-coverage branch August 15, 2019 17:15
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