Skip to content

Commit

Permalink
Comment individual commits
Browse files Browse the repository at this point in the history
  • Loading branch information
tomasbjerre committed May 24, 2015
1 parent 7068ccd commit d5d5c31
Show file tree
Hide file tree
Showing 11 changed files with 193 additions and 50 deletions.
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Jenkins Violation Comments to Stash Plugin
# Violation Comments to Stash Plugin

Changelog of Jenkins Violation Comments to Stash Plugin
Changelog of Violation Comments to Stash Plugin

## 1.2
* Can, optionally, add comments to individual commits. And/Or to pull requests.

## 1.1
* Adding limit parameter to changes request in Stash Client. So that all files in PR gets commented.
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ There is a screenshot of the configuration GUI [here](https://raw.githubusercont
Available in Jenkins [here](https://wiki.jenkins-ci.org/display/JENKINS/Violation+Comments+to+Stash+Plugin).

#Features
* Comment pull requests with code analyzers comments
* Comment pull requests, or individual commits, with code analyzers comments
* Supporting: CheckStyle, CSSLint, JSLint, CodeNarc, CPPLint, FindBugs, FxCop, Gendarme, JCEReport, PEP8, PerlCritic, PMD, PyLint, Simian, StyleCop

## Use case
Here is an example use case where a pull request is triggered from Stash, merged, checked and comments added to pull request in Stash.

You may also use it for an ordinary build job, to simply comment the commit that was built.

### Notify Jenkins from Stash
You may use [Pull Request Notifier for Stash](https://github.com/tomasbjerre/pull-request-notifier-for-stash) to trigger a Jenkins build from an event in Stash. It can supply any parameters and variables you may need. Here is an example URL.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class ConfigurationWebTest {
private static final String JSLINT_PATTERN = "descriptor.config.parserConfigs[14].pattern";
private static final String CSSLINT_PATTERN = "descriptor.config.parserConfigs[15].pattern";
private static final String STASH_PULL_REQUEST_ID = "stashPullRequestId";
private static final String COMMIT_HASH = "commitHash";
private static final String STASH_REPO = "stashRepo";
private static final String STASH_PROJECT = "stashProject";
private static final String STASH_BASE_URL = "stashBaseUrl";
Expand Down Expand Up @@ -97,6 +98,7 @@ public void testPluginConfiguration() throws InterruptedException {
assertTrue(consoleText, consoleText.contains("stashBaseUrl: " + HTTP_LOCALHOST_8456));
assertTrue(consoleText, consoleText.contains("stashProject: ABC"));
assertTrue(consoleText, consoleText.contains("stashPullRequestId: 100"));
assertTrue(consoleText, consoleText.contains("commitHash: abcd1234"));
assertTrue(consoleText, consoleText.contains("stashRepo: a_repo"));
assertTrue(consoleText, consoleText.contains("checkstyle: **/checkstyle-report.xml"));
assertTrue(consoleText, consoleText.contains("checkstyle pathPrefix: pathPrefix"));
Expand Down Expand Up @@ -127,6 +129,7 @@ public void testPluginConfiguration() throws InterruptedException {
webDriver.findElement(name(STASH_PROJECT)).clear();
webDriver.findElement(name(STASH_REPO)).clear();
webDriver.findElement(name(STASH_PULL_REQUEST_ID)).clear();
webDriver.findElement(name(COMMIT_HASH)).clear();
webDriver.findElement(name(PATH_PREFIX)).clear();
webDriver.findElement(name(CHECKSTYLE_PATTERN)).clear();
webDriver.findElement(name(CODENARC_PATTERN)).clear();
Expand All @@ -150,6 +153,7 @@ public void testPluginConfiguration() throws InterruptedException {
webDriver.findElement(name(STASH_PROJECT)).sendKeys("DEF");
webDriver.findElement(name(STASH_REPO)).sendKeys("a_repo2");
webDriver.findElement(name(STASH_PULL_REQUEST_ID)).sendKeys("101");
webDriver.findElement(name(COMMIT_HASH)).sendKeys("abcd12345");
webDriver.findElement(name(CHECKSTYLE_PATTERN)).sendKeys("**/new-checkstyle-report.xml");
webDriver.findElement(name(PMD_PATTERN)).sendKeys("**/new-pmd-report.xml");
webDriver.findElement(name(JSLINT_PATTERN)).sendKeys("**/new-jslint-report.xml");
Expand All @@ -174,6 +178,7 @@ public void testPluginConfiguration() throws InterruptedException {
assertTrue(consoleText, consoleText.contains("stashBaseUrl: http://changed.com"));
assertTrue(consoleText, consoleText.contains("stashProject: DEF"));
assertTrue(consoleText, consoleText.contains("stashPullRequestId: 101"));
assertTrue(consoleText, consoleText.contains("commitHash: abcd12345"));
assertTrue(consoleText, consoleText.contains("stashRepo: a_repo2"));
assertTrue(consoleText, consoleText.contains("checkstyle: **/new-checkstyle-report.xml"));
assertTrue(consoleText, consoleText.contains("pmd: **/new-pmd-report.xml"));
Expand Down Expand Up @@ -202,6 +207,7 @@ public void testPluginConfiguration() throws InterruptedException {
assertEquals("DEF", webDriver.findElement(name(STASH_PROJECT)).getAttribute("value"));
assertEquals("a_repo2", webDriver.findElement(name(STASH_REPO)).getAttribute("value"));
assertEquals("101", webDriver.findElement(name(STASH_PULL_REQUEST_ID)).getAttribute("value"));
assertEquals("abcd12345", webDriver.findElement(name(COMMIT_HASH)).getAttribute("value"));
assertEquals("**/new-checkstyle-report.xml", webDriver.findElement(name(CHECKSTYLE_PATTERN)).getAttribute("value"));
assertEquals("**/new-pmd-report.xml", webDriver.findElement(name(PMD_PATTERN)).getAttribute("value"));
assertEquals("**/new-jslint-report.xml", webDriver.findElement(name(JSLINT_PATTERN)).getAttribute("value"));
Expand Down Expand Up @@ -229,6 +235,7 @@ public void testPluginConfiguration() throws InterruptedException {
webDriver.findElement(name(STASH_PROJECT)).clear();
webDriver.findElement(name(STASH_REPO)).clear();
webDriver.findElement(name(STASH_PULL_REQUEST_ID)).clear();
webDriver.findElement(name(COMMIT_HASH)).clear();
webDriver.findElement(name(CHECKSTYLE_PATTERN)).clear();
webDriver.findElement(name(CODENARC_PATTERN)).clear();
webDriver.findElement(name(CPD_PATTERN)).clear();
Expand All @@ -251,6 +258,7 @@ public void testPluginConfiguration() throws InterruptedException {
webDriver.findElement(name(STASH_PROJECT)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(STASH_REPO)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(STASH_PULL_REQUEST_ID)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(COMMIT_HASH)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(CHECKSTYLE_PATTERN)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(CODENARC_PATTERN)).sendKeys("$BUILD_NUMBER");
webDriver.findElement(name(CPD_PATTERN)).sendKeys("$BUILD_NUMBER");
Expand All @@ -275,6 +283,7 @@ public void testPluginConfiguration() throws InterruptedException {
assertTrue(consoleText, consoleText.contains("stashBaseUrl: 3"));
assertTrue(consoleText, consoleText.contains("stashProject: 3"));
assertTrue(consoleText, consoleText.contains("stashPullRequestId: 3"));
assertTrue(consoleText, consoleText.contains("commitHash: 3"));
assertTrue(consoleText, consoleText.contains("stashRepo: 3"));
assertTrue(consoleText, consoleText.contains("checkstyle: 3"));
assertTrue(consoleText, consoleText.contains("pmd: 3"));
Expand Down Expand Up @@ -303,6 +312,7 @@ public void testPluginConfiguration() throws InterruptedException {
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(STASH_PROJECT)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(STASH_REPO)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(STASH_PULL_REQUEST_ID)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(COMMIT_HASH)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(CHECKSTYLE_PATTERN)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(CODENARC_PATTERN)).getAttribute("value"));
assertEquals("$BUILD_NUMBER", webDriver.findElement(name(CPD_PATTERN)).getAttribute("value"));
Expand Down Expand Up @@ -332,6 +342,7 @@ private void enterDetails() {
webDriver.findElement(name(STASH_PROJECT)).sendKeys("ABC");
webDriver.findElement(name(STASH_REPO)).sendKeys("a_repo");
webDriver.findElement(name(STASH_PULL_REQUEST_ID)).sendKeys("100");
webDriver.findElement(name(COMMIT_HASH)).sendKeys("abcd1234");
webDriver.findElement(name(PATH_PREFIX)).sendKeys("pathPrefix");
webDriver.findElement(name(CHECKSTYLE_PATTERN)).sendKeys("**/checkstyle-report.xml");
webDriver.findElement(name(PMD_PATTERN)).sendKeys("**/pmd-report.xml");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jenkinsci.plugins.jvcts;

import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_COMMIT_HASH;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_PATTERN;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_PREFIX;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_BASE_URL;
Expand Down Expand Up @@ -52,6 +53,7 @@ public Publisher newInstance(StaplerRequest req, JSONObject formData) throws hud
config.setStashPassword(formData.getString(FIELD_STASH_PASSWORD));
config.setStashRepo(formData.getString(FIELD_STASH_REPO));
config.setStashPullRequestId(formData.getString(FIELD_STASH_PULL_REQUEST_ID));
config.setCommitHash(formData.getString(FIELD_COMMIT_HASH));
int i = 0;
for (String pattern : (List<String>) formData.get(FIELD_PATTERN)) {
config.getParserConfigs().get(i++).setPattern(pattern);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class ViolationsToStashConfig {
private String stashProject;
private String stashRepo;
private String stashPullRequestId;
private String commitHash;

public ViolationsToStashConfig() {

Expand Down Expand Up @@ -75,4 +76,12 @@ public String getStashPassword() {
public String getStashUser() {
return stashUser;
}

public void setCommitHash(String commitHash) {
this.commitHash = commitHash;
}

public String getCommitHash() {
return commitHash;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class ViolationsToStashConfigHelper {
public static final String FIELD_PATTERN = "pattern";
public static final String FIELD_PREFIX = "pathPrefix";
public static final String FIELD_STASH_PULL_REQUEST_ID = "stashPullRequestId";
public static final String FIELD_COMMIT_HASH = "commitHash";
public static final String FIELD_STASH_REPO = "stashRepo";
public static final String FIELD_STASH_PROJECT = "stashProject";
public static final String FIELD_STASH_BASE_URL = "stashBaseUrl";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
import static java.util.logging.Level.FINE;
import static java.util.logging.Level.SEVERE;
import static org.jenkinsci.plugins.jvcts.JvctsLogger.doLog;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_COMMIT_HASH;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_BASE_URL;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PROJECT;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PULL_REQUEST_ID;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_REPO;
import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_USER;
import hudson.EnvVars;
import hudson.model.BuildListener;
import hudson.model.AbstractBuild;
Expand Down Expand Up @@ -62,15 +64,24 @@ static void doPerform(ViolationsToStashConfig config, File workspace, BuildListe
private static void commentStash(Map<String, List<Violation>> violationsPerFile, ViolationsToStashConfig config,
BuildListener listener) throws MalformedURLException {
JvctsStashClient jvctsStashClient = new JvctsStashClient(config, listener);
for (String changedFileInStash : jvctsStashClient.getChangedFileInPullRequest()) {
/**
* Should always use filename reported by Stash, then we know Stash will
* recognize it.
*/
doLog(listener, FINE, "Changed file in pull request: \"" + changedFileInStash + "\"");
jvctsStashClient.removeCommentsFromPullRequest(changedFileInStash);
for (Violation violation : getViolationsForFile(violationsPerFile, changedFileInStash, listener)) {
jvctsStashClient.commentPullRequest(changedFileInStash, violation.getLine(), constructCommentMessage(violation));
if (!isNullOrEmpty(config.getStashPullRequestId())) {
doLog(FINE, "Commenting pull request \"" + config.getStashPullRequestId() + "\"");
for (String changedFileInStash : jvctsStashClient.getChangedFileInPullRequest()) {
logger.log(FINE, "Changed file in pull request: \"" + changedFileInStash + "\"");
jvctsStashClient.removeCommentsFromPullRequest(changedFileInStash);
for (Violation violation : getViolationsForFile(violationsPerFile, changedFileInStash, listener)) {
jvctsStashClient.commentPullRequest(changedFileInStash, violation.getLine(), constructCommentMessage(violation));
}
}
}
if (!isNullOrEmpty(config.getCommitHash())) {
doLog(FINE, "Commenting commit \"" + config.getCommitHash() + "\"");
for (String changedFileInStash : jvctsStashClient.getChangedFileInCommit()) {
logger.log(FINE, "Changed file in commit: \"" + changedFileInStash + "\"");
jvctsStashClient.removeCommentsCommit(changedFileInStash);
for (Violation violation : getViolationsForFile(violationsPerFile, changedFileInStash, listener)) {
jvctsStashClient.commentCommit(changedFileInStash, violation.getLine(), constructCommentMessage(violation));
}
}
}
}
Expand Down Expand Up @@ -118,6 +129,7 @@ private static ViolationsToStashConfig expand(ViolationsToStashConfig config, En
expanded.setStashPassword(environment.expand(config.getStashPassword()));
expanded.setStashProject(environment.expand(config.getStashProject()));
expanded.setStashPullRequestId(environment.expand(config.getStashPullRequestId()));
expanded.setCommitHash(environment.expand(config.getCommitHash()));
expanded.setStashRepo(environment.expand(config.getStashRepo()));
for (ParserConfig parserConfig : config.getParserConfigs()) {
ParserConfig p = new ParserConfig();
Expand All @@ -133,9 +145,11 @@ private static ViolationsToStashConfig expand(ViolationsToStashConfig config, En
* Enables testing of configuration GUI.
*/
private static void logConfiguration(ViolationsToStashConfig config, AbstractBuild<?, ?> build, BuildListener listener) {
listener.getLogger().println(FIELD_STASH_USER + ": " + config.getCommitHash());
listener.getLogger().println(FIELD_STASH_BASE_URL + ": " + config.getStashBaseUrl());
listener.getLogger().println(FIELD_STASH_PROJECT + ": " + config.getStashProject());
listener.getLogger().println(FIELD_STASH_PULL_REQUEST_ID + ": " + config.getStashPullRequestId());
listener.getLogger().println(FIELD_COMMIT_HASH + ": " + config.getCommitHash());
listener.getLogger().println(FIELD_STASH_REPO + ": " + config.getStashRepo());
for (ParserConfig parserConfig : config.getParserConfigs()) {
listener.getLogger().println(parserConfig.getParserTypeDescriptorName() + ": " + parserConfig.getPattern());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ static void setStashInvoker(StashInvoker stashInvoker) {
}

public List<String> getChangedFileInPullRequest() {
String url = getStashPulLRequestBase() + "/changes?limit=999999";
return invokeAndParse(url, "$..path.toString");
return invokeAndParse(getStashPulLRequestBase() + "/changes?limit=999999", "$..path.toString");
}

public void commentPullRequest(String changedFile, int line, String message) {
Expand All @@ -58,8 +57,7 @@ private Map<String, Object> toMap(Object o) {
}

private JSONArray getCommentsOnPullRequest(String changedFile) {
String url = getStashPulLRequestBase() + "/comments?path=" + changedFile + "&limit=999999";
return invokeAndParse(url, "$.values[*]");
return invokeAndParse(getStashPulLRequestBase() + "/comments?path=" + changedFile + "&limit=999999", "$.values[*]");
}

private <T> T invokeAndParse(String url, String jsonPath) {
Expand All @@ -82,4 +80,38 @@ private String getStashPulLRequestBase() {
return config.getStashBaseUrl() + "/rest/api/1.0/projects/" + config.getStashProject() + "/repos/"
+ config.getStashRepo() + "/pull-requests/" + config.getStashPullRequestId();
}

private String getStashCommitsBase() {
return config.getStashBaseUrl() + "/rest/api/1.0/projects/" + config.getStashProject() + "/repos/"
+ config.getStashRepo() + "/commits/" + config.getCommitHash();
}

public List<String> getChangedFileInCommit() {
return invokeAndParse(getStashCommitsBase() + "/changes?limit=999999", "$..path.toString");
}

private JSONArray getCommentsOnCommit(String changedFile) {
return invokeAndParse(getStashCommitsBase() + "/comments?path=" + changedFile + "&limit=999999", "$.values[*]");
}

@SuppressWarnings("rawtypes")
public void removeCommentsCommit(String changedFile) {
for (Object comment : getCommentsOnCommit(changedFile)) {
if (toMap(toMap(comment).get("author")).get("name").equals(config.getStashUser())) {
removeCommentFromCommit((Map) comment);
}
}
}

private void removeCommentFromCommit(@SuppressWarnings("rawtypes") Map comment) {
stashInvoker.invokeUrl(config,
getStashCommitsBase() + "/comments/" + comment.get(ID) + "?version=" + comment.get(VERSION),
StashInvoker.Method.DELETE, "", listener);
}

public void commentCommit(String changedFile, int line, String message) {
String postContent = "{ \"text\": \"" + message.replaceAll("\"", "") + "\", \"anchor\": { \"line\": \"" + line
+ "\", \"lineType\": \"ADDED\", \"fileType\": \"TO\", \"path\": \"" + changedFile + "\" }}";
stashInvoker.invokeUrl(config, getStashCommitsBase() + "/comments", StashInvoker.Method.POST, postContent, listener);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,13 @@
<f:textbox name="stashRepo" value="${config.stashRepo}" />
</f:entry>

<f:entry title="Pull Request ID">
<f:entry title="(Optional) Pull Request ID, if you want to comment on the pull request">
<f:textbox name="stashPullRequestId" value="${config.stashPullRequestId}" />
</f:entry>

<f:entry title="(Optional) Commit hash, if you want to comment on the commit">
<f:textbox name="commitHash" value="${config.commitHash}" />
</f:entry>


<f:entry>
Expand All @@ -63,7 +67,7 @@
</td>
</tr>
<tr>
<th align="left" width="150">
<th align="left" width="300">
Pattern
</th>
<th align="left">
Expand All @@ -77,7 +81,7 @@
<f:entry title="${parserConfig.parserTypeDescriptorName}">
<table cellspacing="5">
<tr>
<td width="150">
<td width="300">
<f:textbox name="descriptor.config.parserConfigs[${i}].pattern" value="${parserConfig.pattern}" />
</td>
<td width="200">
Expand Down
Loading

0 comments on commit d5d5c31

Please sign in to comment.