Skip to content

Commit

Permalink
Only fetch unresolved issues
Browse files Browse the repository at this point in the history
When fetching issues from SonarQube, we should only fetch unresolved
issues. This will avoid showing issues which were discovered in previous
runs with the same "PR" ID but have since been resolved and are no
longer issues.
  • Loading branch information
mtughan committed Sep 4, 2024
1 parent be06708 commit ff51ef2
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 23 deletions.
8 changes: 7 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,13 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<artifactId>mockito-inline</artifactId>
<version>4.2.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>4.2.0</version>
<scope>test</scope>
</dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static java.util.Objects.requireNonNull;

import com.google.common.annotations.VisibleForTesting;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.plugins.sonar.SonarInstallation;
Expand Down Expand Up @@ -42,7 +43,8 @@ class PullRequestAnalysisTask {
private final String taskId;
private final String componentKey;

private PullRequestAnalysisTask(
@VisibleForTesting
/* package */ PullRequestAnalysisTask(
WsClient sonarClient, String serverUrl, String taskId, String componentKey) {
this.sonarClient = requireNonNull(sonarClient);
this.serverUrl = requireNonNull(StringUtils.trimToNull(serverUrl));
Expand Down Expand Up @@ -166,6 +168,7 @@ public List<Issue> fetchIssues(TaskListener listener) throws InterruptedExceptio

SearchRequest issueSearchRequest =
new SearchRequest()
.setResolved("false")
.setComponentKeys(Collections.singletonList(componentKey))
.setPullRequest(pullRequestKey);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package org.jenkinsci.plugins.sonargerrit.sonar.pull_request_analysis;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

import hudson.model.TaskListener;
import hudson.util.NullStream;
import hudson.util.StreamTaskListener;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import me.redaalaoui.org.sonarqube.ws.Ce;
import me.redaalaoui.org.sonarqube.ws.Issues;
import me.redaalaoui.org.sonarqube.ws.ProjectPullRequests;
import me.redaalaoui.org.sonarqube.ws.client.WsClient;
import me.redaalaoui.org.sonarqube.ws.client.ce.CeService;
import me.redaalaoui.org.sonarqube.ws.client.ce.TaskRequest;
import me.redaalaoui.org.sonarqube.ws.client.issues.IssuesService;
import me.redaalaoui.org.sonarqube.ws.client.issues.SearchRequest;
import me.redaalaoui.org.sonarqube.ws.client.projectpullrequests.ListRequest;
import me.redaalaoui.org.sonarqube.ws.client.projectpullrequests.ProjectPullRequestsService;
import org.jenkinsci.plugins.sonargerrit.sonar.Issue;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class PullRequestAnalysisTaskTest {
static final String SERVER_URL = "http://sonar.example.com:9000/";
static final String TASK_ID = "task";
static final String COMPONENT_ID = "component";
static final String PR_KEY = "PR-9999";

@Mock WsClient sonarClient;
@Mock CeService ceService;
@Mock Ce.TaskResponse ceTaskResponse;
@Mock Ce.Task ceTask;
@Mock ProjectPullRequestsService pullRequestsService;
@Mock ProjectPullRequests.ListWsResponse prListResponse;
@Mock ProjectPullRequests.PullRequest pullRequest;
@Mock IssuesService issuesService;
@Mock Issues.SearchWsResponse issuesSearchResponse;
@Mock Issues.Component component;
@Mock Issues.Issue issue;
PullRequestAnalysisTask prAnalysisTask;
TaskListener taskListener;

@BeforeEach
void setUp() {
taskListener = new StreamTaskListener(new NullStream());
prAnalysisTask = new PullRequestAnalysisTask(sonarClient, SERVER_URL, TASK_ID, COMPONENT_ID);
}

@Test
void searchOnlyUnresolvedIssues() throws InterruptedException {
final AtomicReference<TaskRequest> taskRequest = new AtomicReference<>();
final AtomicReference<ListRequest> listRequest = new AtomicReference<>();
final AtomicReference<SearchRequest> searchRequest = new AtomicReference<>();

when(sonarClient.ce()).thenReturn(ceService);
when(ceService.task(any(TaskRequest.class)))
.thenAnswer(
invocation -> {
taskRequest.set(invocation.getArgument(0));
return ceTaskResponse;
});
when(ceTaskResponse.getTask()).thenReturn(ceTask);
when(ceTask.getStatus()).thenReturn(Ce.TaskStatus.SUCCESS);
when(ceTask.getPullRequest()).thenReturn(PR_KEY);

when(sonarClient.projectPullRequests()).thenReturn(pullRequestsService);
when(pullRequestsService.list(any(ListRequest.class)))
.thenAnswer(
invocation -> {
listRequest.set(invocation.getArgument(0));
return prListResponse;
});
when(prListResponse.getPullRequestsList()).thenReturn(Collections.singletonList(pullRequest));
when(pullRequest.getKey()).thenReturn(PR_KEY);

when(sonarClient.issues()).thenReturn(issuesService);
when(issuesService.search(any(SearchRequest.class)))
.thenAnswer(
invocation -> {
searchRequest.set(invocation.getArgument(0));
return issuesSearchResponse;
});
when(issuesSearchResponse.getComponentsList()).thenReturn(Collections.singletonList(component));
when(component.getKey()).thenReturn(COMPONENT_ID);
when(issuesSearchResponse.getIssuesList()).thenReturn(Collections.singletonList(issue));
when(issue.getComponent()).thenReturn(COMPONENT_ID);

List<Issue> issues = prAnalysisTask.fetchIssues(taskListener);

assertEquals(1, issues.size());

assertNotNull(taskRequest.get());
assertEquals(TASK_ID, taskRequest.get().getId());

assertNotNull(listRequest.get());
assertEquals(COMPONENT_ID, listRequest.get().getProject());

assertNotNull(searchRequest.get());
assertEquals("false", searchRequest.get().getResolved());
assertIterableEquals(
Collections.singletonList(COMPONENT_ID), searchRequest.get().getComponentKeys());
assertEquals(PR_KEY, searchRequest.get().getPullRequest());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,27 +104,41 @@ void test1() throws Exception {
@Test
@DisplayName("Good quality freestyle build")
void test2() throws Exception {
testWithGoodQualityCode(this::createFreestyleJob);
testWithGoodQualityCode(this::createFreestyleJob, false);
}

@Test
@DisplayName("Bad quality pipeline build")
@DisplayName("Bad then good quality freestyle build")
void test3() throws Exception {
testWithBadQualityCode(this::createFreestyleJob);
testWithGoodQualityCode(this::createFreestyleJob, true);
}

@Test
@DisplayName("Bad quality pipeline build")
void test4() throws Exception {
testWithBadQualityCode(this::createPipelineJob);
}

@Test
@DisplayName("Good quality pipeline build")
void test4() throws Exception {
testWithGoodQualityCode(this::createPipelineJob);
void test5() throws Exception {
testWithGoodQualityCode(this::createPipelineJob, false);
}

@Test
@DisplayName("Bad then good quality pipeline build")
void test6() throws Exception {
testWithBadQualityCode(this::createPipelineJob);
testWithGoodQualityCode(this::createFreestyleJob, true);
}

private void testWithBadQualityCode(JobFactory jobFactory) throws Exception {
git.addAndCommitFile(
"src/main/java/org/example/UselessConstructorDeclaration.java",
"src/main/java/org/example/Foo.java",
"package org.example; "
+ "public class UselessConstructorDeclaration { "
+ "public UselessConstructorDeclaration() {} "
+ "public class Foo { "
+ "public Foo() {} "
+ "}");
GerritChange change = git.createGerritChangeForMaster();

Expand All @@ -141,9 +155,9 @@ private void testWithBadQualityCode(JobFactory jobFactory) throws Exception {
.hasSize(1);
}

private void testWithGoodQualityCode(JobFactory jobFactory) throws Exception {
private void testWithGoodQualityCode(JobFactory jobFactory, boolean amend) throws Exception {
git.addAndCommitFile(
"src/main/java/org/example/Foo.java", "package org.example; public interface Foo {}");
"src/main/java/org/example/Foo.java", "package org.example; public interface Foo {}", amend);
GerritChange change = git.createGerritChangeForMaster();

triggerAndAssertSuccess(jobFactory.build(change));
Expand Down Expand Up @@ -206,24 +220,24 @@ private Job createPipelineJob(GerritChange change) throws IOException {
"node {\n"
+ "stage('Build') {\n"
+ "try {\n"
+ String.format("env.GERRIT_NAME = '%s'\n", cluster.jenkinsGerritTriggerServerName())
+ String.format("env.GERRIT_CHANGE_NUMBER = '%s'\n", change.changeNumericId())
+ String.format("env.GERRIT_PATCHSET_NUMBER = '%s'\n", patchSetNumber)
+ String.format("env.GERRIT_BRANCH = '%s'\n", "master")
+ String.format("env.GERRIT_REFSPEC = '%s'\n", change.refName(patchSetNumber))
+ String.format("env.GERRIT_NAME = '%s'%n", cluster.jenkinsGerritTriggerServerName())
+ String.format("env.GERRIT_CHANGE_NUMBER = '%s'%n", change.changeNumericId())
+ String.format("env.GERRIT_PATCHSET_NUMBER = '%s'%n", patchSetNumber)
+ String.format("env.GERRIT_BRANCH = '%s'%n", "master")
+ String.format("env.GERRIT_REFSPEC = '%s'%n", change.refName(patchSetNumber))
+ "checkout scm: ([\n"
+ "$class: 'GitSCM',\n"
+ String.format(
"userRemoteConfigs: [[url: '%s', refspec: '%s', credentialsId: '%s']],\n",
"userRemoteConfigs: [[url: '%s', refspec: '%s', credentialsId: '%s']],%n",
git.httpUrl(), change.refName(patchSetNumber), cluster.jenkinsGerritCredentialsId())
+ "branches: [[name: 'FETCH_HEAD']]\n"
+ "])\n"
+ String.format(
"withSonarQubeEnv('%s') {\n", cluster.jenkinsSonarqubeInstallationName())
"withSonarQubeEnv('%s') {%n", cluster.jenkinsSonarqubeInstallationName())
+ String.format(
"withMaven(jdk: '%s', maven: '%s') {\n",
"withMaven(jdk: '%s', maven: '%s') {%n",
cluster.jenkinsJdk8InstallationName(), cluster.jenkinsMavenInstallationName())
+ String.format("sh \"mvn %s\"\n", MAVEN_PIPELINE_TARGET)
+ String.format("sh \"mvn %s\"%n", MAVEN_PIPELINE_TARGET)
+ "}\n" // withMaven
+ "}\n" // withSonarQubeEnv
+ "} finally {\n"
Expand All @@ -244,7 +258,7 @@ private Job createPipelineJob(GerritChange change) throws IOException {
+ "newIssuesOnly: false,"
+ "changedLinesOnly: true"
+ "],\n" // issueFilterConfig
+ String.format("category: '%s',\n", GerritServer.CODE_QUALITY_LABEL)
+ String.format("category: '%s',%n", GerritServer.CODE_QUALITY_LABEL)
+ "noIssuesScore: 1,\n"
+ "issuesScore: -1,\n"
+ "]\n" // scoreConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ public Path workTree() {
return git.getRepository().getWorkTree().toPath();
}

public CommitCommand commit(String message) {
public CommitCommand commit(String message, boolean amend) {
String changeId = "I" + DigestUtils.sha1Hex(String.format("%s|%s", UUID.randomUUID(), message));
return git.commit()
.setAmend(amend)
.setMessage(message + "\n\nChange-Id: " + changeId)
.setAuthor(agent)
.setCommitter(agent);
Expand Down Expand Up @@ -98,9 +99,14 @@ public GerritGit resetToOriginMaster() throws GitAPIException {
}

public GerritGit addAndCommitFile(String relativePath, String content)
throws IOException, GitAPIException {
return addAndCommitFile(relativePath, content, false);
}

public GerritGit addAndCommitFile(String relativePath, String content, boolean amend)
throws IOException, GitAPIException {
addFile(relativePath, content);
commit("Add " + relativePath).call();
commit("Add " + relativePath, amend).call();
return this;
}

Expand Down

0 comments on commit ff51ef2

Please sign in to comment.