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 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fe45ac2
Add Modified Files Coverage and GitHub Checks API
fo-code Feb 5, 2023
5686bea
Add Modified Files Coverage and GitHub Checks API
fo-code Feb 5, 2023
444c8ca
Merge branch 'file-change-coverage' of https://github.com/fo-code/cod…
fo-code Feb 11, 2023
bf9b3a5
Rename Change Coverage to Modified Lines Coverage an fix tests
fo-code Feb 11, 2023
42a5ee9
Renaming of changes.
uhafner Feb 14, 2023
c35c60a
Fix checks publisher tests
fo-code Feb 16, 2023
eb78397
Add a toggle to show only changed files in absolute coverage table.
uhafner Feb 28, 2023
14f40ac
Merge branch 'coverage-model' into file-change-coverage
uhafner Mar 1, 2023
24c9815
Merge with latest renames in coverage model.
uhafner Mar 1, 2023
f727183
Fix typo.
uhafner Mar 1, 2023
8c97776
Fix typo.
uhafner Mar 1, 2023
8719661
Fix typo.
uhafner Mar 1, 2023
9c05d7a
Remove @since
uhafner Mar 1, 2023
5709d5c
Remove @since
uhafner Mar 1, 2023
96b4228
Do not create tree nodes that do not have a value attached.
uhafner Mar 1, 2023
7eb5c4f
Add incremental versions of dependencies.
uhafner Mar 1, 2023
cacc7df
Add incremental version of forensics-plugin.
uhafner Mar 1, 2023
495ece7
Fix for review comments
fo-code Mar 1, 2023
1a6c504
Use latest incremental version of plugin-util.
uhafner Mar 3, 2023
f41892d
Add customization options for checks.
uhafner Mar 3, 2023
af27993
Add different types of single lines.
uhafner Mar 7, 2023
afb110e
Bump version of coverage-model to 0.15.0.
uhafner Mar 7, 2023
d7f922e
Delete unused table model.
uhafner Mar 7, 2023
5b483c4
Simplify assertions.
uhafner Mar 7, 2023
377d093
Add test for ALL_LINES.
uhafner Mar 7, 2023
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 @@ -27,23 +27,23 @@ public enum Baseline {
* Coverage of the modified lines (e.g., within the modified lines of a pull or merge request) will focus on new or
* modified code only.
*/
MODIFIED_LINES(Messages._Baseline_MODIFIED_LINES(), "changeCoverage", CoverageLevel::getDisplayColorsOfCoverageLevel),
MODIFIED_LINES(Messages._Baseline_MODIFIED_LINES(), "modifiedLinesCoverage", CoverageLevel::getDisplayColorsOfCoverageLevel),
/**
* Difference between the project coverage and the modified lines coverage of the current build. Teams can use this delta
* value to ensure that the coverage of pull requests is better than the whole project coverage.
*/
MODIFIED_LINES_DELTA(Messages._Baseline_MODIFIED_LINES_DELTA(), "changeCoverage",
MODIFIED_LINES_DELTA(Messages._Baseline_MODIFIED_LINES_DELTA(), "modifiedLinesCoverage",
CoverageChangeTendency::getDisplayColorsForTendency),
/**
* Coverage of the modified files (e.g., within the files that have been touched in a pull or merge request) will
* focus on new or modified code only.
*/
MODIFIED_FILES(Messages._Baseline_MODIFIED_FILES(), "fileCoverage", CoverageLevel::getDisplayColorsOfCoverageLevel),
MODIFIED_FILES(Messages._Baseline_MODIFIED_FILES(), "modifiedFilesCoverage", CoverageLevel::getDisplayColorsOfCoverageLevel),
/**
* Difference between the project coverage and the modified file coverage of the current build. Teams can use this delta
* value to ensure that the coverage of pull requests is better than the whole project coverage.
*/
MODIFIED_FILES_DELTA(Messages._Baseline_MODIFIED_FILES_DELTA(), "fileCoverage", CoverageChangeTendency::getDisplayColorsForTendency),
MODIFIED_FILES_DELTA(Messages._Baseline_MODIFIED_FILES_DELTA(), "modifiedFilesCoverage", CoverageChangeTendency::getDisplayColorsForTendency),
Copy link
Member

@uhafner uhafner Feb 28, 2023

Choose a reason for hiding this comment

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

While looking at the results I think I made a mistake here. When I am developing a PR normally I am interested in the coverage of my changes.

That means we need the totals (this is already working correctly):

  • coverage overall project
  • changed files
  • changed lines

But when looking at the delta we need:

  • delta overall project - delta overall reference (typically not interesting for large projects)
  • delta changed files - delta changed files reference (how is the coverage of the changed files affected)? Currently we check how it changes with respect to the overall coverage.
  • delta changed lines: here I am not sure whether it makes more sense to compare vs. the whole project or the changed files. What do you think? Or do we need both?

Bildschirm­foto 2023-02-28 um 12 00 31

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that makes sense.
Regarding the delta changed lines: I guess we should compare only against the modified files coverage. The whole project has modules with higher and with lower coverage. When we compare the changes of a module with lower coverage against the whole project, there will be a negative trend, even if the modified files are covered quiet good compared to the rest of their module. Therefore, comparing against the changed files would keep the context hence it more accurate, in my opinion.

/**
* Indirect changes of the overall code coverage that are not part of the changed code. These changes might occur,
* if new tests will be added without touching the underlying code under test.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package io.jenkins.plugins.coverage.metrics.model;

import java.util.Comparator;
import java.util.List;
import java.util.Locale;
import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.Fraction;
Expand All @@ -11,6 +16,7 @@
import edu.hm.hafner.metric.FractionValue;
import edu.hm.hafner.metric.IntegerValue;
import edu.hm.hafner.metric.Metric;
import edu.hm.hafner.metric.Node;
import edu.hm.hafner.metric.Percentage;
import edu.hm.hafner.metric.Value;

Expand Down Expand Up @@ -370,6 +376,51 @@ public String getDisplayName(final Metric metric) {
}
}

/**
* Gets the display names of the existing {@link Metric coverage metrics}, sorted by the metrics ordinal.
*
* @return the sorted metric display names
*/
public List<String> getSortedCoverageDisplayNames() {
return Metric.getCoverageMetrics().stream()
// use the default comparator which uses the enum ordinal
.sorted()
.map(this::getDisplayName)
.collect(Collectors.toList());
}

/**
* Formats a stream of values to theis display representation by using the passed locale.
*
* @param values
* The values to be formatted
* @param locale
* The locale to be used for formatting
*
* @return the formatted values in the origin order of the stream
*/
public List<String> getFormattedValues(final Stream<? extends Value> values, final Locale locale) {
return values.map(value -> formatDetails(value, locale)).collect(Collectors.toList());
}

/**
* Returns a stream of {@link Coverage} values for the given root node sorted by the metric ordinal.
*
* @param coverage
* The coverage root node
*
* @return a stream containing the existent coverage values
*/
public Stream<Coverage> getSortedCoverageValues(final Node coverage) {
return Metric.getCoverageMetrics()
.stream()
.map(m -> m.getValueFor(coverage))
.flatMap(Optional::stream)
.filter(value -> value instanceof Coverage)
.map(Coverage.class::cast)
.sorted(Comparator.comparing(Coverage::getMetric));
}

/**
* Returns a localized human-readable label for the specified metric.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ File createFileInBuildFolder(final File buildResults, final String id, final Str
}

/**
* Filters the sourcecode coverage highlighting for analyzing the change coverage only.
* Filters the sourcecode coverage highlighting for analyzing the Modified Lines Coverage only.
*
* @param content
* The original HTML content
Expand All @@ -172,7 +172,7 @@ File createFileInBuildFolder(final File buildResults, final String id, final Str
*
* @return the filtered HTML sourcecode view
*/
public String calculateChangeCoverageSourceCode(final String content, final FileNode fileNode) {
public String calculateModifiedLinesCoverageSourceCode(final String content, final FileNode fileNode) {
Set<Integer> lines = fileNode.getLinesWithCoverage();
lines.retainAll(fileNode.getChangedLines());
Set<String> linesAsText = lines.stream().map(String::valueOf).collect(Collectors.toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ FileNode getOriginalFile() {

@Override
public DetailedCell<?> getLineCoverageDelta() {
return createColoredChangeCoverageDeltaColumn(Metric.LINE);
return createColoredModifiedLinesCoverageDeltaColumn(Metric.LINE);
}

@Override
public DetailedCell<?> getBranchCoverageDelta() {
return createColoredChangeCoverageDeltaColumn(Metric.BRANCH);
return createColoredModifiedLinesCoverageDeltaColumn(Metric.BRANCH);
}

DetailedCell<?> createColoredChangeCoverageDeltaColumn(final Metric metric) {
Coverage changeCoverage = getFile().getTypedValue(metric, Coverage.nullObject(metric));
if (changeCoverage.isSet()) {
DetailedCell<?> createColoredModifiedLinesCoverageDeltaColumn(final Metric metric) {
Coverage modifiedLinesCoverage = getFile().getTypedValue(metric, Coverage.nullObject(metric));
if (modifiedLinesCoverage.isSet()) {
return createColoredCoverageDeltaColumn(metric,
changeCoverage.delta(originalFile.getTypedValue(metric, Coverage.nullObject(metric))));
modifiedLinesCoverage.delta(originalFile.getTypedValue(metric, Coverage.nullObject(metric))));
}
return NO_COVERAGE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,16 @@ public final class CoverageBuildAction extends BuildAction<Node> implements Stap
private final NavigableMap<Metric, Fraction> difference;

/** The coverages filtered by changed lines of the associated change request. */
private final List<? extends Value> changeCoverage;
private final List<? extends Value> modifiedLinesCoverage;

/** The delta of the coverages of the associated change request with respect to the reference build. */
private final NavigableMap<Metric, Fraction> changeCoverageDifference;
private final NavigableMap<Metric, Fraction> modifiedLinesCoverageDifference;

/** The coverage of the modified lines. */
private final List<? extends Value> modifiedFilesCoverage;

/** The coverage delta of the modified lines. */
private final NavigableMap<Metric, Fraction> modifiedFilesCoverageDifference;

/** The indirect coverage changes of the associated change request with respect to the reference build. */
private final List<? extends Value> indirectCoverageChanges;
Expand All @@ -87,7 +93,7 @@ public final class CoverageBuildAction extends BuildAction<Node> implements Stap
CoverageXmlStream.registerConverters(XSTREAM2);
XSTREAM2.registerLocalConverter(CoverageBuildAction.class, "difference",
new MetricFractionMapConverter());
XSTREAM2.registerLocalConverter(CoverageBuildAction.class, "changeCoverageDifference",
XSTREAM2.registerLocalConverter(CoverageBuildAction.class, "modifiedLinesCoverageDifference",
new MetricFractionMapConverter());
}

Expand All @@ -112,7 +118,7 @@ public final class CoverageBuildAction extends BuildAction<Node> implements Stap
public CoverageBuildAction(final Run<?, ?> owner, final String id, final String optionalName, final String icon,
final Node result, final QualityGateResult qualityGateResult, final FilteredLog log) {
this(owner, id, optionalName, icon, result, qualityGateResult, log, NO_REFERENCE_BUILD,
new TreeMap<>(), List.of(), new TreeMap<>(), List.of());
new TreeMap<>(), List.of(), new TreeMap<>(), List.of(), new TreeMap<>(), List.of());
}

/**
Expand All @@ -136,9 +142,9 @@ public CoverageBuildAction(final Run<?, ?> owner, final String id, final String
* the ID of the reference build
* @param delta
* delta of this build's coverages with respect to the reference build
* @param changeCoverage
* @param modifiedLinesCoverage
* the coverages filtered by changed lines of the associated change request
* @param changeCoverageDifference
* @param modifiedLinesCoverageDifference
* the delta of the coverages of the associated change request with respect to the reference build
* @param indirectCoverageChanges
* the indirect coverage changes of the associated change request with respect to the reference build
Expand All @@ -148,11 +154,16 @@ public CoverageBuildAction(final Run<?, ?> owner, final String id, final String
final Node result, final QualityGateResult qualityGateResult, final FilteredLog log,
final String referenceBuildId,
final NavigableMap<Metric, Fraction> delta,
final List<? extends Value> changeCoverage,
final NavigableMap<Metric, Fraction> changeCoverageDifference,
final List<? extends Value> modifiedLinesCoverage,
final NavigableMap<Metric, Fraction> modifiedLinesCoverageDifference,
final List<? extends Value> modifiedFilesCoverage,
final NavigableMap<Metric, Fraction> modifiedFilesCoverageDifference,
final List<? extends Value> indirectCoverageChanges) {
this(owner, id, optionalName, icon, result, qualityGateResult, log, referenceBuildId, delta, changeCoverage,
changeCoverageDifference, indirectCoverageChanges, true);
this(owner, id, optionalName, icon, result, qualityGateResult, log, referenceBuildId, delta,
modifiedLinesCoverage,
modifiedLinesCoverageDifference, modifiedFilesCoverage, modifiedFilesCoverageDifference,
indirectCoverageChanges,
true);
}

@VisibleForTesting
Expand All @@ -161,8 +172,10 @@ public CoverageBuildAction(final Run<?, ?> owner, final String id, final String
final Node result, final QualityGateResult qualityGateResult, final FilteredLog log,
final String referenceBuildId,
final NavigableMap<Metric, Fraction> delta,
final List<? extends Value> changeCoverage,
final NavigableMap<Metric, Fraction> changeCoverageDifference,
final List<? extends Value> modifiedLinesCoverage,
final NavigableMap<Metric, Fraction> modifiedLinesCoverageDifference,
final List<? extends Value> modifiedFilesCoverage,
final NavigableMap<Metric, Fraction> modifiedFilesCoverageDifference,
final List<? extends Value> indirectCoverageChanges,
final boolean canSerialize) {
super(owner, result, false);
Expand All @@ -175,8 +188,10 @@ public CoverageBuildAction(final Run<?, ?> owner, final String id, final String
projectValues = result.aggregateValues();
this.qualityGateResult = qualityGateResult;
difference = delta;
this.changeCoverage = new ArrayList<>(changeCoverage);
this.changeCoverageDifference = changeCoverageDifference;
this.modifiedLinesCoverage = new ArrayList<>(modifiedLinesCoverage);
this.modifiedLinesCoverageDifference = modifiedLinesCoverageDifference;
this.modifiedFilesCoverage = new ArrayList<>(modifiedFilesCoverage);
this.modifiedFilesCoverageDifference = modifiedFilesCoverageDifference;
this.indirectCoverageChanges = new ArrayList<>(indirectCoverageChanges);
this.referenceBuildId = referenceBuildId;

Expand Down Expand Up @@ -207,8 +222,8 @@ public ElementFormatter getFormatter() {
}

public CoverageStatistics getStatistics() {
return new CoverageStatistics(projectValues, difference, changeCoverage, changeCoverageDifference,
List.of(), new TreeMap<>());
return new CoverageStatistics(projectValues, difference, modifiedLinesCoverage, modifiedLinesCoverageDifference,
modifiedFilesCoverage, modifiedFilesCoverageDifference);
}

/**
Expand All @@ -218,7 +233,7 @@ public CoverageStatistics getStatistics() {
*/
@SuppressWarnings("unused") // Called by jelly view
public List<Baseline> getBaselines() {
return List.of(Baseline.PROJECT, Baseline.MODIFIED_LINES, Baseline.INDIRECT);
return List.of(Baseline.PROJECT, Baseline.MODIFIED_FILES, Baseline.MODIFIED_LINES, Baseline.INDIRECT);
}

/**
Expand Down Expand Up @@ -273,6 +288,19 @@ public List<Value> getAllValues(final Baseline baseline) {
return getValueStream(baseline).collect(Collectors.toList());
}

public NavigableMap<Metric, Fraction> getAllDeltas(final Baseline deltaBaseline) {
if (deltaBaseline == Baseline.PROJECT_DELTA) {
return difference;
}
else if (deltaBaseline == Baseline.MODIFIED_LINES_DELTA) {
return modifiedLinesCoverageDifference;
}
else if (deltaBaseline == Baseline.MODIFIED_FILES_DELTA) {
return modifiedFilesCoverageDifference;
}
throw new NoSuchElementException("No delta baseline: " + deltaBaseline);
}

/**
* Returns all important values for the specified baseline.
*
Expand All @@ -288,6 +316,12 @@ public List<Value> getValues(final Baseline baseline) {
return filterImportantMetrics(getValueStream(baseline));
}

public Optional<Value> getValueForMetric(final Baseline baseline, final Metric metric) {
return getAllValues(baseline).stream()
.filter(value -> value.getMetric() == metric)
.findFirst();
}

private List<Value> filterImportantMetrics(final Stream<? extends Value> values) {
return values.filter(v -> getMetricsForSummary().contains(v.getMetric()))
.collect(Collectors.toList());
Expand All @@ -298,7 +332,10 @@ private Stream<? extends Value> getValueStream(final Baseline baseline) {
return projectValues.stream();
}
if (baseline == Baseline.MODIFIED_LINES) {
return changeCoverage.stream();
return modifiedLinesCoverage.stream();
}
if (baseline == Baseline.MODIFIED_FILES) {
return modifiedFilesCoverage.stream();
}
if (baseline == Baseline.INDIRECT) {
return indirectCoverageChanges.stream();
Expand All @@ -316,7 +353,8 @@ private Stream<? extends Value> getValueStream(final Baseline baseline) {
*/
@SuppressWarnings("unused") // Called by jelly view
public boolean hasDelta(final Baseline baseline) {
return baseline == Baseline.PROJECT || baseline == Baseline.MODIFIED_LINES;
return baseline == Baseline.PROJECT || baseline == Baseline.MODIFIED_LINES
|| baseline == Baseline.MODIFIED_FILES;
}

/**
Expand All @@ -334,7 +372,11 @@ public boolean hasDelta(final Baseline baseline, final Metric metric) {
return difference.containsKey(metric);
}
if (baseline == Baseline.MODIFIED_LINES) {
return changeCoverageDifference.containsKey(metric)
return modifiedLinesCoverageDifference.containsKey(metric)
&& Set.of(Metric.BRANCH, Metric.LINE).contains(metric);
}
if (baseline == Baseline.MODIFIED_FILES) {
return modifiedFilesCoverageDifference.containsKey(metric)
&& Set.of(Metric.BRANCH, Metric.LINE).contains(metric);
}
if (baseline == Baseline.INDIRECT) {
Expand All @@ -343,6 +385,39 @@ public boolean hasDelta(final Baseline baseline, final Metric metric) {
throw new NoSuchElementException("No such baseline: " + baseline);
}

/**
* Returns whether a value for the specified metric exists.
*
* @param baseline
* the baseline to use
* @param metric
* the metric to check
*
* @return {@code true} if a value is available for the specified metric, {@code false} otherwise
*/
public boolean hasValue(final Baseline baseline, final Metric metric) {
return getAllValues(baseline).stream()
.anyMatch(v -> v.getMetric() == metric);
}

/**
* Returns a formatted and localized String representation of the value for the specified metric (with respect to
* the given baseline).
*
* @param baseline
* the baseline to use
* @param metric
* the metric to get the delta for
*
* @return the formatted value
*/
public String formatValue(final Baseline baseline, final Metric metric) {
var value = getAllValues(baseline).stream()
.filter(v -> v.getMetric() == metric)
.findFirst();
return value.isPresent() ? FORMATTER.formatValue(value.get()) : Messages.Coverage_Not_Available();
}

/**
* Returns a formatted and localized String representation of the delta for the specified metric (with respect to
* the given baseline).
Expand All @@ -364,7 +439,13 @@ public String formatDelta(final Baseline baseline, final Metric metric) {
}
if (baseline == Baseline.MODIFIED_LINES) {
if (hasDelta(baseline, metric)) {
return FORMATTER.formatDelta(changeCoverageDifference.get(metric), metric,
return FORMATTER.formatDelta(modifiedLinesCoverageDifference.get(metric), metric,
Functions.getCurrentLocale());
}
}
if (baseline == Baseline.MODIFIED_FILES) {
if (hasDelta(baseline, metric)) {
return FORMATTER.formatDelta(modifiedFilesCoverageDifference.get(metric), metric,
Functions.getCurrentLocale());
}
}
Expand Down
Loading