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

Drop pytest-cov and uses vanilla coverage #809

Merged
merged 5 commits into from
Oct 8, 2020
Merged

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Oct 7, 2020

Prompted by #805

I personally dont see the advantage of using pytest-cov, this PR drops it and uses vanilla coverage.

It solves the creation of those coverage.hostname.xxxx left by pytest-cov that the aforementioned PR wanted to ignore.

Any preferences ?

@euri10 euri10 requested a review from a team October 7, 2020 12:48
setup.cfg Outdated Show resolved Hide resolved
@cdeler
Copy link
Member

cdeler commented Oct 7, 2020

I wonder if it solves the problem with tests debugging under PyCharm. If so it's might be reasonable to copy it to httpcore and httpx (pytest-cov is causing breakpoints to be skipped)

Approved 👍
with a small comment

Copy link
Member

@JayH5 JayH5 left a comment

Choose a reason for hiding this comment

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

I also prefer vanilla coverage over pytest-cov, but don't really have a strong argument for why.

I guess my two main reasons for preferring it are:

  1. pytest-cov implicitly adds a few config options to coverage that are sometimes surprising, like with the multiprocess thing you pointed out
  2. I'd rather learn to use Coverage, a generic (and very well-maintained) tool that is not pytest-specific, than pytest-cov. Using vanilla Coverage with pytest is not generally difficult/complicated.

But I don't think these are super strong reasons so it's perhaps a discussion for the larger encode org to have.

setup.cfg Outdated Show resolved Hide resolved
@cdeler
Copy link
Member

cdeler commented Oct 7, 2020

@JayH5 the reason might be the problem with debugging in IDE I mentioned. For example I'm executing each test with --no-cov which drives me crazy (sorry I cannot copy the link from prev comment as I'm writing from phone)

I meant that I want to be able to run tests under debugging without cov enabled

@florimondmanca
Copy link
Member

I think we're using this alternative "no Pytest-cov" approach in the HTTPCore repo...? There's a separate scripts/coverage script that checks for coverage, and I don't remember that scripts/test had to change much... Or maybe we actually do use Pytest-cov, not sure 😅 But might be worth checking how we're doing it there!

@euri10
Copy link
Member Author

euri10 commented Oct 8, 2020

@JayH5 the reason might be the problem with debugging in IDE I mentioned. For example I'm executing each test with --no-cov which drives me crazy (sorry I cannot copy the link from prev comment as I'm writing from phone)

I meant that I want to be able to run tests under debugging without cov enabled

yep I have a pytest template with --no-cov for every projet using pytest-cov and it's super annoying indeed.

@euri10
Copy link
Member Author

euri10 commented Oct 8, 2020

I think we're using this alternative "no Pytest-cov" approach in the HTTPCore repo...? There's a separate scripts/coverage script that checks for coverage, and I don't remember that scripts/test had to change much... Or maybe we actually do use Pytest-cov, not sure But might be worth checking how we're doing it there!

nope you are using pytest-cov and coverage report, the latter being obviously useless since pytest-cov take care of it

@graingert
Copy link
Member

coverage run -m pytest changes how the sys.path exists when running the tests - eg you'll be testing the source files rather than the installed package

@euri10
Copy link
Member Author

euri10 commented Jun 15, 2021

coverage run -m pytest changes how the sys.path exists when running the tests - eg you'll be testing the source files rather than the installed package

care to expand on this @graingert ?
from pytest-cov docs I can indeed see a reference to sys.path

Consistent pytest behavior. If you run coverage run -m pytest you will have slightly different sys.path (CWD will be in it, unlike when running pytest).

I fail to see however how that translates into testing sources files vs installed package, I see no installation process here.

@graingert
Copy link
Member

@euri10 ah it seems you're not installing uvicorn as part of the tests. That's usually desirable because the source files and the distributed package often have subtle differences

@euri10
Copy link
Member Author

euri10 commented Jun 15, 2021

yes indeed, we may want to change that.

the biggest grip I had on pytest-cov was that I had to put a --no-cov flag everytime I wanted to debug in pycharm, if not set breakpoints were simply skipped.

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.

5 participants