bit-team / backintime

Back In Time - An easy-to-use backup tool for GNU Linux using rsync in the back
https://backintime.readthedocs.io
GNU General Public License v2.0
2.01k stars 197 forks source link

Remove Coveralls support / Coveralls setup broken #1463

Closed buhtz closed 4 weeks ago

buhtz commented 1 year ago

I'm sorry I have to open this as an Issue because I don't know whats going on here. It seems that the setup on https://coveralls.io/github/bit-team/backintime is not correct. I do have admin rights (based on my bit-team membership). But I don't understand that UI. So I hope that Germar, Aryoda or someone else can help out here.

The badge on README.md do point to master which should be dev of course. Easy to fix. But I'm not sure if or how coveralls can check for "dev" instead of "master".

I'm not sure how coveralls get's its data but I know there are some coveralls-calls in our TravisCI build setup. Not sure if this is bound to a specific branch.

The landing page looks like this to me and confuses me a lot

image

In the top right you see "dev" with 50% and also "dev" as default branch. But in the bottom there is another branch.

The branch-dropdown-menu do not offer me "dev" even after clicking on "sync branches".

I checked the settings but before I break something I thought it is better to ask.

Related to #1282

buhtz commented 1 year ago

I vote to remove Coveralls service and account because I see no value in it and there is no one who want to take care of it.

Coverage as a measurement has its values but could be done locally on the machine of each developer.

The maintenance cost is high. In regards to #1489 I investigated and modified the make system. I discovered that there is some exceptional code handling coveralls on travisci. Without coveralls the travisci exceptions could be removed and the code is easier to maintain. Maybe there is away to remove the travisci-exception code from configure-make but keeping coveralls. But for that we need someone who understand coveralls and travisci configuration.

emtiu commented 1 year ago

If it's unmaintained and causes complexity, then I agree with removing it.

buhtz commented 11 months ago

It seems that we have admin rights to the coveralls project. So we can delete it our self. But I'll contact Germar first via mail.

Germar commented 10 months ago

The badge on README.md do point to master wich should be dev of course. Easy to fix. But I'm not sure if or how coveralls can check for "dev" instead of "master".

Just change like this will fix the badge

