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 coverage report generation #346

Merged
merged 10 commits into from
Jul 1, 2018
Merged

Add coverage report generation #346

merged 10 commits into from
Jul 1, 2018

Conversation

felixdivo
Copy link
Collaborator

This adds very simple statement coverage reports like this one to the Travis CI & AppVeyor test routines.

It would be nice to also add some tool like Codecov.io. That might help us identify poorly tested regions of code.

Adding coverage tool support via the GitHub Marketplace should be quite straightforward, but I do not have the required admin permissions. @hardbyte Can you do that?

@felixdivo felixdivo added this to the 2.3 Release milestone Jun 29, 2018
@felixdivo felixdivo self-assigned this Jun 29, 2018
Copy link
Owner

@hardbyte hardbyte left a comment

Choose a reason for hiding this comment

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

I like the coverage report.

I can't really comment but just a reminder to try keep changes isolated - the neovi stuff looks fine but is not on the topic of coverage report generation. :-)

setup.py Outdated
]
'pytest-cov ~= 2.5',
] + extras_require['serial']
#for key, requirements in extras_require.items():
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove the commented out lines

setup.py Outdated
@@ -8,6 +8,7 @@
from sys import version_info
import re
import logging
from itertools import chain
Copy link
Owner

Choose a reason for hiding this comment

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

Not used?

return [{
'serial': NeoViBus.get_serial_number(device)
} for device in ics.find_devices()]
except Exception as e:
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if this try/for should be inverted - if the try was around each call to get_serial_number then any working devices can be returned?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made the code a bit clearer to show that only the call to ics.find_devices() is wrapped in a try-catch clause. The method NeoViBus.get_serial_number() itself should never fail.

@hardbyte
Copy link
Owner

I've added the codecov integration and travis should now report coverage... Let's see

@codecov
Copy link

codecov bot commented Jun 30, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@09c236f). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #346   +/-   ##
==========================================
  Coverage           ?   58.66%           
==========================================
  Files              ?       54           
  Lines              ?     4292           
  Branches           ?        0           
==========================================
  Hits               ?     2518           
  Misses             ?     1774           
  Partials           ?        0
Impacted Files Coverage Δ
can/interfaces/ics_neovi/neovi_bus.py 27.9% <100%> (ø)

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 09c236f...cc12faf. Read the comment docs.

@felixdivo
Copy link
Collaborator Author

@hardbyte Well, after changing setup.py Neovi kept crashing, and that simple fix was just there to make the test suite succeed.

@felixdivo felixdivo merged commit 63d57e2 into develop Jul 1, 2018
@felixdivo felixdivo deleted the add-coverage-reports branch July 1, 2018 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants