Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -544,18 +544,25 @@ public String formatDelta(final Baseline baseline, final Metric metric) {
* @param metric
* the metric to check
*
* @return {@code true} if the trend is positive, {@code false} otherwise
* @return {@code roundedDeltaValue} if the trend is positive or negative, {@code 0} otherwise
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @return {@code roundedDeltaValue} if the trend is positive or negative, {@code 0} otherwise
* @return a positive value if the trend is positive, a negative value if the trend is negative, or {@code 0} if there is no significant change in the trend

*/
@SuppressWarnings("unused") // Called by jelly view
public boolean isPositiveTrend(final Baseline baseline, final Metric metric) {
public double getTrend(final Baseline baseline, final Metric metric) {
var delta = getDelta(baseline, metric);
if (delta.isPresent()) {
if (delta.get().compareTo(Fraction.ZERO) > 0) {
return metric.getTendency() == MetricTendency.LARGER_IS_BETTER;
double deltaValue = delta.get().doubleValue();
// to apply rounding off for boundary delta values
double roundedDelta = Math.round(deltaValue * 100.0) / 100.0;
if (-0.1 < roundedDelta && roundedDelta < 0.1) {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, my fault: percentages are stored in the interval [0,1], so it makes sense to compare 0.001. I also do not see the reason why you are rounding?

// for var(--text-color)
return 0;
}
else {
// for var(--red or --green)
return roundedDelta;
}
return metric.getTendency() == MetricTendency.SMALLER_IS_BETTER;
}
return true;
return 0; // default to zero
}

/**
Expand Down
7 changes: 5 additions & 2 deletions plugin/src/main/resources/coverage/coverage-summary.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@
<li>${formatter.formatValueWithMetric(value)}
<j:if test="${it.hasDelta(baseline, value.metric)}">
<j:choose>
<j:when test="${it.isPositiveTrend(baseline, value.metric)}">
<j:when test="${it.getTrend(baseline, value.metric) > 0}">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<j:when test="${it.getTrend(baseline, value.metric) > 0}">
<j:when test="${it.getTrend(baseline, value.metric) gt 0}">

<j:set var="color" value="var(--green)"/>
</j:when>
<j:otherwise>
<j:when test="${it.getTrend(baseline, value.metric) < 0}">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<j:when test="${it.getTrend(baseline, value.metric) < 0}">
<j:when test="${it.getTrend(baseline, value.metric) lt 0}">

<j:set var="color" value="var(--red)"/>
</j:when>
<j:otherwise>
<j:set var="color" value="var(--text-color)"/>
</j:otherwise>
</j:choose>
<span style="color: ${color}">(${it.formatDelta(baseline, value.metric)})</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,38 @@
*/
@DefaultLocale("en")
class CoverageBuildActionTest {
private CoverageBuildAction createCoverageBuildActionWithDelta() {
Node module = new ModuleNode("module");

var coverageBuilder = new CoverageBuilder();
var percent50 = coverageBuilder.setMetric(Metric.BRANCH).setCovered(1).setMissed(1).build();
var percent80 = coverageBuilder.setMetric(Metric.LINE).setCovered(8).setMissed(2).build();
var percent30 = coverageBuilder.setMetric(Metric.INSTRUCTION).setCovered(3).setMissed(7).build();
var percent35 = coverageBuilder.setMetric(Metric.CLASS).setCovered(35).setMissed(65).build();

module.addValue(percent50);
module.addValue(percent80);
module.addValue(percent30);
module.addValue(percent35);

var deltas = new TreeMap<Metric, Fraction>();
var lineDelta = percent80.getCoveredPercentage().subtract(percent50.getCoveredPercentage());
deltas.put(Metric.LINE, lineDelta);
var branchDelta = percent50.getCoveredPercentage().subtract(percent80.getCoveredPercentage());
deltas.put(Metric.BRANCH, branchDelta);
var instructionDelta = percent30.getCoveredPercentage().subtract(percent30.getCoveredPercentage());
deltas.put(Metric.INSTRUCTION, instructionDelta);
var classDelta = percent35.getCoveredPercentage().subtract(percent30.getCoveredPercentage());
deltas.put(Metric.CLASS, classDelta);
deltas.put(Metric.FILE, Fraction.getFraction(99,1000)); // to test for boundary case

var coverages = List.of(percent50, percent80, percent30, percent35);

return spy(new CoverageBuildAction(mock(FreeStyleBuild.class), CoverageRecorder.DEFAULT_ID,
StringUtils.EMPTY, StringUtils.EMPTY, module, new QualityGateResult(),
createLog(), "-", deltas, coverages, deltas, coverages, deltas, coverages, false));
}

@Test
void shouldNotLoadResultIfCoverageValuesArePersistedInAction() {
Node module = new ModuleNode("module");
Expand Down Expand Up @@ -88,4 +120,46 @@ void shouldCreateViewModel() {
assertThat(action.getTarget()).extracting(CoverageViewModel::getNode).isSameAs(root);
assertThat(action.getTarget()).extracting(CoverageViewModel::getOwner).isSameAs(action.getOwner());
}

@Test
void shouldReturnPositiveTrendForLineMetric() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.LINE);
assertThat(trend).isEqualTo(0.3); // deltaValue = 0.3
Copy link
Member

Choose a reason for hiding this comment

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

Almost: what I actually meant is something like:

Suggested change
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.LINE);
assertThat(trend).isEqualTo(0.3); // deltaValue = 0.3
CoverageBuildAction action = createCoverageBuildActionWithDelta(Baseline.PROJECT, Metric.LINE, Fraction.of("0.001");
assertThat(action.getTrend(Baseline.PROJECT, Metric.LINE)).isPositive();

and then

        CoverageBuildAction action = createCoverageBuildActionWithDelta(Baseline.PROJECT, Metric.LINE, Fraction.of("0.0009");
        assertThat(action.getTrend(Baseline.PROJECT, Metric.LINE)).isZero();

So move the interesting parts to the focus of the reader.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be helpful to add here:

assertThat(action.formatDelta(Baseline.PROJECT, Metric.LINE)).isEqualTo("+0.10%");

}

@Test
void shouldReturnNegativeTrendForBranchMetric() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.BRANCH);
assertThat(trend).isEqualTo(-0.3); // deltaValue = -0.3
}

@Test
void shouldReturnZeroForZeroDelta() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.INSTRUCTION);
assertThat(trend).isEqualTo(0.0); // deltaValue = 0.0
}

@Test
void shouldReturnZeroWhenDeltaIsNotPresent() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.METHOD);
assertThat(trend).isEqualTo(0); // deltaValue is not present
}

@Test
void shouldReturnZeroForDeltaWithinBoundaries() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.CLASS);
assertThat(trend).isEqualTo(0); // deltaValue = 0.05
}

@Test
void shouldReturnPositiveTrendForBoundaryDeltaValue() {
CoverageBuildAction action = createCoverageBuildActionWithDelta();
double trend = action.getTrend(Baseline.PROJECT, Metric.FILE);
assertThat(trend).isEqualTo(0.1); // deltaValue = 0.099 (will be rounded off)
}
Copy link
Member

Choose a reason for hiding this comment

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

Tests look good now!

Just a couple of analysis warnings remaining...

}