Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Conversation

reshadat
Copy link

@reshadat reshadat commented Dec 16, 2022

While calculating difference in coverage, there might be a difference even without code change, due to floating point rounding errors. Converting the diff float to BigDecimal with appropriate scale and converting back solves this issue.

Fixes: https://github.com/jenkinsci/code-coverage-api-plugin/issues/191

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #533 (dcdaaa1) into master (e9f081d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #533      +/-   ##
============================================
+ Coverage     72.74%   72.80%   +0.05%     
- Complexity     1026     1028       +2     
============================================
  Files            88       88              
  Lines          3823     3824       +1     
  Branches        440      440              
============================================
+ Hits           2781     2784       +3     
+ Misses          894      893       -1     
+ Partials        148      147       -1     
Impacted Files Coverage Δ
...io/jenkins/plugins/coverage/CoverageProcessor.java 83.55% <100.00%> (+0.71%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 231 to 232
//Converting float point to Big decimal and converting back fixes the float precision point problem
float diff = BigDecimal.valueOf((buildRatio.getPercentageFloat() - referenceRatio.getPercentageFloat())).floatValue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap that code into a method? Then you can write a small unit test that verifies the new behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests. Please re-review

@reshadat reshadat requested a review from uhafner December 18, 2022 06:03
@reshadatavid
Copy link
Contributor

@uhafner What should be the next steps? Should I do something else for this to be merged and releases in new verison?

@uhafner
Copy link
Member

uhafner commented Dec 19, 2022

No, I just needed to wait for Jenkins to mark this PR as green.

Since this code is deprecated anyway there is no need to fix styling issues (I already removed the corresponding checks).

@uhafner uhafner merged commit 2ce1a4b into jenkinsci:master Dec 19, 2022
@uhafner uhafner added the bug Bugs or performance problems label Dec 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Bugs or performance problems

Projects

None yet

Development

Successfully merging this pull request may close these issues.

failBuildIfCoverageDecreasedInChangeRequest should not fail if no code changed

3 participants