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

Add hypothesis tests #532

Merged
merged 2 commits into from
Sep 12, 2017
Merged

Add hypothesis tests #532

merged 2 commits into from
Sep 12, 2017

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Sep 11, 2017

This adds initial support for hypothesis tests. I've seeded this
with a couple of tests for app.py. In addition I've added
configuration such that more iterations of these tests are run on
travis.

Originally mentioned in #506 (comment).

This adds initial support for hypothesis tests.  I've seeded this
with a couple of tests for app.py.  In addition I've added
configuration such that more iterations of these tests are run on
travis.
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks fine to me assuming the travis job passes. Out of curiosity what is the reasoning for splitting this out to another directory instead of putting it under something like under the unit test directory? There is not really a precedence in any of the other aws or boto org projects. So just want to make sure this is precedence we want to set if we expand hypothesis testing to the rest of the projects.

@codecov-io
Copy link

codecov-io commented Sep 12, 2017

Codecov Report

Merging #532 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #532   +/-   ##
=======================================
  Coverage   94.39%   94.39%           
=======================================
  Files          18       18           
  Lines        3107     3107           
  Branches      397      397           
=======================================
  Hits         2933     2933           
  Misses        126      126           
  Partials       48       48

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8bd8c4...f1dda71. Read the comment docs.

@jamesls
Copy link
Member Author

jamesls commented Sep 12, 2017

Capturing offline discussion. It seems most projects don't separate property tests from their other tests, and the only reason I've done it this way in other projects is because I didn't also have to handle functional, integration tests etc.

So I'll:

  • Add this into tests/unit/test_app.py
  • Set the default iterations to really low so we still get fast unit tests time.
  • Keep the high iteration count on the CI profile.

Based on review feedback.  This also makes it easier to add
property tests as functional and integration tests.

I've set the default number of iterations to 10 by default.
On travis, we bump that up to 2000 via the HYPOTHESIS_PROFILE
env var.
@jamesls
Copy link
Member Author

jamesls commented Sep 12, 2017

@kyleknap ready to look at again.

Copy link
Contributor

@kyleknap kyleknap 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. 🚢

Copy link
Contributor

@stealthycoin stealthycoin 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 to me.

@jamesls jamesls merged commit f1dda71 into aws:master Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants