Skip to content

Commit

Permalink
Merge pull request #1006 from jenkinsci/new-issues-in-changes
Browse files Browse the repository at this point in the history 
Mark issues that are part of changed code in the issue difference
  • Loading branch information
@uhafner
uhafner authored Jan 19, 2024
2 parents 290efa8 + 3e48937 commit d4c1ec4
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 34 deletions.
39 changes: 29 additions & 10 deletions src/main/java/edu/hm/hafner/analysis/IssueDifference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import java.util.Map.Entry;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

Expand All @@ -18,9 +17,12 @@
*
* @author Ullrich Hafner
*/
@SuppressWarnings("PMD.DataClass")
public class IssueDifference {
private static final List<Issue> EMPTY = Collections.emptyList();

private final Report newIssues;
private final Report newIssuesInChangedCode;
private final Report fixedIssues;
private final Report outstandingIssues;
private final Map<Integer, List<Issue>> referencesByHash;
Expand Down Expand Up @@ -51,13 +53,15 @@ public IssueDifference(final Report currentIssues, final String referenceId,
* @param referenceIssues
* the issues of a previous report (reference)
* @param includes
* lines in files mapping to include as new issues, all other issues are considered as outstanding
* A mapping of files to changed lines. Using this mapping, we can identify which new issues are part
* of the changes and which issues are indirectly caused by the changes.
*/
public IssueDifference(final Report currentIssues, final String referenceId,
final Report referenceIssues, final Map<String, Integer> includes) {
newIssues = currentIssues.copy();
fixedIssues = referenceIssues.copy();
outstandingIssues = new Report();
newIssuesInChangedCode = new Report();

referencesByHash = new HashMap<>();
referencesByFingerprint = new HashMap<>();
Expand All @@ -72,20 +76,20 @@ public IssueDifference(final Report currentIssues, final String referenceId,
removed.forEach(secondPass::remove);
matchIssuesByFingerprint(secondPass);

newIssues.forEach(issue -> issue.setReference(referenceId));
if (!includes.isEmpty()) {
keepOnlyIncludedIssues(includes);
findIssuesInChangedCode(includes);
}
newIssues.forEach(issue -> issue.setReference(referenceId));
}

private void keepOnlyIncludedIssues(final Map<String, Integer> includes) {
var idsToRemove = newIssues.stream().map(Issue::getId).collect(Collectors.toSet());
private void findIssuesInChangedCode(final Map<String, Integer> includes) {
for (Entry<String, Integer> include : includes.entrySet()) {
newIssues.filter(issue -> filter(issue, include.getKey(), include.getValue()))
.stream().map(Issue::getId)
.forEach(idsToRemove::remove);
.stream()
.map(Issue::getId)
.map(newIssues::remove)
.forEach(newIssuesInChangedCode::add);
}
idsToRemove.stream().map(newIssues::remove).forEach(outstandingIssues::add);
}

private boolean filter(final Issue issue, final String fileName, final int line) {
Expand Down Expand Up @@ -173,7 +177,8 @@ public Report getOutstandingIssues() {

/**
* Returns the new issues. I.e., all issues that are part of the current report but that have not been shown up in
* the previous report.
* the previous report. If the difference is computed for a specific set of changed files, then this set contains
* only the new issues that are not part of the changes. These issues are indirectly caused by the changes.
*
* @return the new issues
*/
Expand All @@ -182,6 +187,20 @@ public Report getNewIssues() {
return newIssues;
}

/**
* Returns the new issues that are part of the changed code lines. I.e., all issues that are part of the current
* report but that have not been shown up in the previous report. If the difference is computed for a specific
* set of changed files, then this set contains only the new issues that are part of the changes. Otherwise, this
* set will be empty.
*
* @return the new issues
* @see IssueDifference#getNewIssues()
*/
@SuppressFBWarnings("EI")
public Report getNewIssuesInChangedCode() {
return newIssuesInChangedCode;
}

/**
* Returns the fixed issues. I.e., all issues that are part of the previous report but that are not present in the
* current report anymore.
Expand Down
74 changes: 50 additions & 24 deletions src/test/java/edu/hm/hafner/analysis/IssueDifferenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,53 +38,77 @@ void shouldFilterByPath() {
createIssue("NEW 3", "NEW 3", "/include/me"));

IssueDifference everything = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues);
assertThatReportContainsExactly(everything.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReportContainsExactly(everything.getNewIssues(),
"NEW 1", "NEW 2", "NEW 3");
assertThatReportContainsExactly(everything.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(everything.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(everything.getNewIssues(), "NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(everything.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(everything.getNewIssuesInChangedCode());
assertThatReportContainsExactly(everything.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(everything.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference included = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("/include/me", 0));
assertThatReportContainsExactly(included.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReportContainsExactly(included.getNewIssues(),
"NEW 1", "NEW 3");
assertThatReportContainsExactly(included.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(included.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(included.getNewIssuesInChangedCode(), "NEW 1", "NEW 3");
assertThatReferenceIsSet(included.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(included.getNewIssues(), "NEW 2");
assertThatReferenceIsSet(included.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(included.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1", "NEW 2");
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(included.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference sameSuffixForAll = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("me", 0));
assertThatReportContainsExactly(sameSuffixForAll.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReportContainsExactly(sameSuffixForAll.getNewIssues(),
assertThatReportContainsExactly(sameSuffixForAll.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(sameSuffixForAll.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(sameSuffixForAll.getNewIssuesInChangedCode(),
"NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(sameSuffixForAll.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(sameSuffixForAll.getNewIssues());
assertThatReportContainsExactly(sameSuffixForAll.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(sameSuffixForAll.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference allFiles = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("/include/me", 0, "/remove/me", 0));
assertThatReportContainsExactly(allFiles.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReportContainsExactly(allFiles.getNewIssues(),
assertThatReportContainsExactly(allFiles.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(allFiles.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(allFiles.getNewIssuesInChangedCode(),
"NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(allFiles.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(allFiles.getNewIssues());
assertThatReportContainsExactly(allFiles.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(allFiles.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference nothingNew = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("other", 0));
assertThatReportContainsExactly(nothingNew.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReportContainsExactly(nothingNew.getNewIssues());
assertThatReferenceIsSet(nothingNew.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(nothingNew.getNewIssuesInChangedCode());
assertThatReportContainsExactly(nothingNew.getNewIssues(), "NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(nothingNew.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(nothingNew.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1", "NEW 1", "NEW 2", "NEW 3");
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(nothingNew.getOutstandingIssues(), REFERENCE_BUILD);
}

private void assertThatReferenceIsSet(final Report report, final String referenceBuild) {
assertThat(report.get()).extracting(Issue::getReference).containsOnly(referenceBuild);
}

private void assertThatReportContainsExactly(final Report report, final String... messages) {
assertThat(report.get())
.extracting(Issue::getMessage)
.containsExactlyInAnyOrder(messages);
if (messages.length == 0) {
assertThat(report).isEmpty();
}
else {
assertThat(report.get())
.extracting(Issue::getMessage)
.containsExactlyInAnyOrder(messages);
}
}

@Test
Expand Down Expand Up @@ -120,8 +144,8 @@ void shouldCreateIssueDifferenceBasedOnPropertiesAndThenOnFingerprint() {
}

/**
* Verifies that issue difference report has only outstanding issues when the current report and reference report have
* same issues.
* Verifies that issue difference report has only outstanding issues when the current report and reference report
* have same issues.
*/
@Test
void shouldCreateOutstandingIssueDifference() {
Expand Down Expand Up @@ -183,7 +207,8 @@ void shouldCreateIssueDifferenceWithEmptyReference() {
assertThat(newIssues.get(1)).hasMessage("NEW 2").hasReference(CURRENT_BUILD);
}

@Test @org.junitpioneer.jupiter.Issue("JENKINS-56324")
@Test
@org.junitpioneer.jupiter.Issue("JENKINS-56324")
void shouldAlsoUseFingerprintIfIssuesAreEqual() {
Report referenceIssues = new Report().addAll(
createIssue("OLD 1", "FP"));
Expand Down Expand Up @@ -246,7 +271,8 @@ private Issue createIssue(final String message, final String fingerprint, final
}
}

@Test @org.junitpioneer.jupiter.Issue("JENKINS-65482")
@Test
@org.junitpioneer.jupiter.Issue("JENKINS-65482")
void shouldHandleAggregatedResults() {
Report firstAxis = readSpotBugsWarnings();
assertThat(firstAxis).hasSize(2);
Expand Down

0 comments on commit d4c1ec4

Please sign in to comment.