[Matplotlib-devel] Why keep codecov?

https://codecov.io/gh/matplotlib/matplotlib/compare/5455bab77e75cee46d260c0f27110d6b4fa78270...3eb471dadd2fd397c840bc98db90a06845697131/changes

The link above is what I find for the "failing" codecov test for https://github.com/matplotlib/matplotlib/pull/17266, which is passing all the other tests.

The codecov.io output looks like complete garbage, listing a huge number of untouched files.

I added 2 tests for the code I modified. As far as I can see, from a practical standpoint, codecov is incorrectly flagging the PR as failing, leading to a misleading red X in the PR list.

I can see how a mechanized code coverage test could be useful for a project at an early stage, when it is building up its test infrastructure, and at a later stage, to be run occasionally, especially when major code additions are made.

As it is being run now, however, I think it is impeding, not aiding, the development process.

I recommend that we remove it from the suite of tests run for every PR, or at least keep it out of the accounting for the overall "pass/fail" rating of the PR.

Eric

···

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
https://mail.python.org/mailman/listinfo/matplotlib-devel

Hi Eric,

I agree for the codecov/project check. I don’t see how it helps, and I routinely ignore it during reviews because it is pretty obscure how you are supposed to fix it.

In my opinion, codecov/patch is pretty helpful to know if someone’s new code actually ever gets exercised.

Cheers, Jody

···

On May 5, 2020, at 18:55 PM, Eric Firing <efiring@hawaii.edu> wrote:

Codecov

The link above is what I find for the "failing" codecov test for Keep explicit ticklabels in sync with ticks from FixedLocator by efiring · Pull Request #17266 · matplotlib/matplotlib · GitHub, which is passing all the other tests.

The codecov.io output looks like complete garbage, listing a huge number of untouched files.

I added 2 tests for the code I modified. As far as I can see, from a practical standpoint, codecov is incorrectly flagging the PR as failing, leading to a misleading red X in the PR list.

I can see how a mechanized code coverage test could be useful for a project at an early stage, when it is building up its test infrastructure, and at a later stage, to be run occasionally, especially when major code additions are made.

As it is being run now, however, I think it is impeding, not aiding, the development process.

I recommend that we remove it from the suite of tests run for every PR, or at least keep it out of the accounting for the overall "pass/fail" rating of the PR.

Eric
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
Matplotlib-devel Info Page

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
https://mail.python.org/mailman/listinfo/matplotlib-devel

I think the current problem with codecov is that on the master branch azure uploads coverage reports but it does not an the PR runs which leads to the drop in coverage due to the edge cases that are hit slightly differently on azure than on travis / appveyor.

Tom

···

Thomas Caswell
tcaswell@gmail.com

Tom,

I'm sorry but I don't understand. Let me put it another way: how often does the codecov flagging and output lead to the discovery and solution of a real problem? At present, it is clearly an annoyance that incurs a real penalty by putting false information into the PR listings; if that isn't balanced by some substantial benefit, then we should drop it. What am I missing?

Eric

···

On 2020/05/05 5:24 PM, Thomas Caswell wrote:

I think the current problem with codecov is that on the master branch azure uploads coverage reports but it does not an the PR runs which leads to the drop in coverage due to the edge cases that are hit slightly differently on azure than on travis / appveyor.

Tom

On Tue, May 5, 2020 at 11:21 PM Jody Klymak <jklymak@uvic.ca > <mailto:jklymak@uvic.ca>> wrote:

    Hi Eric,

    I agree for the codecov/project check. I don’t see how it helps,
    and I routinely ignore it during reviews because it is pretty
    obscure how you are supposed to fix it.

    In my opinion, codecov/patch is pretty helpful to know if someone’s
    new code actually ever gets exercised.

    Cheers, Jody

     > On May 5, 2020, at 18:55 PM, Eric Firing <efiring@hawaii.edu > <mailto:efiring@hawaii.edu>> wrote:
     >
    Codecov
     >
     > The link above is what I find for the "failing" codecov test for
    Keep explicit ticklabels in sync with ticks from FixedLocator by efiring · Pull Request #17266 · matplotlib/matplotlib · GitHub, which is
    passing all the other tests.
     >
     > The codecov.io <http://codecov.io> output looks like complete
    garbage, listing a huge number of untouched files.
     >
     > I added 2 tests for the code I modified. As far as I can see,
    from a practical standpoint, codecov is incorrectly flagging the PR
    as failing, leading to a misleading red X in the PR list.
     >
     > I can see how a mechanized code coverage test could be useful for
    a project at an early stage, when it is building up its test
    infrastructure, and at a later stage, to be run occasionally,
    especially when major code additions are made.
     >
     > As it is being run now, however, I think it is impeding, not
    aiding, the development process.
     >
     > I recommend that we remove it from the suite of tests run for
    every PR, or at least keep it out of the accounting for the overall
    "pass/fail" rating of the PR.
     >
     > Eric
     > _______________________________________________
     > Matplotlib-devel mailing list
     > Matplotlib-devel@python.org <mailto:Matplotlib-devel@python.org>
     > Matplotlib-devel Info Page

    _______________________________________________
    Matplotlib-devel mailing list
    Matplotlib-devel@python.org <mailto:Matplotlib-devel@python.org>
    Matplotlib-devel Info Page

--
Thomas Caswell
tcaswell@gmail.com <mailto:tcaswell@gmail.com>

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
https://mail.python.org/mailman/listinfo/matplotlib-devel

My point is that the issue (the coverage on the tests always being reported as going down in unrelated files) is due to a miss-configuration (azure not uploading reports on PRs) rather than a fundamental problem.

I think “is the check useful in principle” should be considered separately from “is it currently configured correctly”.

Tom

···

Thomas Caswell
tcaswell@gmail.com

OK, you have identified how it is misconfigured (I guess). Now we can separately consider my question, slightly modified, because what we really care about is reality, not an abstract principle: Is the check useful in practice? Has it in the past, and is it likely in the future, to identify a real problem that we fix, to the benefit of the project?

As Jody noted, the codecov/patch seems to be doing something useful, codecov/project does not.

Eric

···

On 2020/05/06 5:10 AM, Thomas Caswell wrote:

I think "is the check useful in principle" should be considered separately from "is it currently configured correctly".

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
https://mail.python.org/mailman/listinfo/matplotlib-devel

So the benefits of the project coverage:

  • Catching code paths that no longer run due to dependency changes

  • Seeing how the PR impacts overall coverage (80% coverage of a large PR is different than 80% of a small one)

  • Seeing code coverage changes due to other changes in CI configuration

So while they’re not relevant to 90% of PRs, they’re still something that should be checked. I’d argue all of that is great benefit that we can get essentially for free by having the bots do this for us.

The problem is that on the matplotlib project there’s no trust in the codecov setup, and as a result barely anyone pays attention to them. IMO, that’s the problem we need to solve. I agree with Tom that this is a configuration problem for matplotlib. This is not a problem, for example, on MetPy where our codecov setup works perfectly fine. Any occasional inconsistency in results or supposed red herrings usually trace back to problems with our config (codecov’s questionable UX aside).

Ryan

···

Ryan May