Skip to content

Conversation

@hugovk
Copy link
Member

Makes it easier to test locally. Also use it in GitHub Actions, this means the tox.ini file itself is tested.

Some example commands:

# Run on all Python versions listed in the tox file# Missing versions are skipped tox # Run all in parallel (faster) tox -p auto # Run on a single Python version tox -e py311 # Run on multiple Python versions tox -e py38,py311

I left Python 3.12 out for now, because we can't yet test against the betas (#330 (comment)) and if we include it, then plain tox will try and run it and fail. We can still test on demand with tox -e py312.

I also formatted tox.ini using https://github.com/tox-dev/tox-ini-fmt (we could add that to a pre-commit hook).

@hugovk
Copy link
MemberAuthor

Okay, another thing blocked until we can move to 3.12 beta :)

Using tox to just {envpython} --version gives TypeError: rmtree() got an unexpected keyword argument 'onexc' on all alphas and not beta 1:

image

https://github.com/hugovk/blurb_it/actions/runs/5154209675

@hugovkhugovk marked this pull request as draft June 2, 2023 09:54
@AlexWaygood
Copy link
Member

Okay, another thing blocked until we can move to 3.12 beta :)

Using tox to just {envpython} --version gives TypeError: rmtree() got an unexpected keyword argument 'onexc' on all alphas and not beta 1:

Same issue that I had in python/bedevere#547 (comment)!

@hugovkhugovk marked this pull request as ready for review August 15, 2023 20:05
@hugovk
Copy link
MemberAuthor

Close/re-open in case it helps the CLA bot.

@hugovkhugovk closed this Aug 15, 2023
@hugovkhugovk reopened this Aug 15, 2023
@hugovk
Copy link
MemberAuthor

Updated to call pytest to measure/report coverage of all module and test code.

Similar to cherry_picker, and more accurately reports the current situation:

Before

Name Stmts Miss Branch BrPart Cover ------------------------------------------------------ blurb_it/error.py 2 0 0 0 100% blurb_it/util.py 36 15 2 0 55% tests/__init__.py 0 0 0 0 100% tests/test_util.py 73 44 6 0 39% ------------------------------------------------------ TOTAL 111 59 8 0 45% 

After

Name Stmts Miss Branch BrPart Cover ---------------------------------------------------------- blurb_it/__main__.py 134 134 35 0 0% blurb_it/error.py 2 0 0 0 100% blurb_it/middleware.py 13 13 4 0 0% blurb_it/util.py 36 15 2 0 55% tests/__init__.py 0 0 0 0 100% tests/test_util.py 73 44 6 0 39% ---------------------------------------------------------- TOTAL 258 206 47 0 18% 

Comment on lines +5 to +9
--cov blurb_it
--cov tests
--cov-report html
--cov-report term
--cov-report xml
Copy link
Member

@AlexWaygoodAlexWaygoodOct 9, 2023

Choose a reason for hiding this comment

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

The disadvantage of putting this config in pytest.ini rather than .coveragerc is that running pytest locally will now run pytest under coverage -- there's now no way to "just run pytest without a coverage report". Whereas if the config was in .coveragerc, we could run pytest locally to run the tests, and coverage run -m pytest to run the tests under coverage.

The tests in this repo are (at the moment) extremely fast, however, so there's not really any reason to not run tests under coverage. So I'm okay with this!

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

I don't use tox much myself, but this LGTM! The coverage config changes are a big improvement -- they highlight that we currently have a lot of code in __main__.py that isn't being exercised by our tests at all.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@hugovkhugovk merged commit ba33c1a into python:mainOct 9, 2023
@hugovkhugovk deleted the add-tox.ini branch October 9, 2023 15:33
Sign up for freeto 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.

2 participants

@hugovk@AlexWaygood