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

Coverage calculation accounting for concurrency #566

Merged
merged 1 commit into from
Aug 29, 2018

Conversation

AnthonyHorton
Copy link
Collaborator

While working on #547 I ran into problems with inaccurate test coverage reports because the existing coverage and Travis CI configurations produced results which did not take into account concurrency, in particular scripts started with subprocess. I was able to come up with some changes to the configurations that appear to resolve these problems, and should properly calculate test coverage involving multiprocessing, threading and subprocess concurrency. At @wtgee's suggestion I have split these config changes out into a Pull Request of their own.

@codecov
Copy link

codecov bot commented Aug 28, 2018

Codecov Report

Merging #566 into develop will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     #566     +/-   ##
==========================================
+ Coverage    70.44%   70.55%   +0.1%     
==========================================
  Files           64       64             
  Lines         5518     5518             
  Branches       767      767             
==========================================
+ Hits          3887     3893      +6     
+ Misses        1425     1420      -5     
+ Partials       206      205      -1
Impacted Files Coverage Δ
pocs/camera/camera.py 90.81% <0%> (+1.08%) ⬆️
pocs/serial_handlers/protocol_arduinosimulator.py 77.3% <0%> (+1.53%) ⬆️

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 a88420f...cdb04fe. Read the comment docs.

Copy link
Member

@wtgee wtgee left a comment

Choose a reason for hiding this comment

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

LGTM but was almost expecting a huge jump in coverage. :) Will be nice to be more accurate. Will leave for @jamessynge to see if he has comments.

@AnthonyHorton
Copy link
Collaborator Author

AnthonyHorton commented Aug 28, 2018

@wtgee It would make more difference if $POCS/scripts/*.py were included in the coverage calculation! Python scripts launched by tests via subprocess are properly counted with this set up, which is the main difference I was after.

Copy link
Contributor

@jamessynge jamessynge left a comment

Choose a reason for hiding this comment

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

I don't have an opinion yet as to how well it works, but would like to try!
I am surprised that we don't see more of a jump, but I'll take a look once merged and I can try it on my laptop.

@wtgee wtgee merged commit f1e9775 into panoptes:develop Aug 29, 2018
@AnthonyHorton AnthonyHorton deleted the coverage-concurrency branch August 29, 2018 05:02
@jamessynge
Copy link
Contributor

I'm looking forward to trying this out, and hopefully getting some improved coverage.

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.

3 participants