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
mtughan committed Dec 9, 2024
1 parent 5f48a9f commit 55cbb7a
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 59 deletions.
1 change: 1 addition & 0 deletions ...rg/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public List<Issue> fetchIssues(TaskListener listener) throws InterruptedExceptio

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

Expand Down
106 changes: 51 additions & 55 deletions ...rg/jenkinsci/plugins/sonargerrit/sonar/pull_request_analysis/PullRequestAnalysisTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,15 @@
import static org.assertj.core.api.Assertions.assertThat;

import hudson.model.FreeStyleProject;
import hudson.model.Job;
import hudson.model.queue.QueueTaskFuture;
import hudson.plugins.git.BranchSpec;
import hudson.plugins.git.GitSCM;
import hudson.plugins.git.UserRemoteConfig;
import hudson.plugins.sonar.SonarBuildWrapper;
import hudson.tasks.Maven;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import jenkins.model.Jenkins;
import jenkins.model.ParameterizedJobMixIn;
import me.redaalaoui.gerrit_rest_java_client.thirdparty.com.google.gerrit.extensions.common.ChangeInfo;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.jenkinsci.plugins.sonargerrit.SonarToGerritPublisher;
Expand All @@ -42,13 +38,13 @@ class PullRequestAnalysisTest {

private static final String MAVEN_FREESTYLE_TARGET =
"clean verify sonar:sonar "
+ "-Dsonar.pullrequest.key=${GERRIT_CHANGE_NUMBER}-${GERRIT_PATCHSET_NUMBER} "
+ "-Dsonar.pullrequest.key=${GERRIT_CHANGE_NUMBER} "
+ "-Dsonar.pullrequest.base=${GERRIT_BRANCH} "
+ "-Dsonar.pullrequest.branch=${GERRIT_REFSPEC}";

private static final String MAVEN_PIPELINE_TARGET =
"clean verify sonar:sonar "
+ "-Dsonar.pullrequest.key=${env.GERRIT_CHANGE_NUMBER}-${env.GERRIT_PATCHSET_NUMBER} "
+ "-Dsonar.pullrequest.key=${env.GERRIT_CHANGE_NUMBER} "
+ "-Dsonar.pullrequest.base=${env.GERRIT_BRANCH} "
+ "-Dsonar.pullrequest.branch=${env.GERRIT_REFSPEC}";

Expand Down Expand Up @@ -96,7 +92,7 @@ static void beforeAll(Cluster cluster, @TempDir Path workTree) throws Exception
new Maven(
"clean verify sonar:sonar -Dsonar.branch.name=master",
cluster.jenkinsMavenInstallationName()));
triggerAndAssertSuccess(masterJob);
cluster.jenkinsRule().buildAndAssertSuccess(masterJob);
}

@BeforeEach
Expand All @@ -113,64 +109,79 @@ 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",
"package org.example; "
+ "public class UselessConstructorDeclaration { "
+ "public UselessConstructorDeclaration() {} "
+ "}");
"src/main/java/org/example/Foo.java",
"package org.example; public class Foo { public Foo() {} }");
GerritChange change = git.createGerritChangeForMaster();

triggerAndAssertSuccess(jobFactory.build(change));
int patchSetNumber = 1;
jobFactory.createAndBuild(change, patchSetNumber);

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
.hasSize(1)
.map(approvalInfo -> approvalInfo.value)
.containsExactly(-1);
assertThat(change.listComments())
.map(commentInfo -> commentInfo.message)
.filteredOn(message -> message.contains("S1186"))
.filteredOn(
comment -> comment.patchSet == patchSetNumber && comment.message.contains("S1186"))
.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));
int patchSetNumber = amend ? 2 : 1;
jobFactory.createAndBuild(change, patchSetNumber);

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
.hasSize(1)
.map(approvalInfo -> approvalInfo.value)
.containsExactly(1);
assertThat(change.listComments()).isEmpty();
assertThat(change.listComments())
.filteredOn(comment -> comment.patchSet == patchSetNumber)
.isEmpty();
}

