Skip to content

Commit

Permalink
review comments + basic test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
pjagielski committed Oct 13, 2015
1 parent 09be2d7 commit 6e098cf
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 42 deletions.
6 changes: 1 addition & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ run {
}

repositories {
mavenLocal()
mavenCentral()
maven { url "http://central.maven.org/maven2" }
}
Expand All @@ -54,8 +53,6 @@ dependencies {
compile 'org.apache.httpcomponents:httpclient:4.3.1'
compile 'com.fasterxml.jackson.core:jackson-databind:2.3.0'
compile 'com.intellij:annotations:12.0'
compile 'org.apache.commons:commons-lang3:3.1'
compile 'org.apache.commons:commons-io:1.3.2'
compile 'org.slf4j:slf4j-api:1.7.5'
compile 'org.slf4j:log4j-over-slf4j:1.7.7'
compile 'ch.qos.logback:logback-classic:1.0.13'
Expand Down Expand Up @@ -110,13 +107,12 @@ dependencies {
compile 'pl.gildur:jshint4j:1.0.1'

// github connector
//compile 'com.jcabi:jcabi-github:0.18.1'
compile 'com.jcabi:jcabi-github:0.24'
compile 'com.github.spullara.mustache.java:compiler:0.8.17'

// Test dependencies
testCompile 'junit:junit:4.11'
testCompile 'org.mockito:mockito-all:1.9.5'
testCompile 'org.mockito:mockito-core:1.9.5'
testCompile 'org.assertj:assertj-core:1.5.0'
testCompile 'com.googlecode.catch-exception:catch-exception:1.2.0'
testCompile('com.github.tomakehurst:wiremock:1.46') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public enum GeneralOption implements ConfigurationOption {
USE_HTTPS("connector.useHttps", "Connector use https?", "false"),
USERNAME("connector.username", "Connector server username", "user"),
PASSWORD("connector.password", "Connector server password", "password"),
PROJECT_KEY("connector.projectKey", "Connector server projectKey", null),
REPOSITORY_SLUG("connector.repositorySlug", "Connector server repositorySlug", null),
OWNER("connector.owner", "Connector server owner", null),
REPOSITORY("connector.repository", "Connector server repository", null),

SCORE_STRATEGY("score.strategy", "Score strategy: <NoScore|ScoreAlwaysPass|ScorePassIfEmpty|ScorePassIfNoErrors>", "ScoreAlwaysPass"),
SCORE_PASSING_KEY("score.passingKey", "Score passing key", "Code-Review"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.io.StringWriter;
import java.io.Writer;

public class ContentRenderer {
class ContentRenderer {

public static final String TEMPLATE_MUSTACHE = "issue.mustache";

Expand Down
18 changes: 1 addition & 17 deletions src/main/java/pl/touk/sputnik/connector/github/GithubFacade.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
@RequiredArgsConstructor
public class GithubFacade implements ConnectorFacade {



private final Repo repo;

private final GithubPatchset githubPatchset;
Expand Down Expand Up @@ -62,22 +60,8 @@ public void setReview(@NotNull Review review) {
new Status(getPull(), review, issueId).update();
}

/**
* post comment for github pull request
*/
private void postComment(Pull pull, int lineNo, String comment, String commitSha, String filename) {
try {
pull.comments().post(comment,
commitSha,
filename, lineNo);
} catch (IOException e) {
log.error("Error adding comment to file " + filename, e);
}
}

private Pull getPull() {
return repo.pulls().get(githubPatchset.pullRequestId);
return repo.pulls().get(githubPatchset.getPullRequestId());
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ public GithubFacade build(Configuration configuration) {
.through(RetryWire.class)
);

Repo repo = github.repos().get(new Coordinates.Simple(githubPatchset.projectPath));
Repo repo = github.repos().get(new Coordinates.Simple(githubPatchset.getProjectPath()));
return new GithubFacade(repo, githubPatchset);
}

@NotNull
public GithubPatchset buildGithubPatchset(Configuration configuration) {
String pullRequestId = configuration.getProperty(CliOption.PULL_REQUEST_ID);
String repositorySlug = configuration.getProperty(GeneralOption.REPOSITORY_SLUG);
String projectKey = configuration.getProperty(GeneralOption.PROJECT_KEY);
String repositorySlug = configuration.getProperty(GeneralOption.REPOSITORY);
String projectKey = configuration.getProperty(GeneralOption.OWNER);

notBlank(pullRequestId, "You must provide non blank Github pull request id");
isTrue(NumberUtils.isNumber(pullRequestId), "Integer value as pull request id required");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
@Data
@AllArgsConstructor
public class GithubPatchset {
public final Integer pullRequestId;
public final String projectPath;
private final Integer pullRequestId;
private final String projectPath;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import java.io.IOException;

@Slf4j
public class Notification {
class Notification {

public static final String SPUTNIK_PREFIX = "[Sputnik]";
public static final String ISSUE_TITLE = " Detected some code smells";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import java.io.IOException;

@Slf4j
public class ReviewStatus {
class ReviewStatus {

private static final String OK_STATUS = "All's well. Great job!";
private static final String NOK_STATUS = "There are some issues with your code. See the linked issue.";
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/pl/touk/sputnik/connector/github/Status.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.Iterator;

@Slf4j
public class Status {
class Status {
public static final String CONTEXT = "Sputnik";
private final Pull pull;
private final Review review;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ public StashFacade build(Configuration configuration) {
HttpClientContext httpClientContext = httpHelper.buildClientContext(httpHost, new BasicScheme());
CloseableHttpClient closeableHttpClient = httpHelper.buildClient(httpHost, connectorDetails);

return new StashFacade(new StashConnector(new HttpConnector(closeableHttpClient, httpClientContext, connectorDetails.getPath()), stashPatchset), configuration);
return new StashFacade(new StashConnector(
new HttpConnector(closeableHttpClient, httpClientContext, connectorDetails.getPath()), stashPatchset), configuration);
}

@NotNull
public StashPatchset buildStashPatchset(Configuration configuration) {
String pullRequestId = configuration.getProperty(CliOption.PULL_REQUEST_ID);
String repositorySlug = configuration.getProperty(GeneralOption.REPOSITORY_SLUG);
String projectKey = configuration.getProperty(GeneralOption.PROJECT_KEY);
String repositorySlug = configuration.getProperty(GeneralOption.REPOSITORY);
String projectKey = configuration.getProperty(GeneralOption.OWNER);

notBlank(pullRequestId, "You must provide non blank Stash pull request id");
notBlank(repositorySlug, "You must provide non blank Stash repository slug");
Expand Down
2 changes: 0 additions & 2 deletions src/test/java/pl/touk/sputnik/CliOptionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ private static class CliAssert {
CommandLine object;
private String optionName;



public CliAssert(CommandLine object) {
this.object = object;
}
Expand Down
106 changes: 106 additions & 0 deletions src/test/java/pl/touk/sputnik/connector/github/GithubFacadeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package pl.touk.sputnik.connector.github;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.jcabi.github.*;
import com.jcabi.immutable.Array;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import pl.touk.sputnik.configuration.Configuration;
import pl.touk.sputnik.configuration.ConfigurationSetup;
import pl.touk.sputnik.connector.FacadeConfigUtil;
import pl.touk.sputnik.review.*;

import javax.json.Json;
import javax.json.JsonObject;
import java.io.IOException;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
public class GithubFacadeTest {

private static Integer SOME_PULL_REQUEST_ID = 12314;
private static String SOME_REPOSITORY = "repo";
private static String SOME_PROJECT = "proj";

private static final Map<String, String> GITHUB_PATCHSET_MAP = ImmutableMap.of(
"cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(),
"connector.repository", SOME_REPOSITORY,
"connector.owner", SOME_PROJECT
);

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Repo repo;

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Pull pull;

@Mock
private Commit commit;

@Mock
private Commits commits;

@Mock
private Statuses statuses;

private GithubFacade githubFacade;
private Configuration config;

@Before
public void setUp() throws IOException {
when(repo.pulls().get(SOME_PULL_REQUEST_ID)).thenReturn(pull);
when(pull.commits()).thenReturn(pullCommits());
when(pull.repo().git().commits()).thenReturn(commits);

config = new ConfigurationSetup().setUp(FacadeConfigUtil.getHttpConfig("github"), GITHUB_PATCHSET_MAP);
githubFacade = new GithubFacade(repo, new GithubPatchset(SOME_PULL_REQUEST_ID, projectPath()));
}

@Test
public void shouldGetChangeInfo() throws Exception {
when(pull.files()).thenReturn(pullFiles());

List<ReviewFile> files = githubFacade.listFiles();

assertThat(files).extracting("reviewFilename").containsOnly("1.java");
}

@Test
public void shouldAddIssue() throws Exception {
when(commit.sha()).thenReturn("sha1");
when(commits.statuses("sha1")).thenReturn(statuses);

String filename = "src/main/java/Main.java";
Review review = new Review(ImmutableList.of(new ReviewFile(filename)), ReviewFormatterFactory.get(config));
review.addError("checkstyle", new Violation(filename, 1, "error message", Severity.ERROR));
review.getMessages().add("Total 1 violations found");

githubFacade.setReview(review);

verify(statuses).create(any(Statuses.StatusCreate.class));
}

private Iterable<Commit> pullCommits() {
return new Array<>(commit);
}


private List<JsonObject> pullFiles() {
return Json.createArrayBuilder().add(Json.createObjectBuilder().add("filename", "1.java").build()).build().getValuesAs(JsonObject.class);
}

private String projectPath() {
return String.format("%s/%s", SOME_PROJECT, SOME_REPOSITORY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public class StashFacadeHttpsTest {

private static final ImmutableMap<String, String> STASH_PATCHSET_MAP = ImmutableMap.of(
"cli.pullRequestId", SOME_PULL_REQUEST_ID,
"connector.repositorySlug", SOME_REPOSITORY,
"connector.projectKey", SOME_PROJECT_KEY
"connector.repository", SOME_REPOSITORY,
"connector.owner", SOME_PROJECT_KEY
);

private StashFacade stashFacade;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ public class StashFacadeTest {

private static final Map<String, String> STASH_PATCHSET_MAP = ImmutableMap.of(
"cli.pullRequestId", SOME_PULL_REQUEST_ID,
"connector.repositorySlug", SOME_REPOSITORY,
"connector.projectKey", SOME_PROJECT_KEY
"connector.repository", SOME_REPOSITORY,
"connector.owner", SOME_PROJECT_KEY
);

private StashFacade stashFacade;
Expand Down
26 changes: 26 additions & 0 deletions src/test/resources/json/github-files.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
[
{
"sha": "d8bb2fc90f4d8b15d5fcfc9869df04aea5124813",
"filename": "src/main/java/TestFile.java",
"status": "modified",
"additions": 2,
"deletions": 1,
"changes": 3,
"blob_url": "https://github.com/pjagielski/sputnik-test/blob/ce66cc07c1dabf4f8b76a3181e39a1bb339244ae/src/main/java/TestFile.java",
"raw_url": "https://github.com/pjagielski/sputnik-test/raw/ce66cc07c1dabf4f8b76a3181e39a1bb339244ae/src/main/java/TestFile.java",
"contents_url": "https://api.github.com/repos/pjagielski/sputnik-test/contents/src/main/java/TestFile.java?ref=ce66cc07c1dabf4f8b76a3181e39a1bb339244ae",
"patch": "@@ -7,8 +7,9 @@ private static void incorrectAssignmentInIfCondition() {\n boolean value = false;\n if (value = true) {\n //do Something\n+ value = false;\n } else {\n- //else Do Something\n+ //else Do Something 123\n }\n }\n }"
},
{
"sha": "dc7ca1de3414e87ca773d62d54287fe8c414316e",
"filename": "src/main/java/TestFile2.java",
"status": "added",
"additions": 13,
"deletions": 0,
"changes": 13,
"blob_url": "https://github.com/pjagielski/sputnik-test/blob/ce66cc07c1dabf4f8b76a3181e39a1bb339244ae/src/main/java/TestFile2.java",
"raw_url": "https://github.com/pjagielski/sputnik-test/raw/ce66cc07c1dabf4f8b76a3181e39a1bb339244ae/src/main/java/TestFile2.java",
"contents_url": "https://api.github.com/repos/pjagielski/sputnik-test/contents/src/main/java/TestFile2.java?ref=ce66cc07c1dabf4f8b76a3181e39a1bb339244ae",
"patch": "@@ -0,0 +1,13 @@\n+class TestFile2 {\n+ public String foo() {\n+ return \"bar\";\n+ }\n+\n+ private static void incorrectAssignmentInIfCondition() {\n+ boolean value = false;\n+ if (value = true) {\n+ //do Something\n+ value = false;\n+ }\n+ }\n+}"
}
]

0 comments on commit 6e098cf

Please sign in to comment.