jenkinsci / code-coverage-api-plugin

Deprecated Jenkins Code Coverage Plugin
https://plugins.jenkins.io/code-coverage-api/
MIT License
111 stars 77 forks source link

Do not use color for unchanged coverage #777

Closed uhafner closed 9 months ago

uhafner commented 11 months ago

When the coverage delta is zero, then no coloring should be applied to the text.

Bildschirmfoto 2023-09-29 um 13 03 15

Amanjain4269 commented 11 months ago

@uhafner Hello !!! I want to work on this issue. Can you plz tell me how to approach it?

uhafner commented 11 months ago

Hi @Amanjain4269

There are basically two things to implement:

This code needs to be changed on the UI side: https://github.com/jenkinsci/code-coverage-api-plugin/blob/9804fdba04bd9edaab3894733fb9018a54c6745c/plugin/src/main/resources/coverage/coverage-summary.jelly#L63

And this is the model on the server side: https://github.com/jenkinsci/code-coverage-api-plugin/blob/9804fdba04bd9edaab3894733fb9018a54c6745c/plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildAction.java#L550

I would suggest to change that method to something like int getTrend, that returns

It makes sense to use rounding in the server method so that 0.05 still is text color while 0.06 uses green. The server method requires a small unit test in https://github.com/jenkinsci/code-coverage-api-plugin/blob/master/plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildActionTest.java.

Amanjain4269 commented 11 months ago

Hello!! Thanks for the guidance. I have tried changing the isPositiveTrend() method to int getTrend() method. Please check it once. CoverageBuildAction.java file - https://github.com/Amanjain4269/code-coverage-api-plugin/blob/615a021cf2af17db2882b157250de87ebd2ed044/plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildAction.java#L550

coverage-summary.jelly file - https://github.com/Amanjain4269/code-coverage-api-plugin/blob/615a021cf2af17db2882b157250de87ebd2ed044/plugin/src/main/resources/coverage/coverage-summary.jelly#L62C27-L62C27

Can you help me out in writing the test case of this function, mainly what values of metric (Metric.LINE or Metric.BRANCH) should be passed to get 1, -1, 0 returned from the getTrend() function?

uhafner commented 11 months ago

Can you please create a draft pull request? Then I can write comments in the diff.

Can you help me out in writing the test case of this function, mainly what values of metric (Metric.LINE or Metric.BRANCH) should be passed to get 1, -1, 0 returned from the getTrend() function?

It makes no difference which metric or baseline you are using. Create a new action (see some other tests on how to do that) with some percentages. Then you can call the method.

Amanjain4269 commented 10 months ago

Hello !!! Sorry for the delay.

I have made the changes and added some test cases for review. https://github.com/jenkinsci/code-coverage-api-plugin/pull/789 Please check.