@SuppressWarnings("rawtypes")
private Job createFreestyleJob(GerritChange change) throws IOException {
private void createFreestyleJob(GerritChange change, int patchSetNumber) throws Exception {
FreeStyleProject job = cluster.jenkinsRule().createFreeStyleProject();
job.setJDK(Jenkins.get().getJDK(cluster.jenkinsJdk17InstallationName()));

int patchSetNumber = 1;
job.setScm(createGitSCM(change, patchSetNumber));

job.getBuildWrappersList()
Expand Down Expand Up @@ -204,35 +215,33 @@ private Job createFreestyleJob(GerritChange change) throws IOException {
scoreConfig.setIssuesScore(-1);
sonarToGerrit.setScoreConfig(scoreConfig);
job.getPublishersList().add(sonarToGerrit);
return job;
cluster.jenkinsRule().buildAndAssertSuccess(job);
}

@SuppressWarnings("rawtypes")
private Job createPipelineJob(GerritChange change) throws IOException {
private void createPipelineJob(GerritChange change, int patchSetNumber) throws Exception {
WorkflowJob job = cluster.jenkinsRule().createProject(WorkflowJob.class);
int patchSetNumber = 1;
String script =
"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.jenkinsJdk17InstallationName(), 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 @@ -253,7 +262,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 All @@ -262,7 +271,7 @@ private Job createPipelineJob(GerritChange change) throws IOException {
+ "}\n" // stage('Build')
+ "}";
job.setDefinition(new CpsFlowDefinition(script, true));
return job;
cluster.jenkinsRule().buildAndAssertSuccess(job);
}

private GitSCM createGitSCM(GerritChange change, int patchSetNumber) {
Expand All @@ -288,20 +297,7 @@ private static GitSCM createGitSCM() {
Collections.emptyList());
}

@SuppressWarnings({"rawtypes", "unchecked"})
private static void triggerAndAssertSuccess(Job job) throws Exception {
final QueueTaskFuture future =
new ParameterizedJobMixIn() {
@Override
protected Job asJob() {
return job;
}
}.scheduleBuild2(0);
cluster.jenkinsRule().assertBuildStatusSuccess(future);
}

private interface JobFactory {
@SuppressWarnings("rawtypes")
Job build(GerritChange change) throws Exception;
void createAndBuild(GerritChange change, int patchSetNumber) throws Exception;
}
}
34 changes: 30 additions & 4 deletions src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/gerrit/GerritGit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.UUID;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -15,11 +16,14 @@
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.RefSpec;

/** @author Réda Housni Alaoui */
public class GerritGit {
private static final String CHANGE_ID = "Change-Id: I";

private final GerritServer gerrit;
private final Git git;
Expand Down Expand Up @@ -55,10 +59,27 @@ public Path workTree() {
return git.getRepository().getWorkTree().toPath();
}

public CommitCommand commit(String message) {
String changeId = "I" + DigestUtils.sha1Hex(String.format("%s|%s", UUID.randomUUID(), message));
public CommitCommand commit(String message, boolean amend) throws IOException {
String changeIdLine = null;
if (amend) {
Repository repository = git.getRepository();
String previousMessage =
repository.parseCommit(repository.resolve(Constants.HEAD)).getFullMessage();
changeIdLine =
Arrays.stream(previousMessage.split("\n"))
.filter(msg -> msg.startsWith(CHANGE_ID))
.findFirst()
.orElse(null);
}

if (changeIdLine == null) {
changeIdLine =
CHANGE_ID + DigestUtils.sha1Hex(String.format("%s|%s", UUID.randomUUID(), message));
}

return git.commit()
.setMessage(message + "\n\nChange-Id: " + changeId)
.setAmend(amend)
.setMessage(message + "\n\n" + changeIdLine)
.setAuthor(agent)
.setCommitter(agent);
}
Expand Down Expand Up @@ -99,8 +120,13 @@ 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 55cbb7a

Please sign in to comment.