ubiquity / ubiquity-dollar

Ubiquity Dollar (UUSD) smart contracts and user interface.
https://uad.ubq.fi
Apache License 2.0
34 stars 89 forks source link

fix incorrect coverage values in the compare test coverage workflow #828

Closed gitcoindev closed 11 months ago

gitcoindev commented 11 months ago

Resolves: #826

ubiquibot[bot] commented 11 months ago
gitcoindev commented 11 months ago

I tested multiple times on my fork and just added deleting development branch folder after calculating coverage to really make sure that coveralls does not read anything from it. Should not happen, but adding this step increases safety.

New QA: https://coveralls.io/jobs/130726799

I propose to review and close this pull request 'as is' as it fixes the bug and open a new edge case issue only when it appears.

It is additionally possible to set pull request alerts on coveralls side if needed, e.g. if 70% or any arbitrary coverage is considered 'green' and anything below should be marked as 'red' in the badge @pavlovcik @rndquu @molecula451 .

image

rndquu commented 11 months ago

@gitcoindev Could you provide a QA CI run when the "Compare Test Coverage" fails on decreased test coverage?

gitcoindev commented 11 months ago

@rndquu sure.

Step 1 - I synchronized my ubiquity-dollar fork development branch 1:1 to match upstream. Step 2 - I opened a pull request https://github.com/gitcoindev/ubiquity-dollar/pull/7 which is a mirror of this pull request, but directed towards my forked repository. Step 3 - I did QA of my mirror pull request here https://github.com/gitcoindev/ubiquity-dollar/actions/runs/6651769749 . I received a comment that proves that coverage percentage was fixed https://github.com/gitcoindev/ubiquity-dollar/pull/7#issuecomment-1780707064 Step 4 - I merged my mirror pull request to simulate merging this one. Step 5 - I removed test cases to reduce coverage and opened an another pull request to my fork https://github.com/gitcoindev/ubiquity-dollar/pull/8

Result: coverage decreased and job is failing. QA: https://github.com/gitcoindev/ubiquity-dollar/actions/runs/6652771962/job/18077404702

gitcoindev commented 11 months ago

image

gitcoindev commented 11 months ago

@molecula451 I applied required changes, and would be grateful if you had a look again at the pr.

molecula451 commented 11 months ago

the improvement looks good, but, lets test it a bit more, I have re-ran rndqnuu PR's action with the old workflow and it seems to be greening now, any thoughts on why? https://github.com/ubiquity/ubiquity-dollar/actions/runs/6627359435/workflow you can check #attempt 2

gitcoindev commented 11 months ago

the improvement looks good, but, lets test it a bit more, I have re-ran rndqnuu PR's action with the old workflow and it seems to be greening now, any thoughts on why? https://github.com/ubiquity/ubiquity-dollar/actions/runs/6627359435/workflow you can check #attempt 2

@molecula451 yes, it is greenish but my thoughts are: it is broken.

Development branch coverage: 10.1 PR branch coverage: 10.1

Should be > 60%. The job is green and passing only because it does not show coverage degradation, but the calculated coverage value itself is wrong. Please have a look at the compared coverage values.

gitcoindev commented 11 months ago

Current correct values:

Development branch coverage: 69.1 PR branch coverage: 69.1

gitcoindev commented 11 months ago

Thank you for approvals and merging!