add option to generate test coverage report without codecov

[zacharyburnett]

using the guide here: https://hynek.me/articles/ditch-codecov-python/

This does not create coverage diffs or report to PRs, just creates a markdown summary table

closes #189

EDIT: here's what the summary table looks like:

[Cadair]

I tried this out on sunpy and it seems to explode. https://github.com/sunpy/sunpy/pull/7668

[Cadair]

It seems that it's not picking up the coverage.xml to upload here: https://github.com/sunpy/sunpy/actions/runs/9445798095/job/26014155532?pr=7668#step:14:16 :thinking:

[pllim]

tl;dr -- this does not generate patch coverage, right?

[Cadair]

Not yet, but I found a way to make it.

[zacharyburnett]

tl;dr -- this does not generate patch coverage, right?

Not yet, but I found a way to make it.

For patch coverage we'd need to store the coverage from main somewhere, to compare it to; some people I've seen online have done that by storing it in a hidden page in the repository's wiki, reading and writing it with a github token with ${{ secrets.GITHUB_TOKEN }}

[pllim]

I am trying this out too at https://github.com/spacetelescope/acstools/pull/205 and it still crashes:

https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26068438067?pr=205

[zacharyburnett]

I am trying this out too at spacetelescope/acstools#205 and it still crashes:

https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26068438067?pr=205

oh whoops, I fixed the artifact issue:

but didn't pass any filenames to coverage combine 😅

[pllim]

Getting late here. @zacharyburnett , feel free to restart the CI over at https://github.com/spacetelescope/acstools/pull/205 to pick up new changes from this PR. Thanks!

[pllim]

Still not working

Run mv .coverage .coverage.ubuntu-latest-py39-test-alldeps
mv: cannot stat '.coverage': No such file or directory

[pllim]

Ah, looks like the test failed, so no coverage file was written. Maybe have the mv command skip if no coverage file is found.

[pllim]

Even when CI passes, it failed with the same error.

Coverage XML written to file coverage.xml

mv: cannot stat '.coverage': No such file or directory

[zacharyburnett]

Even when CI passes, it failed with the same error.

Coverage XML written to file coverage.xml

mv: cannot stat '.coverage': No such file or directory

it should be uploading / downloading the .coverage database, to be able to combine multiple machine types, so if that's not present then something is messed up

[zacharyburnett]

it looks like --cov-report=xml is what's creating the coverage.xml; however, it should still be creating a .coverage database file... I'll so some local testing to see what files it spits out https://github.com/spacetelescope/acstools/actions/runs/9463279566/job/26118526244#step:10:105

pytest --pyargs acstools --cov-report=xml --cov=acstools /home/runner/work/acstools/acstools/doc --cov --remote-data -v

[zacharyburnett]

running pytest --pyargs acstools --cov-report=xml --cov=acstools --cov --remote-data -v locally does appear to create the .coverage file:

❯ ls -a
.                           CITATION.md
..                          CODE_OF_CONDUCT.md
.bandit.yaml                        LICENSE.md
.coverage                       MANIFEST.in
.coverage.urelialia2022.local.stsci.edu.35749.XCMKciAx  README.rst
.git                            __pycache__
.github                         acstools
.gitignore                      acstools.egg-info
.mypy_cache                     conftest.py
.pytest_cache                       coverage.xml
.readthedocs.yaml                   doc
.tmp                            pyproject.toml
.tox                            tox.ini

[pllim]

What is the difference between .coverage and coverage.xml?

[zacharyburnett]

What is the difference between .coverage and coverage.xml?

I'm not intimately familiar with it, but as far as I know .coverage is a SQLite database file with support for parallel-mode (storing coverage for multiple machine configurations / runs) while coverage.xml is report-focused and so is expected to only contain results from a single run.

We could use coverage.xml, but then we'd need to have a summary table of coverage for each and every run, instead of a single table collating all runs

[pllim]

Tried to debug at #209 but no luck. I see the coverage ran but the the upload failed: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26134042811?pr=205

[zacharyburnett]

Tried to debug at #209 but no luck. I see the coverage ran but the the upload failed: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26134042811?pr=205

could you also add -a to the ls?