[![Coverage Status](https://coveralls.io/repos/github/bit-team/backintime/badge.svg?branch=dev)](https://coveralls.io/github/bit-team/backintime?branch=dev)

Like I said in https://github.com/bit-team/backintime/issues/1553#issuecomment-1794740677_ I used coveralls.io to monitor the impact of tests. For me coveralls.io always worked flawlessly in background. These three lines do the trick:

https://github.com/bit-team/backintime/blob/6df8c901ceecd659eaadb993103da3c6e89b4041/.travis.yml#L50-L52

buhtz commented 9 months ago

I am still not convinced and vote to remove coveralls support.

To "monitor the impact of tests" someone can clone the repo and run coveralls on their local machine. I see no value in doing this in public.

In my understanding based on my experience and studying of that topic the coverage of productive code by testing code only counts for unit tests. Integration and system tests do not count because they do not test one isolated behavior (sometimes misinterpreted as a block of code). They always test multiple behaviors at once.

Currently there is no pure unit test in BITs test suite. Because of my previous argument the coverage value is misleading. The code is nearly uncovered.

One intention of unit tests is to make it secure to do modifications in the code. I need to be sure that the modification of one line won't affect other parts of the code and its behavior. In the current situation I can not be sure.

aryoda commented 9 months ago

To "monitor the impact of tests" someone can clone the repo and run coveralls on their local machine. I see no value in doing this in public.

I think coveralls is kinda quality indicator for our users since they

I also don't expect much maintenance overhead since coveralls is just started by executing the unit tests (some text execution exclusions may be required for Travis CI - not coveralls per se.

So I would like to keep coveralls because it doesn't hurt and I do not always run the code coverage check locally...

buhtz commented 9 months ago

I think coveralls is kinda quality indicator for our users since they

I am aware of the marketing aspect and also like it. But the cost is higher then the benefit.

So I would like to keep coveralls because it doesn't hurt and I do not always run the code coverage check locally...

In the current state it hurts because the number is incorrect even if we count our tests as real unit tests. I am not sure but it seems to me that the coverage calculation do take the unit test code and its coverage also into account.

And we need someone to maintain it. Currently there is no one. The coveralls.io is stupid. Example: grafik

Top right "dev" branch is selected. But the list shows a pull request that belong to another branch.

And "0 of 0 relevant lines covered". WTF!?

I can not find a list of files and their values.

It is unclear/nontransparent where the current "75%" comes from. I assume that this value comes from a very old build long ago in the past.

The UI is not only bad it is broken. And I don't want to invest more time into it.

EDIT: I triggered a manual build of the dev branch on TravisCI. Now some file information appear on coveralls.io. image

As you can see here. The test files are included in the calculation and the common folder is not even included.

EDIT2: Even on my local machine it is not possible to generate an evident coverage report because of our project setup. See #1575 .

Partly "code coverage" (not test coverage) just in common:

Name                                                                         Stmts   Miss  Cover
------------------------------------------------------------------------------------------------
/home/user/mluCloud/my.work/bit/backintime/qt/plugins/notifyplugin.py           35     22    37%
/home/user/mluCloud/my.work/bit/backintime/qt/plugins/systrayiconplugin.py      44     19    57%
applicationinstance.py                                                          79      4    95%
backintime.py                                                                  534    258    52%
bcolors.py                                                                      18      8    56%
cli.py                                                                         171    153    11%
config.py                                                                      952    428    55%
configfile.py                                                                  330     18    95%
diagnostics.py                                                                 146     22    85%
encfstools.py                                                                  421    358    15%
exceptions.py                                                                   38      8    79%
languages.py                                                                     2      0   100%
logger.py                                                                       65      5    92%
mount.py                                                                       340     88    74%
password.py                                                                    198    156    21%
password_ipc.py                                                                 82     58    29%
pluginmanager.py                                                               134     35    74%
plugins/usercallbackplugin.py                                                   56     32    43%
progress.py                                                                     16      2    88%
snapshotlog.py                                                                  89     12    87%
snapshots.py                                                                  1339    313    77%
sshtools.py                                                                    423     90    79%
tools.py                                                                      1143    396    65%
------------------------------------------------------------------------------------------------
TOTAL                                                                         6655   2485    63%

Be aware that some *.py files with productive code (askpass.py, driveinfo.py, guiapplicationinstance.py, ...) are missing in this calculation because they are never loaded by pytest/unittest. This problem will disappear when fixing #1575. I assume that even if we would have a coveralls.io expert it is not possible to setup the report in a realistic way without hacky workarounds until we migrate the projects infrastructure to a Python build-system.

Interpretation of the result: The value "63%" is optimistic because some files with productive code are missing. So make it 55% I would say. But just for common. Put qt onto it wich is nearly uncovered the value do dramatically decrease again. My broad but optimistic estimation would be 20 to 30%. And again: This is just code coverage and not test coverage because we do not have isolated unit test. Do not take this wrong. Integration and system tests are needed and valuable also. But there goal is different. They do not solve the problem with introducing bugs and modified behavior via fixing other bugs. It is just luck (or a side effect) when they catch new bugs.

Using coveralls.io is IMHO not an indicator of quality in our case. It is the opposite.

EDIT3: Just for your information here an Issue-based discussion about why coverage.py by default do measure by more then just the productive code. https://github.com/nedbat/coveragepy/issues/1707

EDIT4: As more I get into this topic (e.g. my tec demo) and the unexpected behavior of coverage.py I come to the conclusion that coverage values of most of the projects out there are just wrong because coverage.py is not correct configured and its default behavior is useless in context of code quality and test coverage.

buhtz commented 3 months ago

There have been no new arguments on this matter since it was last discussed here. Since this blocks the upcoming release, the active maintainer team has made a decision to remove coveralls from the current project. After migration to modern python packaging standards (#1575) coveragepy will be configured again to calculate strict test coverage.