search

beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.78k stars 1.82k forks source link

Improve GitHub build output and configure coverage #5220

Closed snejus closed 5 months ago

snejus commented 5 months ago

Description

pytest

isort and black

Coverage

Add pytest-cov which automatically measures coverage while tests are executing and update tox and GitHub CI configuration accordingly.

Configuration

  1. Enable analysing logical branches
  2. Save coverage details as HTML for local development use
  3. Configure the HTML output to include coverage context: on the right-hand side of each covered code line there is an expandable section which lists every test that runs/covers this line. See below image
snejus commented 5 months ago

Failed isort check image

snejus commented 5 months ago

Thanks! I left two small questions.

Have you tested whether the diff reporting works as expected (i.e. just force-push some intentionally broken code to this PR)?

Good point, forgot to do it!

snejus commented 5 months ago

I've noticed that the coverage is not getting uploaded. I wonder whether this has to do with the fact that this PR is coming from the fork, and potentially CODECOV_TOKEN is not available in the build?

I generated coverage for the last couple of commits on master and uploaded it upstream, so you can have a glance here

snejus commented 5 months ago

That's the output from failed black check image

snejus commented 5 months ago

And finally, failed tests are now appearing at the end of the output image

wisp3rwind commented 5 months ago

I've noticed that the coverage is not getting uploaded. I wonder whether this has to do with the fact that this PR is coming from the fork, and potentially CODECOV_TOKEN is not available in the build?

Not sure, I don't have access to the repo settings, so can't check whether the token is in fact set. In any case, according to https://docs.codecov.com/docs/adding-the-codecov-token#github-actions, we would need to explicitly use the token in the CI config.

Maybe let's merge this PR, check whether things work, and if required follow up with a small fixup PR?

In any case: Looks good to me, and thanks again! (In fact, I remember adding the failed tests output back when we used nosetest, but it seems that this got lost in the transition to pytest.)

wisp3rwind commented 4 months ago

This breaks local tox runs for me, since pytest seems to not recognize the --cov... arguments. @snejus does this work for you?

arogl commented 4 months ago

This breaks local tox runs for me, since pytest seems to not recognize the --cov... arguments. @snejus does this work for you?

I had to locally install pip install pytest-cov.

@wisp3rwind rerun setup for test @snejus added it to the config.

snejus commented 4 months ago

You can run pip install -e .[test] in your virtual environment

snejus commented 4 months ago

And then potentially run the tests directly without tox 😈