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

Move verbose and coverage options for pytest back to ci script #78

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vxgmichel
Copy link
Contributor

@vxgmichel vxgmichel commented Jan 14, 2019

Address the point made in this comment by @njsmith (PR #76):

The full options there are good for CI where you can't easily re-run tests, but not what I normally like for local runs...

@codecov
Copy link

codecov bot commented Jan 14, 2019

Codecov Report

Merging #78 (7515975) into master (c4e1fe3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #78   +/-   ##
=======================================
  Coverage   91.28%   91.28%           
=======================================
  Files          16       16           
  Lines         574      574           
=======================================
  Hits          524      524           
  Misses         50       50           

Copy link
Member

@altendky altendky left a comment

Choose a reason for hiding this comment

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

I am fine, maybe even prefer, for this PR to stay focused on what it is now but will note that it might be worth a full catch up with https://github.com/python-trio/trio itself for the CI setup. Cookie cutter... does a thing but leaves us finding and fixing bugs repeatedly in the various projects.

@@ -1,2 +1,2 @@
[pytest]
addopts = -W error -ra -v --pyargs pytest_trio --verbose --cov
addopts = -W error --pyargs pytest_trio
Copy link
Member

Choose a reason for hiding this comment

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

I would personally like to see the --pyargs pytest_trio go away too as it keeps me from running individual tests such as when trying to debug with breakpoints in PyCharm. At which point we are left with -W error and could just move everything to CI and drop this file. I think this is where https://github.com/python-trio/trio/ itself is at.

For that matter, the --cov (that you already moved) broke breakpoints entirely in PyCharm too.

https://youtrack.jetbrains.com/issue/PY-20186

ci/travis.sh Outdated
@@ -92,7 +92,7 @@ else
mkdir empty
cd empty

pytest
pytest -r a --verbose --cov --cov-config=../.coveragerc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pytest -r a --verbose --cov --cov-config=../.coveragerc
# We have to copy .coveragerc into this directory, rather than passing
# --cov-config=../.coveragerc to pytest, because codecov.sh will run
# 'coverage xml' to generate the report that it uses, and that will only
# apply the ignore patterns in the current directory's .coveragerc.
cp ../.coveragerc .
pytest -r a --verbose --cov

python-trio/trio@02066df
altendky/qtrio@f9d2d1c

Though I'll note that I would personally rather just have a src/ directory rather than creating empty/ and copying all the config files around. python-trio/trio#274 has some related commentary against src/.

@pquentin
Copy link
Member

pquentin commented Jan 6, 2021

Rebased on top of master to get GitHub Actions CI.

@CoolCat467
Copy link
Member

Fixed merge conflicts

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.

4 participants