[CI] Improve coverage config #3050

abravalheri · 2022-01-25T15:14:22Z

Summary of changes

Currently there is a lot of noise and problems when running coverage in the CI.
This is an attempt to improve the configuration and clean the coverage output.

Closes

Pull Request Checklist

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

.github/workflows/main.yml

abravalheri · 2022-01-25T15:15:51Z

.coveragerc

 omit =
 	# leading `*/` for pytest-dev/pytest-cov#456
-	*/.tox/*
+	*/_vendor/*
+	*/_distutils/*


Currently most of _distutils shows up as 0 hit, so it seems to me that omitting is the best, but I might be wrong...

My guess is it shows up as not being hit because of the name mismatch (setuptools._distutils versus distutils). I do try to avoid these snowflake configurations, but I also recognize they may be needed.

@abravalheri I wonder if just Codecov needs to be configured not to take it into account vs the underlying coveragepy.

Thank you @jaraco and @webknjaz for the review.

I wonder if just Codecov needs to be configured not to take it into account vs the underlying coveragepy.

My approach would be trying to solve things with coveragepy first, because on my personal machine I rely on the terminal report which is very useful. Any Codecov change would just be useful after a PR is made...

abravalheri · 2022-01-25T15:50:00Z

.coveragerc

+	pkg_resources/
+	*/site-packages/pkg_resources/
+	*/pip-install-*/setuptools_*/pkg_resources/
+	*/tmp*/pkg_resources


The combination of the project layout and the kinds of tests we run is rather unique (2 top-level packages, in-tree tests and flat-layout, tests using pip-run), which require a bespoke path configuration...

For most of packages using src-layout, the following configuration is enough:

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

The equivalent for flat-layout is:

[paths] source = ../ # seems to be resolved against the `.coveragerc` file path, not the current folder */site-packages/

... but coverage does not allow specifying wildcards as the last segment in [paths], therefore we need an entry for each setuptools and pkg_resources.
*/pip-install-* paths seem to be created by pip-run and */tmp* paths are created when installing into directories created with the tempfile module.

This makes me sad. I've noticed recently (3-6 months ago) in my other projects that they too have started reporting coverage (or errors measuring coverage) on files outside the source tree. Making this change here implies adding a similar but unique change to every other project.

What I'd really like to see is some automation in pytest-cov or coverage to automatically detect the files in the local repo and use those files to determine where to measure coverage.

I've filed jaraco/skeleton#56 to track the broader issue. Can we start by investigating the issue in a simpler project like jaraco/tempora and solve other meta issues that affect other projects in the skeleton before applying solutions that are setuptools-specific and add more maintenance burden to this project?

How about reporting this to @nedbat as an upstream FR?

Thank you very much for the comment guys.

I spent quite a lot of time the past week trying to investigate on how to make this configuration as generic as possible, experimenting with several projects, but unfortunately I could not find that "one configuration to rule them all".

There seem to be a lot of factors that can influence the report, for example:

flat vs src-layout

In-package tests vs separated tests folder

Usage of develop=True in tox.ini

Usage of isolated_build=True in tox.ini

Usage of changedir in tox.ini

Usage of --installpkg flag with tox

coverage vs pytest-cov

Some things I have noticed:

When using a flat layout and develop=True, omitting the [paths] seems to work OK. However that does not work well with src-layout...

When I was experimenting with tempora, for some reason, using --cov alone seems to cause coverage warnings, while using --cov tempora results in a cleaner output, regardless of the presence of [run] source = tempora in .coveragerc.

So far the best approach I did manage to find (less noisy) is to use [run] source = ..., [paths] and still pass in the command line --cov=PACKAGE_NAME instead of just --cov. That seems to work for a broad combination of the factors I described previously, but it does need the name of the package under test to be hardcoded in some configuration files...

.coveragerc

abravalheri · 2022-02-08T18:37:09Z

Thank you very much @jaraco and @webknjaz for the kind reviews. I tried to address some straight-forward changes as proposed, but others are a bit more complicated.

In the latest commits, I tried to simplify the configuration by removing the [paths] section I had previously added. In general this section is important for testing installed packages, but since setuptools is tested via usedevelop=True, it should not impact too much (the reason why I added it in the first place was because I was doing some experiments to remove usedevelop=True).

