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

feat: support coverage>=5.0 #214

Merged
merged 5 commits into from Dec 31, 2019
Merged

feat: support coverage>=5.0 #214

merged 5 commits into from Dec 31, 2019

Conversation

TheKevJames
Copy link
Owner

I can't say I'm excited about this code, but at the very least this should be fully backwards compatible with how coverage<5 + coveralls work together. There's definitely going to need to be some future work to clean this up, but hopefully this should be enough to get folks unblocked from using coverage>=5.0.

@TheKevJames
Copy link
Owner Author

@nedbat looks like I'm getting some errors about mixing coverage versions' .coverage files (eg. see this CI run). Theoretically, this issue only affects me since I'm explicitly matrix testing coverage versions, but... any thoughts on ways I might be able to merge those results?

@@ -101,7 +189,14 @@ def get_arcs(analysis):
if not analysis.has_arcs():
return None

branch_lines = analysis.branch_lines()
if not hasattr(analysis, 'branch_lines'):
# N.B. switching to the public method analysis.missing_branch_arcs
Copy link
Owner Author

Choose a reason for hiding this comment

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

@nedbat is there anything I missed in the public coverage v5+ API which would support this? Specifically, its the following block that I couldn't work out how to do without private APIs:

for l1, l2 in analysis.arcs_executed():
    if l1 in branch_lines:
        # SNIP

Copy link

Choose a reason for hiding this comment

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

@TheKevJames let's talk about this in another place. Clearly the Coverage public API doesn't give you everything you need yet. You called out this spot, but looking at the rest of the file, there are many things you are using from coverage.py that I didn't anticipate you would need.

I've added an issue for coverage.py, let's discuss the details there: nedbat/coveragepy#921

@TheKevJames TheKevJames force-pushed the coverage-5 branch 6 times, most recently from 1c4f723 to 5890f72 Compare December 31, 2019 02:58
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