Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to filter score and review issues by path #123

Merged
merged 1 commit into from
Jul 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ It is possible to filter issues by:
is not created by processing commit and this issue is not supposed to be included to review.
3. Changed lines only - when only several lines are changed in a commit user may not want other lines to be commented by
Gerrit. With "Add comments to changed lines only" unchanged in the commit lines will not be commented in Gerrit.
4. Included paths' glob pattern - The glob pattern that file paths should match to be included by the filter. Leaving a blank value or setting value `**/*` means any file path will match. Tested file paths always start with `/`.

![Filter settings](doc/filter-settings.png)

Expand Down Expand Up @@ -325,7 +326,8 @@ node {
issueFilterConfig : [
severity : 'INFO',
newIssuesOnly : false,
changedLinesOnly: false
changedLinesOnly: false,
includedPathsGlobPattern: null,
],
noIssuesTitleTemplate : 'SonarQube violations have not been found.',
someIssuesTitleTemplate: '<total_count> SonarQube violations have been found.',
Expand All @@ -335,7 +337,8 @@ node {
issueFilterConfig: [
severity : 'INFO',
newIssuesOnly : false,
changedLinesOnly: false
changedLinesOnly: false,
includedPathsGlobPattern: null,
],
category : 'Code-Review',
noIssuesScore : 0,
Expand Down Expand Up @@ -402,7 +405,8 @@ node {
issueFilterConfig : [
severity : 'INFO',
newIssuesOnly : false,
changedLinesOnly: false
changedLinesOnly: false,
includedPathsGlobPattern: null,
],
noIssuesTitleTemplate : 'SonarQube violations have not been found.',
someIssuesTitleTemplate: '<total_count> SonarQube violations have been found.',
Expand All @@ -412,7 +416,8 @@ node {
issueFilterConfig: [
severity : 'INFO',
newIssuesOnly : false,
changedLinesOnly: false
changedLinesOnly: false,
includedPathsGlobPattern: null,
],
category : 'Code-Review',
noIssuesScore : 0,
Expand Down
Binary file modified doc/filter-settings.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.jenkinsci.plugins.sonargerrit.sonar;

import com.google.common.base.Predicate;
import java.nio.file.FileSystems;
import java.nio.file.PathMatcher;
import java.nio.file.Paths;
import org.apache.commons.lang3.StringUtils;

/** @author Réda Housni Alaoui */
public class ByGlobPatternPredicate implements Predicate<Issue> {

private final PathMatcher pathMatcher;

public ByGlobPatternPredicate(String pattern) {
this.pathMatcher = FileSystems.getDefault().getPathMatcher("glob:" + pattern);
}

@Override
public boolean apply(Issue input) {
String filepath = StringUtils.prependIfMissing(input.getFilepath(), "/");
return pathMatcher.matches(Paths.get(filepath));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ public Iterable<Issue> filter() {
toBeApplied.add(ByMinSeverityPredicate.apply(severity));
}

String includedPathsGlobPattern = filterConfig.getIncludedPathsGlobPattern();
if (includedPathsGlobPattern != null) {
toBeApplied.add(new ByGlobPatternPredicate(includedPathsGlobPattern));
}

return Iterables.filter(issues, Predicates.and(toBeApplied));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.jenkinsci.plugins.sonargerrit.util.Localization.getLocalized;

import com.google.common.base.MoreObjects;
import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.Extension;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
Expand All @@ -18,6 +19,7 @@ public class IssueFilterConfig extends AbstractDescribableImpl<IssueFilterConfig
private String severity; // todo make enum
private boolean newIssuesOnly;
private boolean changedLinesOnly;
private String includedPathsGlobPattern;

public IssueFilterConfig(String severity, boolean newIssuesOnly, boolean changedLinesOnly) {
setSeverity(severity);
Expand All @@ -35,30 +37,40 @@ public String getSeverity() {
return severity;
}

public boolean isChangedLinesOnly() {
return changedLinesOnly;
}

public boolean isNewIssuesOnly() {
return newIssuesOnly;
}

@DataBoundSetter
public void setSeverity(String severity) {
severity = DataHelper.checkEnumValueCorrect(Severity.class, severity);
this.severity = MoreObjects.firstNonNull(severity, DescriptorImpl.SEVERITY);
}

public boolean isNewIssuesOnly() {
return newIssuesOnly;
}

@DataBoundSetter
public void setNewIssuesOnly(boolean newIssuesOnly) {
this.newIssuesOnly = newIssuesOnly;
}

public boolean isChangedLinesOnly() {
return changedLinesOnly;
}

@DataBoundSetter
public void setChangedLinesOnly(boolean changedLinesOnly) {
this.changedLinesOnly = changedLinesOnly;
}

@Nullable
public String getIncludedPathsGlobPattern() {
return includedPathsGlobPattern;
}

@DataBoundSetter
public void setIncludedPathsGlobPattern(String includedPathsGlobPattern) {
this.includedPathsGlobPattern = includedPathsGlobPattern;
}

@Override
public DescriptorImpl getDescriptor() {
return new DescriptorImpl();
Expand All @@ -82,9 +94,11 @@ public static class DescriptorImpl extends Descriptor<IssueFilterConfig> {
*/
@SuppressWarnings(value = "unused")
public FormValidation doCheckSeverity(@QueryParameter String value) {
if (value == null || Severity.valueOf(value) == null) {
if (value == null) {
return FormValidation.error(
getLocalized("jenkins.plugin.error.review.filter.severity.unknown"));
} else {
Severity.valueOf(value);
}
return FormValidation.ok();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,11 @@
description="${%jenkins.plugin.settings.gerrit.filter.lines.changed.description}">
<f:checkbox selected="${changedLinesOnly}" default="descriptor.CHANGED_LINES_ONLY"/>
</f:entry>
</j:jelly>

<f:entry
title="${%jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern}"
field="includedPathsGlobPattern"
description="${%jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern.description}">
<f:textbox/>
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ jenkins.plugin.settings.gerrit.filter.new=Report new issues only?
jenkins.plugin.settings.gerrit.filter.new.description=Reports new as well as existing issues when unchecked. Note that pull request analysis only reports new issues and is therefore unaffected by this parameter.
jenkins.plugin.settings.gerrit.filter.lines.changed=Affect changed lines only
jenkins.plugin.settings.gerrit.filter.lines.changed.description=Reports issues for changed as well as not changed lines in files affected by patchset when unchecked
jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern=Included paths'' glob pattern
jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern.description=The glob pattern that file paths should match to be included by the filter. Leaving a blank value or setting value ''**/*'' means any file path will match. Tested file paths always start with ''/''.

INFO=Info
MINOR=Minor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collections;
import java.util.Optional;
import jenkins.model.Jenkins;
import jenkins.model.ParameterizedJobMixIn;
import me.redaalaoui.gerrit_rest_java_client.thirdparty.com.google.gerrit.extensions.common.ChangeInfo;
import me.redaalaoui.gerrit_rest_java_client.thirdparty.com.google.gerrit.extensions.restapi.RestApiException;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.jenkinsci.plugins.sonargerrit.test_infrastructure.cluster.Cluster;
import org.jenkinsci.plugins.sonargerrit.test_infrastructure.cluster.EnableCluster;
Expand All @@ -38,6 +40,8 @@ class ReviewTest {
+ "-Dsonar.pullrequest.key=${env.GERRIT_CHANGE_NUMBER}-${env.GERRIT_PATCHSET_NUMBER} "
+ "-Dsonar.pullrequest.base=${env.GERRIT_BRANCH} "
+ "-Dsonar.pullrequest.branch=${env.GERRIT_REFSPEC}";
private static final String FILEPATH =
"src/main/java/org/example/UselessConstructorDeclaration.java";

private static Cluster cluster;
private static GerritGit git;
Expand Down Expand Up @@ -86,7 +90,7 @@ void beforeEach() throws GitAPIException {
@DisplayName("STANDARD comment type")
void test1() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD));
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD, null));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -103,7 +107,7 @@ void test1() throws Exception {
@DisplayName("ROBOT comment type")
void test2() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT));
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, null));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -126,7 +130,7 @@ void test2() throws Exception {
@DisplayName("Review tag is autogenerated:sonar")
void test3() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD));
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD, null));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -135,10 +139,42 @@ void test3() throws Exception {
.containsExactly("autogenerated:sonar");
}

@Test
@DisplayName("Issue ignored because of path glob pattern")
void test4() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/[!src]/**"));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
.hasSize(1)
.map(approvalInfo -> approvalInfo.value)
.containsExactly(1);

assertThat(change.listRobotComments()).isEmpty();
}

@Test
@DisplayName("Issue considered despite of path glob pattern")
void test5() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/src/**"));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
.hasSize(1)
.map(approvalInfo -> approvalInfo.value)
.containsExactly(-1);

assertThat(change.listRobotComments())
.filteredOn(comment -> comment.message.contains("S1186"))
.hasSize(1);
}

private GerritChange createChangeViolatingS1186()
throws GitAPIException, IOException, RestApiException {
git.addAndCommitFile(
"src/main/java/org/example/UselessConstructorDeclaration.java",
FILEPATH,
"package org.example; "
+ "public class UselessConstructorDeclaration { "
+ "public UselessConstructorDeclaration() {} "
Expand All @@ -147,10 +183,15 @@ private GerritChange createChangeViolatingS1186()
}

@SuppressWarnings("rawtypes")
private Job createPipelineJob(GerritChange change, ReviewCommentType commentType)
private Job createPipelineJob(
GerritChange change, ReviewCommentType commentType, String includedPathsGlobPattern)
throws IOException {
WorkflowJob job = cluster.jenkinsRule().createProject(WorkflowJob.class);
int patchSetNumber = 1;
String quotedIncludedPathsGlobPattern =
Optional.ofNullable(includedPathsGlobPattern)
.map(s -> StringUtils.wrap(s, "'"))
.orElse(null);
String script =
"node {\n"
+ "stage('Build') {\n"
Expand Down Expand Up @@ -185,14 +226,16 @@ private Job createPipelineJob(GerritChange change, ReviewCommentType commentType
+ "issueFilterConfig: [\n"
+ "severity: 'MINOR',\n"
+ "newIssuesOnly: false,\n"
+ "changedLinesOnly: true\n"
+ "changedLinesOnly: true,\n"
+ String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern)
+ "]\n" // issueFilterConfig
+ "],\n" // reviewConfig
+ "scoreConfig: [\n"
+ "issueFilterConfig: [\n"
+ "severity: 'MINOR',"
+ "newIssuesOnly: false,"
+ "changedLinesOnly: true"
+ "severity: 'MINOR',\n"
+ "newIssuesOnly: false,\n"
+ "changedLinesOnly: true,\n"
+ String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern)
+ "],\n" // issueFilterConfig
+ String.format("category: '%s',\n", GerritServer.CODE_QUALITY_LABEL)
+ "noIssuesScore: 1,\n"
Expand Down
Loading