Skip to content

Commit

Permalink
Limit overview comment length
Browse files Browse the repository at this point in the history 
Fixed AmadeusITGroup#113
  • Loading branch information
Oleg Marchuk committed Mar 27, 2017
1 parent 8596c66 commit bba85d4
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 20 deletions.
6 changes: 6 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@
<version>4.12</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<version>1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
40 changes: 33 additions & 7 deletions src/main/java/org/sonar/plugins/stash/issue/MarkdownPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,26 @@
import java.util.Map;

import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.issue.Issue;
import org.sonar.api.rule.Severity;
import org.sonar.plugins.stash.IssuePathResolver;
import org.sonar.plugins.stash.PullRequestRef;
import org.sonar.plugins.stash.StashProjectBuilder;
import org.sonar.plugins.stash.coverage.CoverageRule;

import com.google.common.collect.Lists;

public final class MarkdownPrinter {

static final String NEW_LINE = "\n";
static final String CODING_RULES_RULE_KEY = "coding_rules#rule_key=";
static final List<String> orderedSeverities = Lists.reverse(Severity.ALL);
private static final Logger LOGGER = LoggerFactory.getLogger(StashProjectBuilder.class);

private static final int SOFT_SUMMARY_COMMENT_MAX_LENGTH = 30000;
private static final int HARD_SUMMARY_COMMENT_MAX_LENGTH = Short.MAX_VALUE;
private static final String NEW_LINE = "\n";
private static final String CODING_RULES_RULE_KEY = "coding_rules#rule_key=";
private static final List<String> orderedSeverities = Lists.reverse(Severity.ALL);

private MarkdownPrinter(){
// DO NOTHING
Expand Down Expand Up @@ -51,7 +58,7 @@ public static String printIssueNumberBySeverityMarkdown(List<Issue> report, Stri
return sb.toString();
}

public static String printIssueListBySeverityMarkdown(List<Issue> report, String sonarQubeURL, String severity) {
public static String printIssueListBySeverityMarkdown(int maxLength, List<Issue> report, String sonarQubeURL, String severity) {
StringBuilder sb = new StringBuilder();

Map<String, Issue> rules = getUniqueRulesBySeverity(report, severity);
Expand All @@ -60,6 +67,11 @@ public static String printIssueListBySeverityMarkdown(List<Issue> report, String
for (Map.Entry<String, Issue> rule : rules.entrySet()) {
Issue issue = rule.getValue();
sb.append("| ").append(printIssueMarkdown(issue, sonarQubeURL)).append(" |").append(NEW_LINE);
if (sb.length() > maxLength) {
sb.append("| ").append(MarkdownPrinter.printSeverityMarkdown(issue.severity()));
sb.append("The rest issues are skipped |").append(NEW_LINE);
break;
}
}

return sb.toString();
Expand Down Expand Up @@ -119,20 +131,30 @@ public static String printReportMarkdown(PullRequestRef pr, String stashURL, Str
sb.append("| Issues list |").append(NEW_LINE);
sb.append("|------------|").append(NEW_LINE);
for (String severity: orderedSeverities) {
sb.append(printIssueListBySeverityMarkdown(generalIssues, sonarQubeURL, severity));
int maxLength = SOFT_SUMMARY_COMMENT_MAX_LENGTH / 2 - sb.length();
sb.append(printIssueListBySeverityMarkdown(maxLength, generalIssues, sonarQubeURL, severity));
if (sb.length() > SOFT_SUMMARY_COMMENT_MAX_LENGTH / 2) {
break;
}
}
sb.append(NEW_LINE).append(NEW_LINE);
}

// Code coverage
if (!coverageIssues.isEmpty()) {
sb.append(printCoverageReportMarkdown(stashProject, stashRepo, pullRequestId, coverageIssues, stashURL, projectCoverage, previousProjectCoverage, issuePathResolver));
int maxLength = SOFT_SUMMARY_COMMENT_MAX_LENGTH - sb.length();
sb.append(printCoverageReportMarkdown(maxLength, stashProject, stashRepo, pullRequestId, coverageIssues, stashURL, projectCoverage, previousProjectCoverage, issuePathResolver));
}

if (sb.length() > HARD_SUMMARY_COMMENT_MAX_LENGTH) {
LOGGER.debug("Overview comment is too big, trimming");
sb.setLength(HARD_SUMMARY_COMMENT_MAX_LENGTH);
}

return sb.toString();
}

public static String printCoverageReportMarkdown(String stashProject, String stashRepo, int pullRequestId, List<Issue> coverageReport, String stashURL,
public static String printCoverageReportMarkdown(int maxLength, String stashProject, String stashRepo, int pullRequestId, List<Issue> coverageReport, String stashURL,
Double projectCoverage, Double previousProjectCoverage, IssuePathResolver issuePathResolver) {
StringBuilder sb = new StringBuilder("| Line Coverage: ");

Expand All @@ -149,6 +171,10 @@ public static String printCoverageReportMarkdown(String stashProject, String sta

for (Issue issue : coverageReport) {
sb.append("| ").append(MarkdownPrinter.printCoverageIssueMarkdown(stashProject, stashRepo, String.valueOf(pullRequestId), stashURL, issue, issuePathResolver)).append(" |").append(NEW_LINE);
if (sb.length() > maxLength) {
sb.append("| The rest issues are skipped |");
break;
}
}

return sb.toString();
Expand Down
50 changes: 37 additions & 13 deletions src/test/java/org/sonar/plugins/stash/issue/MarkdownPrinterTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.sonar.plugins.stash.issue;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -18,16 +21,15 @@

public class MarkdownPrinterTest {

Issue issue;
Issue coverageIssue;

List<Issue> report = new ArrayList<>();
DummyIssuePathResolver issuePathResolver = new DummyIssuePathResolver();

private static final String SONAR_URL = "sonarqube/URL";
private static final String STASH_URL = "stash/URL";

private Issue coverageIssue;

PullRequestRef pr = PullRequestRef.builder()
private List<Issue> report = new ArrayList<>();
private DummyIssuePathResolver issuePathResolver = new DummyIssuePathResolver();

private PullRequestRef pr = PullRequestRef.builder()
.setProject("stashProject")
.setRepository("stashRepo")
.setPullRequestId(1)
Expand All @@ -44,13 +46,11 @@ public void setUp(){
report.add(issueCritical);
report.add(issueMajor);

issue = issueBlocker;
coverageIssue = new DefaultIssue().setKey("key4").setSeverity(Severity.MAJOR).setRuleKey(CoverageRule.decreasingLineCoverageRule("java"))
.setMessage(CoverageSensorTest.formatIssueMessage("path/code/coverage", 40.0, 50.0));

report.add(coverageIssue);

issuePathResolver = new DummyIssuePathResolver();
issuePathResolver.add(coverageIssue, "path/code/coverage");
}

Expand Down Expand Up @@ -123,13 +123,13 @@ public void testPrintIssueListBySeverityMarkdown() {
report.remove(coverageIssue);

assertEquals( "| *BLOCKER* - messageBlocker [[RepoBlocker:RuleBlocker](sonarqube/URL/coding_rules#rule_key=RepoBlocker:RuleBlocker)] |\n",
MarkdownPrinter.printIssueListBySeverityMarkdown(report, SONAR_URL, "BLOCKER"));
MarkdownPrinter.printIssueListBySeverityMarkdown(1000, report, SONAR_URL, "BLOCKER"));

assertEquals( "| *CRITICAL* - messageCritical [[RepoCritical:RuleCritical](sonarqube/URL/coding_rules#rule_key=RepoCritical:RuleCritical)] |\n",
MarkdownPrinter.printIssueListBySeverityMarkdown(report, SONAR_URL, "CRITICAL"));
MarkdownPrinter.printIssueListBySeverityMarkdown(1000, report, SONAR_URL, "CRITICAL"));

assertEquals("| *MAJOR* - messageMajor [[RepoMajor:RuleMajor](sonarqube/URL/coding_rules#rule_key=RepoMajor:RuleMajor)] |\n",
MarkdownPrinter.printIssueListBySeverityMarkdown(report, SONAR_URL, "MAJOR"));
MarkdownPrinter.printIssueListBySeverityMarkdown(1000, report, SONAR_URL, "MAJOR"));
}

private String printReportMarkdown(List<Issue> report, int issueThreshold) {
Expand Down Expand Up @@ -240,7 +240,7 @@ public void testPrintReportMarkdownWithEmptyCoverageReport() {
}

private String printCoverageReportMarkdown(List<Issue> report, double projectCoverage, double previousProjectCoverage) {
return MarkdownPrinter.printCoverageReportMarkdown("project", "repo", 1, report, STASH_URL, projectCoverage, previousProjectCoverage, issuePathResolver);
return MarkdownPrinter.printCoverageReportMarkdown(1000, "project", "repo", 1, report, STASH_URL, projectCoverage, previousProjectCoverage, issuePathResolver);
}

@Test
Expand Down Expand Up @@ -299,4 +299,28 @@ public void testPrintCoverageReportMarkdownWithNoLoweredIssues() {

assertEquals(expectedReportMarkdown, reportMarkdown);
}

@Test
public void testPrintReportMarkdownMaxLength() {
for(int i=0; i < 5000; i++) {
Issue issueBlocker = new DefaultIssue().setKey("key"+i).setSeverity(Severity.BLOCKER).setMessage("messageBlocker"+i).setRuleKey(RuleKey.of("RepoBlocker"+i, "RuleBlocker"+i)).setLine(1);
Issue issueCritical = new DefaultIssue().setKey("key"+i).setSeverity(Severity.CRITICAL).setMessage("messageCritical"+i).setRuleKey(RuleKey.of("RepoCritical"+i, "RuleCritical"+i)).setLine(1);
Issue issueMajor = new DefaultIssue().setKey("key"+i).setSeverity(Severity.MAJOR).setMessage("messageMajor"+i).setRuleKey(RuleKey.of("RepoMajor"+i, "RuleMajor"+i)).setLine(1);

report.add(issueBlocker);
report.add(issueCritical);
report.add(issueMajor);

coverageIssue = new DefaultIssue().setKey("key"+i).setSeverity(Severity.MAJOR).setRuleKey(CoverageRule.decreasingLineCoverageRule("java"))
.setMessage(CoverageSensorTest.formatIssueMessage("path/code/coverage"+i, 40.0, 50.0));

report.add(coverageIssue);
}

String issueReportMarkdown = printReportMarkdown(report, 100);

assertThat(issueReportMarkdown.length(), lessThanOrEqualTo((int) Short.MAX_VALUE));
assertThat(issueReportMarkdown, containsString("| *BLOCKER* - The rest issues are skipped |"));
assertThat(issueReportMarkdown, containsString("| The rest issues are skipped |"));
}
}

0 comments on commit bba85d4

Please sign in to comment.