From ff51ef2fbba49b1598ddcc83b4827fd75572bbe9 Mon Sep 17 00:00:00 2001 From: Michael Tughan Date: Wed, 24 Jul 2024 12:13:00 -0400 Subject: [PATCH] Only fetch unresolved issues 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. --- pom.xml | 8 +- .../PullRequestAnalysisTask.java | 5 +- .../PullRequestAnalysisTaskTest.java | 114 ++++++++++++++++++ .../PullRequestAnalysisTest.java | 52 +++++--- .../test_infrastructure/gerrit/GerritGit.java | 10 +- 5 files changed, 166 insertions(+), 23 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTaskTest.java diff --git a/pom.xml b/pom.xml index 419dc602..61e72472 100644 --- a/pom.xml +++ b/pom.xml @@ -182,7 +182,13 @@ org.mockito - mockito-core + mockito-inline + 4.2.0 + test + + + org.mockito + mockito-junit-jupiter 4.2.0 test diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTask.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTask.java index b8f1d3b5..c7658ac9 100644 --- a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTask.java +++ b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTask.java @@ -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; @@ -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)); @@ -166,6 +168,7 @@ public List fetchIssues(TaskListener listener) throws InterruptedExceptio SearchRequest issueSearchRequest = new SearchRequest() + .setResolved("false") .setComponentKeys(Collections.singletonList(componentKey)) .setPullRequest(pullRequestKey); diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTaskTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTaskTest.java new file mode 100644 index 00000000..b1eb281a --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTaskTest.java @@ -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 = new AtomicReference<>(); + final AtomicReference listRequest = new AtomicReference<>(); + final AtomicReference 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 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()); + } +} diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTest.java index 71988783..877044ed 100644 --- a/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTest.java +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTest.java @@ -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(); @@ -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)); @@ -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" @@ -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 diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/gerrit/GerritGit.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/gerrit/GerritGit.java index 309d30bc..e9f23808 100644 --- a/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/gerrit/GerritGit.java +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/gerrit/GerritGit.java @@ -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); @@ -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; }