[pllim]

Ah, yes, added. New log: https://github.com/spacetelescope/acstools/actions/runs/9483014271/job/26138000458?pr=205

[zacharyburnett]

testing done at https://github.com/spacetelescope/stpipe/actions/runs/9618208358

[pllim]

I see error in https://github.com/spacetelescope/stpipe/actions/runs/9618208358/job/26531763051 (despite the green status). Is this expected?

##[error]Process completed with exit code 1.

[zacharyburnett]

I see error in https://github.com/spacetelescope/stpipe/actions/runs/9618208358/job/26531763051 (despite the green status). Is this expected?

##[error]Process completed with exit code 1.

yes it should be green; that run has no coverage at all, to make sure the coverage report step doesn't fail due to having no files to combine

[pllim]

I think @astrofrog mentioned elsewhere that would be nice if there can be tests for this, but not sure how to add them.

[pllim]

BTW I think this fix #189 but only a maintainer can add that to the original post for auto-close to take effect. cc @Cadair

[pllim]

@zacharyburnett is this ready for re-review or you still working on it? Thanks!

[zacharyburnett]

@zacharyburnett is this ready for re-review or you still working on it? Thanks!

This is ready now, thanks!

BTW I think this fix #189 but only a maintainer can add that to the original post for auto-close to take effect. cc @Cadair

Oh I didn't even see that 😅

[pllim]

Alas, still cannot get this to work with acstools. What am I doing wrong there?

https://github.com/spacetelescope/acstools/pull/205

https://github.com/spacetelescope/acstools/actions/runs/9669917380/job/26677517727?pr=205

[zacharyburnett]

Alas, still cannot get this to work with acstools. What am I doing wrong there?

spacetelescope/acstools#205

https://github.com/spacetelescope/acstools/actions/runs/9669917380/job/26677517727?pr=205

I'm not sure, this seems to be correct but the .coverage file isn't being created (only coverage.xml)

[pllim]

A lot of CI only outputs coverage.xml. I think this case also has to be handled.

[zacharyburnett]

A lot of CI only outputs coverage.xml. I think this case also has to be handled.

It was my belief that coverage always creates a .coverage file and uses this as the base data file, and coverage.xml is an additional report file generated with coverage xml or --cov-report=xml; however, I guess that's wrong

EDIT: wait a second, I just cloned acstools and ran tox -e py312-test-alldeps -- --cov on my machine, and it created the .coverage file along with coverage.xml. Now I am very confused why it creates it locally but not on GitHub Actions... 😓

[zacharyburnett]

@pllim would you mind rerunning that job now that 1f4a851 is pushed? It should pick it up

[pllim]

Here is the updated error log https://github.com/spacetelescope/acstools/actions/runs/9669917380/job/26717352610?pr=205

[pllim]

Maybe the coverage stuff is inside .tox

[zacharyburnett]

Maybe the coverage stuff is inside .tox

ah! You're using pyargs; perhaps that is causing this issue by moving the working dir to somewhere else

that might explain why it worked locally on my machine; perhaps there's some shell shenanigans going on with github actions

[pllim]

astropy also uses pyargs, so it is not uncommon. See https://github.com/astropy/astropy/blob/5519605a3b6ec71e9ddb4c6827b8ac1658647f58/tox.ini#L121-L122

[zacharyburnett]

looks like that fixed it https://github.com/spacetelescope/acstools/actions/runs/9683845227/attempts/1#summary-26720539461 thanks for being so patient! 😄

[zacharyburnett]

the failing test_conda test on macOS is fixed by https://github.com/OpenAstronomy/github-actions-workflows/pull/123

[pllim]

Awesome, thanks! Now that the simple case of acstools works, I plan to open an experimental PR to astropy with this PR branch, or feel free to do it if you have time now as I cannot until later today.

[zacharyburnett]

Awesome, thanks! Now that the simple case of acstools works, I plan to open an experimental PR to astropy with this PR branch, or feel free to do it if you have time now as I cannot until later today.

sure thing, here: https://github.com/astropy/astropy/pull/16619

I don't have permissions to label that PR though

[pllim]

@astrofrog , do you want to take this for a spin on your repos?