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

Improve tests #86

Merged
merged 4 commits into from
Nov 2, 2017
Merged

Improve tests #86

merged 4 commits into from
Nov 2, 2017

Conversation

immerrr
Copy link
Contributor

@immerrr immerrr commented Oct 31, 2017

This PR adds pytest-catchlog for reviewing errors sent to logs, e.g. from background threads.

It also adds a rather unique frontier name for each test case so that test case ordering doesn't affect the vcrpy cassettes.

Copy link
Contributor

@vshlapakov vshlapakov left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 👍 I've created a related PR with a couple of minor proposals & cassettes on top of the changes, please take a look and let me know if you want to discuss any of the changes.

@immerrr
Copy link
Contributor Author

immerrr commented Nov 1, 2017

Sweet, I like them, merged your changes into my branch right away :)

@codecov
Copy link

codecov bot commented Nov 1, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   93.26%   93.26%           
=======================================
  Files          28       28           
  Lines        1870     1870           
=======================================
  Hits         1744     1744           
  Misses        126      126

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 c2c94ab...aa2453b. Read the comment docs.

frontier.flush()

assert frontier.newcount == 150 + old_count

# insert repeated fingerprints
fps4 = [{'fp': '/index_%s.html' % fp} for fp in range(0, batch_size)]
Copy link
Contributor

Choose a reason for hiding this comment

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

@immerrr last question about the PR before merge, why the test with repeated fps was changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because fps4 was unused and fps3 were used as "repeated fingerprints" instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, thanks for cleaning it, I didn't spot it even now 🤦‍♂️

@vshlapakov vshlapakov merged commit dca8a49 into master Nov 2, 2017
@vshlapakov vshlapakov deleted the improve-tests branch November 2, 2017 12:15
@vshlapakov
Copy link
Contributor

Thanks again @immerrr! 👍

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.

None yet

2 participants