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 exclude score and review issues by glob pattern #124

Merged
merged 1 commit into from
Jul 4, 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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ It is possible to filter issues by:
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 `/`.
5. Excluded paths' glob pattern - The glob pattern that file paths should match to be excluded by the filter. Leaving a blank value means no file path will match. Tested file paths always start with `/`.

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

Expand Down Expand Up @@ -328,6 +329,7 @@ node {
newIssuesOnly : false,
changedLinesOnly: false,
includedPathsGlobPattern: null,
excludedPathsGlobPattern: null,
],
noIssuesTitleTemplate : 'SonarQube violations have not been found.',
someIssuesTitleTemplate: '<total_count> SonarQube violations have been found.',
Expand All @@ -339,6 +341,7 @@ node {
newIssuesOnly : false,
changedLinesOnly: false,
includedPathsGlobPattern: null,
excludedPathsGlobPattern: null,
],
category : 'Code-Review',
noIssuesScore : 0,
Expand Down Expand Up @@ -407,6 +410,7 @@ node {
newIssuesOnly : false,
changedLinesOnly: false,
includedPathsGlobPattern: null,
excludedPathsGlobPattern: null,
],
noIssuesTitleTemplate : 'SonarQube violations have not been found.',
someIssuesTitleTemplate: '<total_count> SonarQube violations have been found.',
Expand All @@ -418,6 +422,7 @@ node {
newIssuesOnly : false,
changedLinesOnly: false,
includedPathsGlobPattern: null,
excludedPathsGlobPattern: 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
Expand Up @@ -10,14 +10,33 @@
public class ByGlobPatternPredicate implements Predicate<Issue> {

private final PathMatcher pathMatcher;
private final boolean negated;

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

public ByGlobPatternPredicate(String pattern, boolean negated) {
this(FileSystems.getDefault().getPathMatcher("glob:" + pattern), negated);
}

public ByGlobPatternPredicate(PathMatcher pathMatcher, boolean negated) {
this.pathMatcher = pathMatcher;
this.negated = negated;
}

public ByGlobPatternPredicate negate() {
return new ByGlobPatternPredicate(pathMatcher, !negated);
}

@Override
public boolean apply(Issue input) {
String filepath = StringUtils.prependIfMissing(input.getFilepath(), "/");
return pathMatcher.matches(Paths.get(filepath));
boolean matched = pathMatcher.matches(Paths.get(filepath));
if (negated) {
return !matched;
} else {
return matched;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ public Iterable<Issue> filter() {
toBeApplied.add(new ByGlobPatternPredicate(includedPathsGlobPattern));
}

String excludedPathsGlobPattern = filterConfig.getExcludedPathsGlobPattern();
if (excludedPathsGlobPattern != null) {
toBeApplied.add(new ByGlobPatternPredicate(excludedPathsGlobPattern).negate());
}

return Iterables.filter(issues, Predicates.and(toBeApplied));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class IssueFilterConfig extends AbstractDescribableImpl<IssueFilterConfig
private boolean newIssuesOnly;
private boolean changedLinesOnly;
private String includedPathsGlobPattern;
private String excludedPathsGlobPattern;

public IssueFilterConfig(String severity, boolean newIssuesOnly, boolean changedLinesOnly) {
setSeverity(severity);
Expand Down Expand Up @@ -71,6 +72,16 @@ public void setIncludedPathsGlobPattern(String includedPathsGlobPattern) {
this.includedPathsGlobPattern = includedPathsGlobPattern;
}

@Nullable
public String getExcludedPathsGlobPattern() {
return excludedPathsGlobPattern;
}

@DataBoundSetter
public void setExcludedPathsGlobPattern(String excludedPathsGlobPattern) {
this.excludedPathsGlobPattern = excludedPathsGlobPattern;
}

@Override
public DescriptorImpl getDescriptor() {
return new DescriptorImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,11 @@
description="${%jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern.description}">
<f:textbox/>
</f:entry>

<f:entry
title="${%jenkins.plugin.settings.gerrit.filter.excluded-paths-glob-pattern}"
field="excludedPathsGlobPattern"
description="${%jenkins.plugin.settings.gerrit.filter.excluded-paths-glob-pattern.description}">
<f:textbox/>
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ 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 ''/''.
jenkins.plugin.settings.gerrit.filter.excluded-paths-glob-pattern=Excluded paths'' glob pattern
jenkins.plugin.settings.gerrit.filter.excluded-paths-glob-pattern.description=The glob pattern that file paths should match to be excluded by the filter. Leaving a blank value means no 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 @@ -319,9 +319,13 @@ void test5(JenkinsRule jenkinsRule) throws Exception {
"reviewConfig.issueFilterConfig.changedLinesOnly",
"reviewConfig.issueFilterConfig.newIssuesOnly",
"reviewConfig.issueFilterConfig.severity",
"reviewConfig.issueFilterConfig.includedPathsGlobPattern",
"reviewConfig.issueFilterConfig.excludedPathsGlobPattern",
"scoreConfig.issueFilterConfig.changedLinesOnly",
"scoreConfig.issueFilterConfig.newIssuesOnly",
"scoreConfig.issueFilterConfig.severity",
"scoreConfig.issueFilterConfig.includedPathsGlobPattern",
"scoreConfig.issueFilterConfig.excludedPathsGlobPattern",
"scoreConfig.noIssuesScore",
"scoreConfig.issuesScore",
"scoreConfig.category",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ReviewTest {
+ "-Dsonar.pullrequest.base=${env.GERRIT_BRANCH} "
+ "-Dsonar.pullrequest.branch=${env.GERRIT_REFSPEC}";
private static final String FILEPATH =
"src/main/java/org/example/UselessConstructorDeclaration.java";
"child1/src/main/java/org/example/UselessConstructorDeclaration.java";

private static Cluster cluster;
private static GerritGit git;
Expand All @@ -54,15 +54,35 @@ static void beforeAll(Cluster cluster, @TempDir Path workTree) throws Exception
git = GerritGit.createAndCloneRepository(cluster.gerrit(), workTree);

git.addAndCommitFile(
"pom.xml",
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<project>\n"
+ " <modelVersion>4.0.0</modelVersion>\n"
+ "\n"
+ " <groupId>org.example</groupId>\n"
+ " <artifactId>example</artifactId>\n"
+ " <version>1.0-SNAPSHOT</version>\n"
+ "</project>");
"pom.xml",
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<project>\n"
+ " <modelVersion>4.0.0</modelVersion>\n"
+ "\n"
+ " <groupId>org.example</groupId>\n"
+ " <artifactId>example</artifactId>\n"
+ " <version>1.0-SNAPSHOT</version>\n"
+ " <packaging>pom</packaging>"
+ "\n"
+ "<modules>\n"
+ "<module>child1</module>"
+ "</modules>"
+ "</project>")
.addAndCommitFile(
"child1/pom.xml",
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<project>\n"
+ " <modelVersion>4.0.0</modelVersion>\n"
+ "\n"
+ "<parent>\n"
+ " <groupId>org.example</groupId>\n"
+ " <artifactId>example</artifactId>\n"
+ " <version>1.0-SNAPSHOT</version>\n"
+ "</parent>\n"
+ "\n"
+ " <artifactId>child1</artifactId>\n"
+ "\n"
+ "</project>");

git.push();

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

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

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

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -143,7 +163,7 @@ void test3() throws Exception {
@DisplayName("Issue ignored because of path glob pattern")
void test4() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/[!src]/**"));
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/child2/**", null));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -158,7 +178,7 @@ void test4() throws Exception {
@DisplayName("Issue considered despite of path glob pattern")
void test5() throws Exception {
GerritChange change = createChangeViolatingS1186();
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/src/**"));
triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, null, "/child2/**"));

ChangeInfo changeDetail = change.getDetail();
assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all)
Expand All @@ -184,14 +204,22 @@ private GerritChange createChangeViolatingS1186()

@SuppressWarnings("rawtypes")
private Job createPipelineJob(
GerritChange change, ReviewCommentType commentType, String includedPathsGlobPattern)
GerritChange change,
ReviewCommentType commentType,
String includedPathsGlobPattern,
String excludedPathsGlobPattern)
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 quotedExcludedPathsGlobPattern =
Optional.ofNullable(excludedPathsGlobPattern)
.map(s -> StringUtils.wrap(s, "'"))
.orElse(null);
String script =
"node {\n"
+ "stage('Build') {\n"
Expand Down Expand Up @@ -227,15 +255,17 @@ private Job createPipelineJob(
+ "severity: 'MINOR',\n"
+ "newIssuesOnly: false,\n"
+ "changedLinesOnly: true,\n"
+ String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern)
+ String.format("includedPathsGlobPattern: %s, \n", quotedIncludedPathsGlobPattern)
+ String.format("excludedPathsGlobPattern: %s\n", quotedExcludedPathsGlobPattern)
+ "]\n" // issueFilterConfig
+ "],\n" // reviewConfig
+ "scoreConfig: [\n"
+ "issueFilterConfig: [\n"
+ "severity: 'MINOR',\n"
+ "newIssuesOnly: false,\n"
+ "changedLinesOnly: true,\n"
+ String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern)
+ String.format("includedPathsGlobPattern: %s,\n", quotedIncludedPathsGlobPattern)
+ String.format("excludedPathsGlobPattern: %s\n", quotedExcludedPathsGlobPattern)
+ "],\n" // issueFilterConfig
+ String.format("category: '%s',\n", GerritServer.CODE_QUALITY_LABEL)
+ "noIssuesScore: 1,\n"
Expand Down