openfisca / openfisca-core

OpenFisca core engine. See other repositories for countries-specific code & data.
https://openfisca.org
GNU Affero General Public License v3.0
168 stars 75 forks source link

Remove unused Coveralls #1073

Open MattiSG opened 2 years ago

MattiSG commented 2 years ago

Hi there!

I really enjoy OpenFisca, but I recently encountered an issue.

Here is what I did:

Here is what I expected to happen:

Here is what actually happened:

Here is data (or links to it) that can help you reproduce this issue:

https://github.com/openfisca/openfisca-core/pull/1070#issuecomment-945785704

Context

I identify more as a:

Coverage data storage was introduced by @maukoquiroga in b785382722eeb72b8cc72f18cb0942e0b6424c1e. It does not seem to be used by anyone in the build / release process, and a quick question on Slack did not yield any other reply. I am in favour of removing coveralls.io altogether, which I suggest to do unless someone chimes in within 4 days to say they use it, with a fast removal if @maukoquiroga confirms it can be removed.

The alternative would be to not execute the Coveralls CI job on third-party PRs.

MattiSG commented 2 years ago

The data seems to show that coverage went down from 76% to 54% on 30th of September 2021. Investigating the origin of that change is not obvious.

Capture d’écran 2021-10-26 à 09 13 50

Based on time match, it seems the origin would be #1046, which among other things removed the --capture flag in test_runner.py.

In my opinion, the fact that such a sharp drop in coverage did not trigger any investigation tends to demonstrate this data is not used by anyone. I understand the cost of continuing to store it is low, and it might make sense to keep it since we have data dating back from March 2015, but it would then need to be actually integrated in a quality management agreement.

MattiSG commented 2 years ago

Data collection doesn't seem reliable: https://github.com/openfisca/openfisca-core/pull/1057#issuecomment-950595085

bonjourmauko commented 2 years ago

Hello @MattiSG , I do personally do not care about the history, but I do care about coverage.

In my opinion, the fact that such a sharp drop in coverage did not trigger any investigation tends to demonstrate this data is not used by anyone.

I do, but coverage is rather useless as a socio-technical mechanism unless there is a policy enforced programatically.

Concerning data collection, I proposed in #1059 what I think is the way it should be done.

Concerning Coveralls itself, as a service, we can remove it 😃

bonjourmauko commented 2 years ago

Just to complement my previous comment:

  1. We can remove the service Coveralls
  2. I'll propose a PR to fix the coverage issue —thanks for reporting it!
bonjourmauko commented 2 years ago

Thanks to this issue, I discovered that doctests are not being run 🤯