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

Commit

Permalink
Use the coverages that are persisted within the action if possible.
Browse files Browse the repository at this point in the history
Otherwise, the whole coverage result XML will be loaded just for a single value.
See jenkins-infra/helpdesk#3029 for details.
  • Loading branch information
uhafner committed Jul 10, 2022
1 parent 3534e47 commit 1db93ae
Showing 3 changed files with 128 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -33,7 +33,6 @@
*/
@SuppressWarnings("PMD.GodClass")
public class CoverageBuildAction extends BuildAction<CoverageNode> implements HealthReportingAction, StaplerProxy {

/** Relative URL to the details of the code coverage results. */
public static final String DETAILS_URL = "coverage";
/** The coverage report icon. */
@@ -51,9 +50,14 @@ public class CoverageBuildAction extends BuildAction<CoverageNode> implements He
private final String referenceBuildId;

// since 3.0.0
/** The delta of this build's coverages with respect to the reference build. */
private SortedMap<CoverageMetric, CoveragePercentage> difference;
/** The coverages filtered by changed lines of the associated change request. */
// FIXME: actually we need the coverages?
private SortedMap<CoverageMetric, CoveragePercentage> changeCoverage;
/** The delta of the coverages of the associated change request with respect to the reference build. */
private SortedMap<CoverageMetric, CoveragePercentage> changeCoverageDifference;
/** The indirect coverage changes of the associated change request with respect to the reference build. */
private SortedMap<CoverageMetric, CoveragePercentage> indirectCoverageChanges;

@SuppressWarnings("unused")
@@ -86,13 +90,13 @@ public CoverageBuildAction(final Run<?, ?> owner, final CoverageNode result, fin
* @param referenceBuildId
* the ID of the reference build
* @param delta
* the delta coverage with respect to the reference build
* delta of this build's coverages with respect to the reference build
* @param changeCoverage
* the change coverage with respect to the reference build
* the coverages filtered by changed lines of the associated change request
* @param changeCoverageDifference
* the change coverage delta with respect to the reference build
* the delta of the coverages of the associated change request with respect to the reference build
* @param indirectCoverageChanges
* the indirect coverage changes with respect to the reference build
* the indirect coverage changes of the associated change request with respect to the reference build
*/
@SuppressWarnings("checkstyle:ParameterNumber")
public CoverageBuildAction(final Run<?, ?> owner, final CoverageNode result,
@@ -112,7 +116,8 @@ public CoverageBuildAction(final Run<?, ?> owner, final CoverageNode result,
final SortedMap<CoverageMetric, CoveragePercentage> delta,
final SortedMap<CoverageMetric, CoveragePercentage> changeCoverage,
final SortedMap<CoverageMetric, CoveragePercentage> changeCoverageDifference,
final SortedMap<CoverageMetric, CoveragePercentage> indirectCoverageChanges, final boolean canSerialize) {
final SortedMap<CoverageMetric, CoveragePercentage> indirectCoverageChanges,
final boolean canSerialize) {
super(owner, result, canSerialize);

lineCoverage = result.getCoverage(CoverageMetric.LINE);
@@ -130,8 +135,7 @@ public CoverageBuildAction(final Run<?, ?> owner, final CoverageNode result,
protected Object readResolve() {
if (difference == null) {
difference = StreamEx.of(delta.entrySet())
.toSortedMap(Entry::getKey,
e -> CoveragePercentage.valueOf(e.getValue()));
.toSortedMap(Entry::getKey, e -> CoveragePercentage.valueOf(e.getValue()));
}
if (changeCoverage == null) {
changeCoverage = new TreeMap<>();
@@ -154,26 +158,39 @@ public Coverage getBranchCoverage() {
}

/**
* Returns whether the {@link Coverage} for the passed metric exists.
* Returns whether a {@link Coverage} for the specified metric exists.
*
* @param coverageMetric
* the metric to check
* the coverage metric
*
* @return {@code true} if a coverage is available for the specified metric
* @return {@code true} if a coverage is available for the specified metric, {@code false} otherwise
*/
public boolean hasCoverage(final CoverageMetric coverageMetric) {
if (coverageMetric.equals(CoverageMetric.LINE)) {
return lineCoverage.isSet();
}
if (coverageMetric.equals(CoverageMetric.BRANCH)) {
return branchCoverage.isSet();
}

return getResult().getCoverage(coverageMetric).isSet();
}

/**
* Gets the {@link Coverage} for the passed metric.
* Returns the {@link Coverage} for the specified metric.
*
* @param coverageMetric
* The coverage metric
* the coverage metric
*
* @return the coverage
*/
public Coverage getCoverage(final CoverageMetric coverageMetric) {
if (coverageMetric.equals(CoverageMetric.LINE)) {
return lineCoverage;
}
if (coverageMetric.equals(CoverageMetric.BRANCH)) {
return branchCoverage;
}
return getResult().getCoverage(coverageMetric);
}

@@ -183,7 +200,7 @@ public Coverage getCoverage(final CoverageMetric coverageMetric) {
* @return {@code true} if the change coverage exist, else {@code false}
*/
public boolean hasChangeCoverage() {
return getResult().hasChangeCoverage();
return hasChangeCoverage(CoverageMetric.LINE) || hasChangeCoverage(CoverageMetric.BRANCH);
}

/**
@@ -195,7 +212,7 @@ public boolean hasChangeCoverage() {
* @return {@code true} if the change coverage exist for the metric, else {@code false}
*/
public boolean hasChangeCoverage(final CoverageMetric coverageMetric) {
return getResult().hasChangeCoverage(coverageMetric);
return changeCoverage.containsKey(coverageMetric);
}

/**
@@ -207,6 +224,7 @@ public boolean hasChangeCoverage(final CoverageMetric coverageMetric) {
* @return the change coverage
*/
public Coverage getChangeCoverage(final CoverageMetric coverageMetric) {
// FIXME: is percentage sufficient?
return getResult().getChangeCoverageTree().getCoverage(coverageMetric);
}

@@ -216,7 +234,7 @@ public Coverage getChangeCoverage(final CoverageMetric coverageMetric) {
* @return {@code true} if indirect coverage changes exist, else {@code false}
*/
public boolean hasIndirectCoverageChanges() {
return getResult().hasIndirectCoverageChanges();
return hasIndirectCoverageChanges(CoverageMetric.LINE) || hasIndirectCoverageChanges(CoverageMetric.BRANCH);
}

/**
@@ -228,7 +246,7 @@ public boolean hasIndirectCoverageChanges() {
* @return {@code true} if indirect coverage changes exist for the metric, else {@code false}
*/
public boolean hasIndirectCoverageChanges(final CoverageMetric coverageMetric) {
return getResult().hasIndirectCoverageChanges(coverageMetric);
return indirectCoverageChanges.containsKey(coverageMetric);
}

/**
@@ -240,6 +258,7 @@ public boolean hasIndirectCoverageChanges(final CoverageMetric coverageMetric) {
* @return the indirect coverage changes
*/
public Coverage getIndirectCoverageChanges(final CoverageMetric coverageMetric) {
// FIXME: is percentage sufficient?
return getResult().getIndirectCoverageChangesTree().getCoverage(coverageMetric);
}

Original file line number Diff line number Diff line change
@@ -134,7 +134,8 @@ public void run(final CoverageResult rootResult, final Run<?, ?> build, final Fi
}

action = new CoverageBuildAction(build, rootNode, healthReport,
referenceAction.getOwner().getExternalizableId(), coverageDelta,
referenceAction.getOwner().getExternalizableId(),
coverageDelta,
changeCoverageRoot.getMetricPercentages(),
changeCoverageDelta,
indirectCoverageChangesTree.getMetricPercentages());
Original file line number Diff line number Diff line change
@@ -8,9 +8,12 @@
import org.junit.jupiter.api.Test;

import hudson.Functions;
import hudson.model.FreeStyleBuild;
import hudson.model.HealthReport;
import hudson.model.Run;

import io.jenkins.plugins.coverage.model.Coverage.CoverageBuilder;

import static io.jenkins.plugins.coverage.model.testutil.CoverageStubs.*;
import static io.jenkins.plugins.coverage.model.testutil.JobStubs.*;
import static org.assertj.core.api.Assertions.*;
@@ -22,7 +25,6 @@
* @author Ullrich Hafner
*/
class CoverageBuildActionTest {

private static final Locale LOCALE = Functions.getCurrentLocale();

private static final Fraction COVERAGE_FRACTION = Fraction.ONE_HALF;
@@ -32,6 +34,93 @@ class CoverageBuildActionTest {
private static final int COVERAGE_FILE_CHANGES = 5;
private static final long COVERAGE_LINE_CHANGES = 10;

@Test
void shouldNotLoadResultIfCoverageValuesArePersistedInAction() {
CoverageNode module = new CoverageNode(CoverageMetric.MODULE, "module");

CoverageBuilder coverageBuilder = new CoverageBuilder();

Coverage percent50 = coverageBuilder.setCovered(1).setMissed(1).build();
module.add(new CoverageLeaf(CoverageMetric.BRANCH, percent50));

Coverage percent80 = coverageBuilder.setCovered(8).setMissed(2).build();
module.add(new CoverageLeaf(CoverageMetric.LINE, percent80));

CoverageBuildAction action = spy(createEmptyAction(module));
when(action.getResult()).thenThrow(new IllegalStateException("Result should not be accessed with getResult() when getting a coverage metric that is persisted in the build"));

assertThat(action.hasCoverage(CoverageMetric.LINE)).isTrue();
assertThat(action.getCoverage(CoverageMetric.LINE)).isEqualTo(percent80);
assertThat(action.hasCoverage(CoverageMetric.BRANCH)).isTrue();
assertThat(action.getCoverage(CoverageMetric.BRANCH)).isEqualTo(percent50);

assertThatIllegalStateException().isThrownBy(
() -> action.hasCoverage(CoverageMetric.INSTRUCTION));
assertThatIllegalStateException().isThrownBy(
() -> action.getCoverage(CoverageMetric.INSTRUCTION));

assertThat(action.formatChangeCoverage(CoverageMetric.BRANCH)).isEqualTo("Branch: n/a");
assertThat(action.formatChangeCoverageOverview()).isEqualTo("n/a");

assertThat(action.formatIndirectCoverageChanges(CoverageMetric.BRANCH)).isEqualTo("Branch: n/a");
assertThat(action.formatIndirectCoverageChangesOverview()).isEqualTo("n/a");

assertThat(action.formatChangeCoverageDifference(CoverageMetric.BRANCH)).isEqualTo("n/a");
assertThat(action.formatDelta(CoverageMetric.BRANCH)).isEqualTo("n/a");
}

private static CoverageBuildAction createEmptyAction(final CoverageNode module) {
return new CoverageBuildAction(mock(FreeStyleBuild.class), module, mock(HealthReport.class), "-",
new TreeMap<>(), new TreeMap<>(),
new TreeMap<>(), new TreeMap<>(), false);
}

@Test
void shouldNotLoadResultIfDeltasArePersistedInAction() {
SortedMap<CoverageMetric, CoveragePercentage> deltas = new TreeMap<>();

CoverageBuilder coverageBuilder = new CoverageBuilder();

CoveragePercentage percent50 = CoveragePercentage.valueOf(coverageBuilder.setCovered(1).setMissed(1).build()
.getCoveredFraction());
CoveragePercentage percent80 = CoveragePercentage.valueOf(coverageBuilder.setCovered(8).setMissed(2).build()
.getCoveredFraction());

deltas.put(CoverageMetric.BRANCH, percent50);
deltas.put(CoverageMetric.LINE, percent80);

CoverageBuildAction action = new CoverageBuildAction(mock(FreeStyleBuild.class),
new CoverageNode(CoverageMetric.MODULE, "module"),
mock(HealthReport.class), "-",
deltas, deltas,
deltas, deltas, false);

CoverageBuildAction spy = spy(action);
when(spy.getResult()).thenThrow(new IllegalArgumentException("Result should not be accessed with getResult() when getting a coverage metric that is persisted in the build"));

assertThat(spy.hasChangeCoverage()).isTrue();
assertThat(spy.hasChangeCoverage(CoverageMetric.LINE)).isTrue();
assertThat(spy.hasChangeCoverage(CoverageMetric.BRANCH)).isTrue();
// FIXME: those values are not persisted yet
// assertThat(spy.getChangeCoverage(CoverageMetric.LINE)).isEqualTo(percent80);

assertThat(spy.hasIndirectCoverageChanges()).isTrue();
assertThat(spy.hasIndirectCoverageChanges(CoverageMetric.LINE)).isTrue();
assertThat(spy.hasIndirectCoverageChanges(CoverageMetric.BRANCH)).isTrue();
// FIXME: those values are not persisted yet
// assertThat(spy.getIndirectCoverageChanges(CoverageMetric.LINE)).isEqualTo(percent80);

assertThat(spy.hasChangeCoverageDifference(CoverageMetric.LINE)).isTrue();
assertThat(spy.hasChangeCoverageDifference(CoverageMetric.BRANCH)).isTrue();
// FIXME: those values are not persisted yet
// assertThat(spy.getIndirectCoverageChanges(CoverageMetric.LINE)).isEqualTo(percent80);

assertThat(spy.hasDelta(CoverageMetric.LINE)).isTrue();
assertThat(spy.hasDelta(CoverageMetric.BRANCH)).isTrue();
assertThat(spy.getDifference()).contains(entry(CoverageMetric.LINE, percent80));
assertThat(spy.getDifference()).contains(entry(CoverageMetric.BRANCH, percent50));
}

@Test
void shouldCreateViewModel() {
Run<?, ?> build = mock(Run.class);
@@ -120,24 +209,6 @@ void shouldFormatChangeCoverageDifference() {
assertThat(action.formatChangeCoverageDifference(COVERAGE_METRIC)).isEqualTo(expected);
}

@Test
void shouldFormatNotAvailableCoverageValues() {
CoverageNode root = createCoverageNode(COVERAGE_FRACTION, CoverageMetric.BRANCH);
when(root.hasChangeCoverage()).thenReturn(false);
when(root.hasIndirectCoverageChanges()).thenReturn(false);

CoverageBuildAction action = createCoverageBuildAction(root);

assertThat(action.formatChangeCoverage(CoverageMetric.BRANCH)).isEqualTo("Branch: n/a");
assertThat(action.formatChangeCoverageOverview()).isEqualTo("n/a");

assertThat(action.formatIndirectCoverageChanges(CoverageMetric.BRANCH)).isEqualTo("Branch: n/a");
assertThat(action.formatIndirectCoverageChangesOverview()).isEqualTo("n/a");

assertThat(action.formatChangeCoverageDifference(CoverageMetric.BRANCH)).isEqualTo("n/a");
assertThat(action.formatDelta(CoverageMetric.BRANCH)).isEqualTo("n/a");
}

/**
* Creates a {@link CoverageBuildAction} which represents the coverage for the metric {@link #COVERAGE_METRIC} with
* the value {@link #COVERAGE_PERCENTAGE}.

0 comments on commit 1db93ae

Please sign in to comment.