Skip to content

Commit

Permalink
Gerrit won't add deleted files to review, closes TouK#1
Browse files Browse the repository at this point in the history
Change-Id: I3d67e8a2433222d66f87de7a55d2f4d4cf63d047
  • Loading branch information
SpOOnman committed Jul 2, 2014
1 parent 49dbf20 commit 95fe509
Show file tree
Hide file tree
Showing 15 changed files with 47 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ public List<ReviewFile> listFiles() {
if (COMMIT_MSG.equals(stringFileInfoEntry.getKey())) {
continue;
}
files.add(new ReviewFile(stringFileInfoEntry.getKey(), stringFileInfoEntry.getValue().getStatus().getModificationType()));
FileInfo value = stringFileInfoEntry.getValue();
if (value.getStatus() == FileInfo.Status.DELETED) {
continue;
}
files.add(new ReviewFile(stringFileInfoEntry.getKey()));
}
return files;
} catch (IOException | URISyntaxException e) {
Expand Down
18 changes: 7 additions & 11 deletions src/main/java/pl/touk/sputnik/connector/gerrit/json/FileInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
import com.fasterxml.jackson.annotation.JsonValue;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.Getter;
import lombok.NoArgsConstructor;
import pl.touk.sputnik.review.ModificationType;

/**
* Gerrit FileInfo entry.
Expand All @@ -22,15 +20,13 @@ public class FileInfo {

@AllArgsConstructor
public enum Status {
MODIFIED(ModificationType.MODIFIED, "M"),
ADDED(ModificationType.ADDED, "A"),
DELETED(ModificationType.DELETED, "D"),
RENAMED(ModificationType.RENAMED, "R"),
COPIED(ModificationType.COPIED, "C"),
REWRITTEN(ModificationType.REWRITTEN, "R");

@Getter
private final ModificationType modificationType;
MODIFIED("M"),
ADDED("A"),
DELETED("D"),
RENAMED("R"),
COPIED("C"),
REWRITTEN("R");

private final String symbol;

@JsonValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void setLabelToPlusOne() {
labels.put(CODE_REVIEW, 1);
}

public int getRevievCount() {
public int getReviewCount() {
int count = 0;
for (Map.Entry<String, List<ReviewFileComment>> reviewFile : comments.entrySet()) {
count += reviewFile.getValue().size();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,20 @@
import com.jayway.jsonpath.JsonPath;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONObject;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.impl.client.CloseableHttpClient;
import org.jetbrains.annotations.NotNull;
import pl.touk.sputnik.Connectors;
import pl.touk.sputnik.connector.ConnectorFacade;
import pl.touk.sputnik.connector.gerrit.json.ReviewFileComment;
import pl.touk.sputnik.connector.gerrit.json.ReviewInput;
import pl.touk.sputnik.connector.gerrit.json.ReviewLineComment;
import pl.touk.sputnik.connector.http.HttpConnector;
import pl.touk.sputnik.connector.stash.json.*;
import pl.touk.sputnik.review.ModificationType;
import pl.touk.sputnik.review.ReviewFile;

import java.io.IOException;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import pl.touk.sputnik.Connectors;

@Slf4j
public class StashFacade implements ConnectorFacade {
Expand All @@ -49,7 +45,7 @@ public List<ReviewFile> listFiles() {
List<ReviewFile> files = new ArrayList<>();
for (ReviewElement container : containers) {
String filePath = String.format("%s/%s", container.parent, container.name);
files.add(new ReviewFile(filePath, ModificationType.MODIFIED));
files.add(new ReviewFile(filePath));
}
return files;
} catch (URISyntaxException | IOException e) {
Expand Down
10 changes: 0 additions & 10 deletions src/main/java/pl/touk/sputnik/review/ModificationType.java

This file was deleted.

4 changes: 1 addition & 3 deletions src/main/java/pl/touk/sputnik/review/ReviewFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ public class ReviewFile {
private final String reviewFilename;
private final String javaClassName;
private final File ioFile;
private final ModificationType modificationType;
private final List<Comment> comments = new ArrayList<>();

public ReviewFile(@NotNull String reviewFilename, @NotNull ModificationType modificationType) {
public ReviewFile(@NotNull String reviewFilename) {
this.reviewFilename = reviewFilename;
this.modificationType = modificationType;
this.javaClassName = createJavaClassName();
this.ioFile = new File(reviewFilename);
}
Expand Down
3 changes: 1 addition & 2 deletions src/test/java/pl/touk/sputnik/ReviewFileTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package pl.touk.sputnik;

import org.junit.Test;
import pl.touk.sputnik.review.ModificationType;
import pl.touk.sputnik.review.ReviewFile;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -72,6 +71,6 @@ public void shouldReturnTestJavaBuildDirectory() {
}

private ReviewFile createReviewFile(String reviewFileName) {
return new ReviewFile(reviewFileName, ModificationType.MODIFIED);
return new ReviewFile(reviewFileName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ public void shouldGetChangeInfo() throws Exception {
.willReturn(aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(IOUtils.toString(getClass().getResourceAsStream("/gerrit-changes.json")))));
.withBody(IOUtils.toString(getClass().getResourceAsStream("/json/gerrit-listfiles.json")))));

//when
List<ReviewFile> files = fixture.listFiles();

//then
assertThat(files).hasSize(2);
assertThat(files).hasSize(1);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ public void shouldGetChangeInfo() throws Exception {
.willReturn(aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(IOUtils.toString(getClass().getResourceAsStream("/gerrit-changes.json")))));
.withBody(IOUtils.toString(getClass().getResourceAsStream("/json/gerrit-listfiles.json")))));

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

assertThat(files).hasSize(2);
assertThat(files).hasSize(1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ public void shouldParseListFilesResponse() throws IOException, URISyntaxExceptio

List<ReviewFile> reviewFiles = gerritFacade.listFiles();

assertThat(reviewFiles).isNotEmpty();
}

@Test
public void shouldNotListDeletedFiles() throws IOException, URISyntaxException {
String listFilesJson = Resources.toString(Resources.getResource("json/gerrit-listfiles.json"), Charsets.UTF_8);
when(gerritConnectorMock.listFiles()).thenReturn(listFilesJson);

List<ReviewFile> reviewFiles = gerritFacade.listFiles();

assertThat(reviewFiles).hasSize(1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import pl.touk.sputnik.configuration.ConfigurationHolder;
import pl.touk.sputnik.review.ModificationType;
import pl.touk.sputnik.review.Review;
import pl.touk.sputnik.review.ReviewFile;
import pl.touk.sputnik.review.ReviewResult;
Expand Down Expand Up @@ -40,7 +39,7 @@ public void tearDown() throws Exception {
@Test
public void shouldReturnNotFoundViolation() {
//given
Review review = new Review(ImmutableList.of(new ReviewFile("test", ModificationType.MODIFIED)), true);
Review review = new Review(ImmutableList.of(new ReviewFile("test")), true);

//when
ReviewResult reviewResult = fixture.process(review);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import pl.touk.sputnik.configuration.ConfigurationHolder;
import pl.touk.sputnik.review.ModificationType;
import pl.touk.sputnik.review.Review;
import pl.touk.sputnik.review.ReviewFile;
import pl.touk.sputnik.review.ReviewResult;
Expand Down Expand Up @@ -40,7 +39,7 @@ public void tearDown() throws Exception {
@Test
public void shouldReturnEmptyResultWhenFileNotFound() {
//given
Review review = new Review(ImmutableList.of(new ReviewFile("test", ModificationType.MODIFIED)), true);
Review review = new Review(ImmutableList.of(new ReviewFile("test")), true);

//when
ReviewResult reviewResult = fixture.process(mock(Review.class));
Expand All @@ -55,7 +54,7 @@ public void shouldReturnEmptyResultWhenFileNotFound() {
@Ignore
public void shouldReturnBasicViolationsOnEmptyClass() {
//given
Review review = new Review(ImmutableList.of(new ReviewFile(Resources.getResource("TestFile.java").getFile(), ModificationType.MODIFIED)), true);
Review review = new Review(ImmutableList.of(new ReviewFile(Resources.getResource("TestFile.java").getFile())), true);

//when
ReviewResult reviewResult = fixture.process(review);
Expand Down
18 changes: 9 additions & 9 deletions src/test/java/pl/touk/sputnik/review/ReviewTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@

public class ReviewTest {

public Review prepare(boolean revievTestFiles) {
public Review prepare(boolean reviewTestFiles) {
List<ReviewFile> reviewList = ImmutableList.of(
new ReviewFile("/src/main/java/file1.java", ModificationType.MODIFIED),
new ReviewFile("/src/main/java/file2.java", ModificationType.MODIFIED),
new ReviewFile("/src/test/java/file1.java", ModificationType.MODIFIED),
new ReviewFile("/src/test/java/file2.java", ModificationType.MODIFIED)
new ReviewFile("/src/main/java/file1.java"),
new ReviewFile("/src/main/java/file2.java"),
new ReviewFile("/src/test/java/file1.java"),
new ReviewFile("/src/test/java/file2.java")
);

Review review = new Review(reviewList, revievTestFiles);
Review review = new Review(reviewList, reviewTestFiles);

// Create warnings for all files
int i = 0;
Expand All @@ -42,7 +42,7 @@ public void shouldConvertToReviewInput() {
//then
assertThat(reviewInput.comments)
.hasSize(4);
assertThat(reviewInput.getRevievCount() == 10);
assertThat(reviewInput.getReviewCount() == 10);
}

@Test
Expand All @@ -57,7 +57,7 @@ public void shouldNotProcessTestFiles() {
assertThat(reviewInput.comments)
.hasSize(2)
.containsKeys("/src/main/java/file1.java", "/src/main/java/file2.java");
assertThat(reviewInput.getRevievCount() == 3);
assertThat(reviewInput.getReviewCount() == 3);
}

@Test
Expand All @@ -74,7 +74,7 @@ public void shouldNotProcessMoreFiles() {
assertThat(reviewInput.comments)
.hasSize(2);

assertThat(reviewInput.getRevievCount() == 3);
assertThat(reviewInput.getReviewCount() == 3);
}

}
8 changes: 0 additions & 8 deletions src/test/resources/gerrit-changes.json

This file was deleted.

4 changes: 4 additions & 0 deletions src/test/resources/json/gerrit-listfiles.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,9 @@
"gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": {
"lines_inserted": 5,
"lines_deleted": 3
},
"gerrit-server/src/main/java/com/google/gerrit/server/project/DeletedClass.java": {
"lines_deleted": 3,
"status": "D"
}
}

0 comments on commit 95fe509

Please sign in to comment.