Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use color for unchanged coverage #777

Closed
uhafner opened this issue Sep 29, 2023 · 5 comments
Closed

Do not use color for unchanged coverage #777

uhafner opened this issue Sep 29, 2023 · 5 comments
Assignees
Labels
enhancement Enhancement of existing functionality

Comments

@uhafner
Copy link
Member

uhafner commented Sep 29, 2023

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

Bildschirmfoto 2023-09-29 um 13 03 15

@uhafner uhafner added enhancement Enhancement of existing functionality hacktoberfest labels Sep 29, 2023
@Amanjain4269
Copy link
Contributor

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

@uhafner
Copy link
Member Author

uhafner commented Oct 2, 2023

Hi @Amanjain4269

There are basically two things to implement:

This code needs to be changed on the UI side:

<j:when test="${it.isPositiveTrend(baseline, value.metric)}">

And this is the model on the server side:

public boolean isPositiveTrend(final Baseline baseline, final Metric metric) {

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

  • a negative value when the color should be var(--red)
  • a positive value when the color should be var(--green)
  • zero when the color should be var(--text-color)

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
Copy link
Contributor

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
Copy link
Member Author

uhafner commented Oct 8, 2023

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 added a commit to Amanjain4269/code-coverage-api-plugin that referenced this issue Oct 11, 2023
The isPostiveTrend() is changed to getTrend() and corresponding file changes are made as per the PR - Do not use color for unchanged coverage jenkinsci#777
@Amanjain4269
Copy link
Contributor

Hello !!!
Sorry for the delay.

I have made the changes and added some test cases for review.
#789
Please check.

Amanjain4269 added a commit to Amanjain4269/code-coverage-api-plugin that referenced this issue Oct 12, 2023
Some changes are made again as per the feedback in the draft PR Do not use color for unchanged coverage jenkinsci#777
Amanjain4269 added a commit to Amanjain4269/code-coverage-api-plugin that referenced this issue Oct 13, 2023
This is regarding the PR Do not use color for unchanged coverage jenkinsci#777
uhafner added a commit that referenced this issue Oct 16, 2023
Do not use color for unchanged coverage #777
@uhafner uhafner closed this as completed Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants