-
-
Notifications
You must be signed in to change notification settings - Fork 532
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
#945: Gather statistics for issues fixed in a pull request
Sonarqube currently reports a fixed issues metric for pull requests, but the plugin isn't providing the data to allow that value to be calculated. To resolve this an additional IssueVisitor has been introduced that compares the issues from the target branch with the findings on the source branch and finds any target code blocks that no longer exists - implying the issue line has been removed - or any code that still exists but is now reporting the issue as fixed, and reports them to the PullRequestFixedIssuesRepository which is used within Sonarqube to gather the count of issues fixed in the current analysis.
- Loading branch information
Showing
4 changed files
with
250 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
...om/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitor.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| /* | ||
| * Copyright (C) 2024 Michael Clarke | ||
| * | ||
| * This program is free software; you can redistribute it and/or | ||
| * modify it under the terms of the GNU Lesser General Public | ||
| * License as published by the Free Software Foundation; either | ||
| * version 3 of the License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
| * Lesser General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Lesser General Public License | ||
| * along with this program; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
| * | ||
| */ | ||
| package com.github.mc1arke.sonarqube.plugin.ce.pullrequest; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import org.sonar.api.issue.IssueStatus; | ||
| import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; | ||
| import org.sonar.ce.task.projectanalysis.component.Component; | ||
| import org.sonar.ce.task.projectanalysis.issue.DefaultTrackingInput; | ||
| import org.sonar.ce.task.projectanalysis.issue.IssueVisitor; | ||
| import org.sonar.ce.task.projectanalysis.issue.fixedissues.PullRequestFixedIssueRepository; | ||
| import org.sonar.core.issue.DefaultIssue; | ||
| import org.sonar.core.issue.tracking.Input; | ||
| import org.sonar.core.issue.tracking.NonClosedTracking; | ||
| import org.sonar.core.issue.tracking.Tracker; | ||
| import org.sonar.core.issue.tracking.Tracking; | ||
|
|
||
| public class PullRequestFixedIssuesIssueVisitor extends IssueVisitor { | ||
|
|
||
| private static final List<IssueStatus> NON_CLOSED_ISSUE_STATUSES = List.of(IssueStatus.OPEN, IssueStatus.ACCEPTED, IssueStatus.CONFIRMED); | ||
|
|
||
| private final PullRequestFixedIssueRepository pullRequestFixedIssueRepository; | ||
| private final AnalysisMetadataHolder analysisMetadataHolder; | ||
| private final Tracker<DefaultIssue, DefaultIssue> tracker; | ||
|
|
||
| public PullRequestFixedIssuesIssueVisitor(PullRequestFixedIssueRepository pullRequestFixedIssueRepository, | ||
| AnalysisMetadataHolder analysisMetadataHolder, | ||
| Tracker<DefaultIssue, DefaultIssue> tracker) { | ||
| this.pullRequestFixedIssueRepository = pullRequestFixedIssueRepository; | ||
| this.analysisMetadataHolder = analysisMetadataHolder; | ||
| this.tracker = tracker; | ||
| } | ||
|
|
||
| @Override | ||
| public void onRawIssues(Component component, Input<DefaultIssue> rawIssues, Input<DefaultIssue> baseIssues) { | ||
| if (!analysisMetadataHolder.isPullRequest() || baseIssues == null) { | ||
| return; | ||
| } | ||
|
|
||
| for (DefaultIssue fixedIssue : findFixedIssues(rawIssues, baseIssues)) { | ||
| pullRequestFixedIssueRepository.addFixedIssue(fixedIssue); | ||
| } | ||
| } | ||
|
|
||
| private List<DefaultIssue> findFixedIssues(Input<DefaultIssue> rawIssues, Input<DefaultIssue> baseIssues) { | ||
| List<DefaultIssue> nonClosedBaseIssues = baseIssues.getIssues().stream() | ||
| .filter(issue -> Optional.ofNullable(issue.issueStatus()) | ||
| .map(NON_CLOSED_ISSUE_STATUSES::contains) | ||
| .orElse(false)) | ||
| .collect(Collectors.toList()); | ||
| Input<DefaultIssue> trackingIssues = new DefaultTrackingInput(nonClosedBaseIssues, baseIssues.getLineHashSequence(), baseIssues.getBlockHashSequence()); | ||
| NonClosedTracking<DefaultIssue, DefaultIssue> nonClosedTrackedBaseIssues = tracker.trackNonClosed(rawIssues, trackingIssues); | ||
|
|
||
| List<DefaultIssue> fixedIssues = new ArrayList<>(); | ||
| fixedIssues.addAll(findFixedIssues(nonClosedTrackedBaseIssues)); | ||
| fixedIssues.addAll(nonClosedTrackedBaseIssues.getUnmatchedBases().collect(Collectors.toList())); | ||
| return fixedIssues; | ||
| } | ||
|
|
||
| private static List<DefaultIssue> findFixedIssues(Tracking<DefaultIssue, DefaultIssue> nonClosedIssues) { | ||
| return nonClosedIssues.getMatchedRaws().entrySet().stream() | ||
| .filter(issueEntry -> issueEntry.getKey().issueStatus() == IssueStatus.FIXED) | ||
| .map(Map.Entry::getValue) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
...ithub/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitorTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| /* | ||
| * Copyright (C) 2024 Michael Clarke | ||
| * | ||
| * This program is free software; you can redistribute it and/or | ||
| * modify it under the terms of the GNU Lesser General Public | ||
| * License as published by the Free Software Foundation; either | ||
| * version 3 of the License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
| * Lesser General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Lesser General Public License | ||
| * along with this program; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
| * | ||
| */ | ||
| package com.github.mc1arke.sonarqube.plugin.ce.pullrequest; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.ArgumentMatchers.eq; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.mockito.Mockito.verifyNoInteractions; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.stream.Stream; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.ArgumentCaptor; | ||
| import org.sonar.api.issue.IssueStatus; | ||
| import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; | ||
| import org.sonar.ce.task.projectanalysis.component.Component; | ||
| import org.sonar.ce.task.projectanalysis.issue.fixedissues.PullRequestFixedIssueRepository; | ||
| import org.sonar.core.issue.DefaultIssue; | ||
| import org.sonar.core.issue.tracking.Input; | ||
| import org.sonar.core.issue.tracking.NonClosedTracking; | ||
| import org.sonar.core.issue.tracking.Tracker; | ||
|
|
||
| class PullRequestFixedIssuesIssueVisitorTest { | ||
|
|
||
| @Test | ||
| void shouldSkipVisitIfNotAPullRequest() { | ||
| AnalysisMetadataHolder analysisMetadataHolder = mock(); | ||
| PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); | ||
| Tracker<DefaultIssue, DefaultIssue> tracker = mock(); | ||
|
|
||
| when(analysisMetadataHolder.isPullRequest()).thenReturn(false); | ||
|
|
||
| Component component = mock(); | ||
| Input<DefaultIssue> rawIssues = mock(); | ||
| Input<DefaultIssue> baseIssues = mock(); | ||
|
|
||
| PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); | ||
| underTest.onRawIssues(component, rawIssues, baseIssues); | ||
|
|
||
| verify(analysisMetadataHolder).isPullRequest(); | ||
|
|
||
| verifyNoInteractions(pullRequestFixedIssuesRepository, tracker); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldSkipVisitIfPullRequestButNoBaseIssuesToFix() { | ||
| AnalysisMetadataHolder analysisMetadataHolder = mock(); | ||
| PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); | ||
| Tracker<DefaultIssue, DefaultIssue> tracker = mock(); | ||
|
|
||
| when(analysisMetadataHolder.isPullRequest()).thenReturn(true); | ||
|
|
||
| Component component = mock(); | ||
| Input<DefaultIssue> rawIssues = mock(); | ||
| Input<DefaultIssue> baseIssues = null; | ||
|
|
||
| PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); | ||
| underTest.onRawIssues(component, rawIssues, baseIssues); | ||
|
|
||
| verify(analysisMetadataHolder).isPullRequest(); | ||
|
|
||
| verifyNoInteractions(pullRequestFixedIssuesRepository, tracker); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldPassFixedAndRemovedIssuesToRepository() { | ||
| AnalysisMetadataHolder analysisMetadataHolder = mock(); | ||
| PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); | ||
| Tracker<DefaultIssue, DefaultIssue> tracker = mock(); | ||
|
|
||
| when(analysisMetadataHolder.isPullRequest()).thenReturn(true); | ||
|
|
||
| Component component = mock(); | ||
| Input<DefaultIssue> rawIssues = mock(); | ||
| List<DefaultIssue> baseIssueList = List.of(mockIssue(IssueStatus.FALSE_POSITIVE), mockIssue(IssueStatus.OPEN), mockIssue(IssueStatus.FIXED), mockIssue(IssueStatus.OPEN), mockIssue(IssueStatus.FALSE_POSITIVE), mockIssue(null)); | ||
| Input<DefaultIssue> baseIssues = mock(); | ||
| when(baseIssues.getIssues()).thenReturn(baseIssueList); | ||
|
|
||
| NonClosedTracking<DefaultIssue, DefaultIssue> nonClosedTracking = mock(); | ||
| Map<DefaultIssue, DefaultIssue> matchedRaws = Map.of(mockIssue(IssueStatus.OPEN), baseIssueList.get(1), mockIssue(IssueStatus.FIXED), baseIssueList.get(3)); | ||
| when(nonClosedTracking.getMatchedRaws()).thenReturn(matchedRaws); | ||
| Stream<DefaultIssue> unmatchedBaseIssues = Stream.of(baseIssueList.get(0), baseIssueList.get(2)); | ||
| when(nonClosedTracking.getUnmatchedBases()).thenReturn(unmatchedBaseIssues); | ||
| when(tracker.trackNonClosed(any(), any())).thenReturn(nonClosedTracking); | ||
|
|
||
| PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); | ||
| underTest.onRawIssues(component, rawIssues, baseIssues); | ||
|
|
||
| verify(analysisMetadataHolder).isPullRequest(); | ||
| ArgumentCaptor<Input<DefaultIssue>> trackingIssuesCaptor = ArgumentCaptor.captor(); | ||
| verify(tracker).trackNonClosed(eq(rawIssues), trackingIssuesCaptor.capture()); | ||
| assertThat(trackingIssuesCaptor.getValue().getIssues()).containsExactly(baseIssueList.get(1), baseIssueList.get(3)); | ||
| verify(nonClosedTracking).getUnmatchedBases(); | ||
|
|
||
| verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(0)); | ||
| verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(2)); | ||
| verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(3)); | ||
| } | ||
|
|
||
| private static DefaultIssue mockIssue(IssueStatus issueStatus) { | ||
| DefaultIssue issue = mock(DefaultIssue.class); | ||
| when(issue.issueStatus()).thenReturn(issueStatus); | ||
| return issue; | ||
| } | ||
| } |