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

Try out coverage instead of pytest-cov #2665

Merged
merged 22 commits into from
Jun 28, 2023
Merged

Conversation

A5rocks
Copy link
Contributor

@A5rocks A5rocks commented Jun 20, 2023

Not sure this will work. Fixes #2645

@A5rocks A5rocks marked this pull request as draft June 20, 2023 23:36
@A5rocks A5rocks requested a review from jakkdl June 21, 2023 00:04
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #2665 (41b9b02) into master (61775d5) will increase coverage by 0.72%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2665      +/-   ##
==========================================
+ Coverage   98.11%   98.84%   +0.72%     
==========================================
  Files         118      114       -4     
  Lines       16539    16514      -25     
  Branches     3003     3003              
==========================================
+ Hits        16227    16323      +96     
+ Misses        254      134     -120     
+ Partials       58       57       -1     
Impacted Files Coverage Δ
trio/_core/_tests/test_multierror.py 100.00% <100.00%> (ø)

... and 12 files with indirect coverage changes

@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 21, 2023

#2664 has the fix for the Ubuntu 3.11 failure.

@A5rocks A5rocks marked this pull request as ready for review June 21, 2023 00:18
Copy link
Member

@jakkdl jakkdl left a comment

Choose a reason for hiding this comment

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

Nice, the code in testing/ is showing up as covered as expected!

Seems like we've lost coverage of test_multierror_scripts since they're run with subprocess.run. But looks like there's documentation on how to fix that: https://coverage.readthedocs.io/en/7.2.7/subprocess.html
Is the drop in trio/_core/_multierror.py also because of that?

The drop in trio/_tests/test_exports.py seems to be the test timeout on 3.12 so that's not an issue.

@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 21, 2023

Wait a second, trio/_core/_tests/test_multierror_scripts/_common.py literally does the thing coverage.py recommends doing. It still doesn't work. What??

@A5rocks A5rocks marked this pull request as draft June 21, 2023 22:41
@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 22, 2023

of course the thing that literally reads the file is not going to work. why did i miss that...

@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 22, 2023

welp no clue why the coverage for a few more files isn't working. i give up for now.

@A5rocks A5rocks marked this pull request as ready for review June 22, 2023 00:49
@jakkdl
Copy link
Member

jakkdl commented Jun 23, 2023

welp no clue why the coverage for a few more files isn't working. i give up for now.

I suppose we can just exclude trio/_core/_tests/test_multierror_scripts/* from coverage and leave fighting with this for some future crazy person. After that it looks fine for merging!

@A5rocks A5rocks requested a review from jakkdl June 27, 2023 00:29
Add leading wildcard to the omit path, in case that makes a difference
@jakkdl
Copy link
Member

jakkdl commented Jun 28, 2023

Huh, there's four codecov checks? :o

Anyway, it looks good now - merging~~

@jakkdl jakkdl merged commit afd51d1 into python-trio:master Jun 28, 2023
33 checks passed
@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 28, 2023

Huh, there's four codecov checks? :o

... weird. Eh, we can fix that eventually!

@A5rocks A5rocks deleted the use-coverage branch June 28, 2023 14:39
@A5rocks
Copy link
Contributor Author

A5rocks commented Jun 28, 2023

Ok, the four codecov thing was just cause this was an old PR: #2674 only has 2 checks! (as it should be)

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.

use coverage instead of pytest-cov
2 participants