Pytest: [RFC] consider droping codecov

Created on 31 Mar 2020  路  15Comments  路  Source: pytest-dev/pytest

With codecov i observe a lot of failures/issues/false positives.

So i would appreciate if we found and switched to a more stable alternative.

infrastructure proposal

Most helpful comment

How about this then:

  1. Disable codecov/project, keep codecov/patch: this is really quick to do and will provide immediate benefits.
  2. In a separate PR, we go with @Zac-HD's suggestion of annotating all places missing coverage, and then we can enable codecov/project again.

I propose this because it might take a while to get to 2, while we can ripe the benefits of 1 fairly quickly.

All 15 comments

This is due to flaky coverage - e.g. observed with Windows+xdist - not using xdist makes it stable (for that part). But there are other parts that are racy, would need to be covered explicitly.
You could disable the project status for now.

IMO it's important to have coverage measurements, but we don't need to use an external service.

For Hypothesis we just use the --fail-under=100 option to enforce full code coverage, and explicitly annotate each branch that isn't expected to be covered. This would look noisy at first, but is really helpful in preventing test regressions longer term!

I don't see how switching to another service would change anything - all that codecov does is presenting the data, if that data is flaky, switching to e.g. coveralls won't change much.

+1 on disabling the PR status check as long as that's the case, though.

im fine with that - given the presented arguments i believe status checks should pause but at the same time we should open a project for trying for 100% reported coverage (for which well placed+reasoned coverage ignores may be acceptable)

we should open a project for trying for 100% reported coverage (for which well placed+reasoned coverage ignores may be acceptable)

I'd approach this from the other direction:

  1. Add an new ignore pattern, e.g. # pragma: TODO cover this
  2. --fail-under=100
  3. Standing ticket for contributors to write a test that allows for an instance to be removed, or (rarely) convert it to a standard # pragma: no cover. This would be a great one for sprints or new contributors generally.

This involves a little more code churn, but means we get to enforce 'all new code must be covered' instantly. It's also pretty easy to check our progress with git grep, remove the ignore patter to check progress, and so on.

@Zac-HD i love that suggestion

from my pov it expands mine by enabling us to place coverage todos and issues while trying to enforce full coverage on new commits

so in effect putting much better and stricter tooling into the project i proposed

the project would just group the issues/tickets/pr's around sorting out coverage

I also like the idea of ensuring full coverage for new commits, and the proposed solution by @Zac-HD would take care of getting they flaky coverage out of the way.

FWIW the coverage checks are not required at the moment:

image

codecov's diff status is usually correct. It defaults to requirering increased coverage, and could be bumped to 100%.
The project status could be disabled. It will naturally also be red for when you remove code etc, and the total coverage drops.
I find codecov much more easier to revisit then manual coverage reports (which you would need to combine then) - that's also a problem with "--fail-under" then: some things are only covered by a subset of jobs etc.

i created https://github.com/pytest-dev/pytest/projects/6 to organize tracking this effort

I find the "patch" report useful, it just shows which parts of the diff are covered and fails if something isn't. But the "project" one, not so much, often it fails without any apparent reason and no way to fix in the PR.

There's also https://github.com/wemake-services/coverage-conditional-plugin to cover some complex cases.

+1 for disabling codecov/project status but keeping codecov/patch

How about this then:

  1. Disable codecov/project, keep codecov/patch: this is really quick to do and will provide immediate benefits.
  2. In a separate PR, we go with @Zac-HD's suggestion of annotating all places missing coverage, and then we can enable codecov/project again.

I propose this because it might take a while to get to 2, while we can ripe the benefits of 1 fairly quickly.

Sounds good to me :+1:

I like this idea

Was this page helpful?
0 / 5 - 0 ratings