Report coverage from integration test jobs

[webknjaz]

Summary of changes

$sbj.

Pull Request Checklist

• Changes have tests
• News fragment added in changelog.d/.
  (See documentation for details)

[webknjaz]

Updating this version is a hard prerequisite because the python-version output is only provided since v4.

[webknjaz]

I've triggered the manual run in my fork since it doesn't run on PRs here. And here's the result confirming that this works: https://app.codecov.io/github/webknjaz/setuptools/commit/b24a1f1a61c1cad476558f847ee7cc3066b37aa2.

[abravalheri]

The coverage reports in setuptools and in the need of some love, thank you very much for working on this @webknjaz!

I am having some trouble to interpret the reports, it seems that adding the integration tests to codecov just results in a marginal increment¹...

Do you think this is caused by the fact that the integration tests basically consists of nested subprocesses invoking setuptools? (i.e. calling pip with subprocess, that in turn calls setuptools with a subprocess).

Or is it because we are only traversing code paths that are already exercised with the normal test? (and therefore no increment is expected)

Footnotes

1. I see that the setuptools/config/_validate_pyproject/error_reporting.py file gets some incremental coverage, but I think we can completely ignore the setuptools/config/_validate_pyproject directory, since this code is automatically compiled from JSON schema... ↩

[webknjaz]

@abravalheri no idea.
Per

setuptools/.coveragerc

Line 8 in 48adabe

*/_validate_pyproject/* # generated code, tested in `validate-pyproject`

, it seems to be requested to be skipped... Maybe pytest-cov somehow ignores that config? Maybe there's some env var influencing this? Dunno. Or maybe the unit tests just don't run any code that calls that module and the integration tests do?

I see it is also ignored during the test collection stage in pytest:

setuptools/conftest.py

Line 35 in f54ec14

'setuptools/config/_validate_pyproject',

.
So this means that pytest doesn't import it by itself (creating some coverage with such an import).

I wonder if coveragepy's dynamic context could shed some light on the call stack. Perhaps, show which test invokes it. I don't think this information is recognized/rendered by Codecov, though.
So possibly it'd a good idea to save combined coverage reports as GHA artifacts, as demonstrated in https://hynek.me/articles/ditch-codecov-python/ (while still reporting to Codecov too). If such reports are created with a bundled HTML, we could download them locally and see the context details that coveragepy produces.

[webknjaz]

1. I see that the setuptools/config/_validate_pyproject/error_reporting.py file gets some incremental coverage

Could you clarify where exactly did you see this? If you follow the link I provided (https://app.codecov.io/github/webknjaz/setuptools/commit/b24a1f1a61c1cad476558f847ee7cc3066b37aa2) and click on the Download link for the 3285769568 upload, which is https://storage.googleapis.com/codecov/v4/raw/2022-10-20/126E87BDED6CD66FF85612E5176E1BA6/b24a1f1a61c1cad476558f847ee7cc3066b37aa2/4deed74b-8255-47e5-a143-c1cbfb508a64.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=GOOG1EJOGFN2JQ4OCTGA2MU5AEIT7OT5Z7HTFOAN2SPG4NWSN2UJYOY5U6LZQ%2F20221020%2FUS%2Fs3%2Faws4_request&X-Amz-Date=20221020T140455Z&X-Amz-Expires=300&X-Amz-SignedHeaders=host&X-Amz-Signature=49174098d20f8490cc46444d118677fcba923ed37f152fdc2910ddc56eceb4ac — it opens a preview in browser, you can use Ctrl+F with _validate_pyproject. I can only see it present in the env file list. Which is fine because it just lists things present on disk.
If you scroll down to the XML report part, there are no matches in it.

@abravalheri can it be that you were looking at a wrong place?

[abravalheri]

@abravalheri can it be that you were looking at a wrong place?

Probably 😅. I was looking at https://app.codecov.io/gh/pypa/setuptools/pull/3643.
(It doesn't make a lot of sense to me, the configuration files should skip the _validate_pyproject directory ...).

I wonder if coveragepy's dynamic context could shed some light on the call stack. Perhaps, show which test invokes it. I don't think this information is recognized/rendered by Codecov, though.
So possibly it'd a good idea to save combined coverage reports as GHA artifacts, as demonstrated in https://hynek.me/articles/ditch-codecov-python/ (while still reporting to Codecov too). If such reports are created with a bundled HTML, we could download them locally and see the context details that coveragepy produces.

I have the impression that the increment in coverage that I was expecting is not happening because we run pip install in a subprocess and that command uses setuptools installed in a different directory, but I might be wrong here.

Usually in my personal projects I have a configuration to ensure all the corresponding paths are unified by coverage.py and reported as if they were the same source file:

[paths]
source =
    src/
    */site-packages/

However, since setuptools does not follow a src-layout this kind of config is not applicable...
(Also maybe newer versions of coverage.py already do that automatically and this config is outdated).

[webknjaz]

@abravalheri my impression was that those files weren't used in run-time either, no?
Also, do you have any requests for moving this PR forward?
Currently, the only thing missing is probably enabling Codecov's carryforward flags.

[webknjaz]

@abravalheri hopefully, this will allow Codecov not to report coverage drop on PRs not running integration tests.

• https://docs.codecov.com/docs/github-5-carryforward-flags
• https://docs.codecov.com/docs/flags#carryforward-flags
• https://docs.codecov.com/docs/carryforward-flags

[abravalheri]

@abravalheri my impression was that those files weren't used in run-time either, no?

Leaving the _validate_pyproject topic aside, currently we see that the coverage report does not show any hit in the setuptools/pkg_resources actual package code (aside from the testing files). (I have some experiment in https://github.com/abravalheri/setuptools/actions/runs/3445117022/jobs/5748716471, https://github.com/abravalheri/setuptools/tree/abravalheri/maintenance/gha-integration-codecov)

do you have any requests for moving this PR forward?

What I would like to investigate is how to make coverage correctly report the hits in the setuptools/pkg_resource code. This is probably related to a coverage.py configuration like the one I mentioned in #3643 (comment) and/or https://coverage.readthedocs.io/en/6.2/subprocess.html.

[abravalheri]

In https://github.com/abravalheri/setuptools/tree/abravalheri/maintenance/gha-integration-codecov I did a series of investigations to see if we could have the tests to report actual hits in the setuptools codebase. Unfortunately, I was not very successful.

The idea behind integration tests is the following:

• For each package that we want to make sure is built with the current version of setuptools we create a new virtual environment and install the version of setuptools under-test in it.
• Then we proceed to install all build dependencies and proceed to run:

  pip install --no-build-isolation <sdist>

In theory coverage.py would allow us to unify multiple equivalent paths when reporting coverage. That would mean that we can collect coverage data from the setuptools installed in each virtualenv created during the tests, but report as equivalent to the development source tree.

Also in theory, coverage.py would allow us to run measure coverage from sub-processes (e.g. the calls to pip install during the integration tests). I tried the 2 approaches discussed in coverage.py' docs: (a) using coverage.process_startup() with a .pth file, (b) calling the sub-commands with coverage run.
Unfortunately I could not verify hits in the setuptools implementation files for any of the approaches, and I don't know exactly why.

---

I understand that it is a good practice to report the coverage over test files, but at the same time it would be good if the integration tests would report the hits in the setuptools implementation files.

If anyone is interested in taking the investigation further, that would be very nice.