Unfortunately I did not find a way of not specifying explicitly the package name in the configuration and still obtain clean output (without explicitly specifying the package names there is a lot of warnings and noise in the output).

webknjaz

I guess it's ready now...

Omit path equivalence to see if logs are still usable.

abravalheri · 2022-02-18T16:25:28Z

.coveragerc


 [report]
 show_missing = True
+skip_covered = True


This configuration is more appropriate for large projects such as setuptools, as indicated in #3050 (comment)

abravalheri · 2022-02-18T16:40:16Z

.coveragerc

 omit =
 	# leading `*/` for pytest-dev/pytest-cov#456
-	*/.tox/*
+	*/_vendor/*
+	*/_distutils/tests/*


setuptools/_distutils/tests are not being run in the setuptools CI. So I think it make sense to omit them.

agronholm · 2022-03-25T10:40:41Z

Shouldn't this configuration be merged into pyproject.toml?

jaraco · 2022-03-26T21:55:30Z

Shouldn't this configuration be merged into pyproject.toml?

I haven't yet had a chance to write a blog post about it. Combining all the config into a single file is problemmatic because it conflates concerns and increases the possibility of conflicts as projects share config and have divergent concerns. Let's keep the concerns separated for now. If you think the files should be combined, please file the report at jaraco/skeleton and I can discuss it further there.

abravalheri commented Jan 25, 2022

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

abravalheri commented Jan 25, 2022

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

abravalheri commented Jan 25, 2022

View reviewed changes

abravalheri force-pushed the improve-coverage-config branch 2 times, most recently from 84c2a2c to b5fd0c1 Compare January 25, 2022 15:44

abravalheri commented Jan 25, 2022

View reviewed changes

abravalheri marked this pull request as ready for review January 26, 2022 10:17

abravalheri changed the title ~~Improve coverage config~~ [CI] Improve coverage config Jan 26, 2022

jaraco reviewed Jan 30, 2022

View reviewed changes

.coveragerc Outdated Show resolved Hide resolved

abravalheri force-pushed the improve-coverage-config branch from b5fd0c1 to fa281e7 Compare February 8, 2022 18:31

webknjaz approved these changes Feb 8, 2022

View reviewed changes

abravalheri added 5 commits February 18, 2022 15:53

Cleanup coverage output

4ea7307

Produce both coverage reports (XML and term) on CI

08d4c3a

Coverage skip files completely covered in CI

f3b1fc7

Attempt to simplify configuration for coveragepy

89d696d

Omit path equivalence to see if logs are still usable.

Add skip_covered since it is more appropriate for large code bases

4952a1a

abravalheri force-pushed the improve-coverage-config branch from fa281e7 to 4952a1a Compare February 18, 2022 16:23

abravalheri commented Feb 18, 2022

View reviewed changes

jaraco force-pushed the main branch 3 times, most recently from a85759c to 93ce5a0 Compare December 16, 2022 18:21

jaraco force-pushed the main branch from 9296481 to 6067fa4 Compare April 18, 2024 21:31

jaraco force-pushed the main branch from 809a9a7 to ffdf0bd Compare September 15, 2024 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI] Improve coverage config #3050

[CI] Improve coverage config #3050

abravalheri commented Jan 25, 2022

abravalheri Jan 25, 2022 •

edited

Loading

jaraco Jan 30, 2022

webknjaz Jan 31, 2022

abravalheri Feb 8, 2022

abravalheri Jan 25, 2022

jaraco Jan 30, 2022

webknjaz Jan 31, 2022

abravalheri Feb 8, 2022

abravalheri commented Feb 8, 2022

webknjaz left a comment

abravalheri Feb 18, 2022

abravalheri Feb 18, 2022

agronholm commented Mar 25, 2022

jaraco commented Mar 26, 2022

[CI] Improve coverage config #3050

Are you sure you want to change the base?

[CI] Improve coverage config #3050

Conversation

abravalheri commented Jan 25, 2022

Summary of changes

Pull Request Checklist

abravalheri Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abravalheri commented Feb 8, 2022

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agronholm commented Mar 25, 2022

jaraco commented Mar 26, 2022

abravalheri Jan 25, 2022 •

edited

Loading