diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/LimitCommentVisitor.java b/src/main/java/pl/touk/sputnik/engine/visitor/LimitCommentVisitor.java index aa127301..edc345a0 100644 --- a/src/main/java/pl/touk/sputnik/engine/visitor/LimitCommentVisitor.java +++ b/src/main/java/pl/touk/sputnik/engine/visitor/LimitCommentVisitor.java @@ -22,8 +22,8 @@ public void afterReview(@NotNull Review review) { return; } log.info("There are {} total violations for this review, which is higher than maximum comment count {}. {} comments will be filtered out.", review.getTotalViolationCount(), maximumCount, review.getTotalViolationCount() - maximumCount); - filterOutComments(review); addMessage(review); + filterOutComments(review); } private void filterOutComments(Review review) { diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitor.java b/src/main/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitor.java index 6434a854..9bfae9dc 100644 --- a/src/main/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitor.java +++ b/src/main/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitor.java @@ -27,7 +27,7 @@ private void addSummaryMessage(Review review) { } private String getSummaryMessage(@NotNull Review review) { - if (review.getTotalViolationCount() == 0) { + if (review.getTotalViolationCount() == 0L) { return perfectMessage; } String violationNoun = review.getTotalViolationCount() == 1 ? "violation" : "violations"; diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java b/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java index 362bafd7..4488d43c 100644 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java +++ b/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java @@ -19,8 +19,8 @@ public class ScorePassIfNoErrors implements AfterReviewVisitor { @Override public void afterReview(@NotNull Review review) { - Integer errorCount = review.getViolationCount().get(Severity.ERROR); - if (errorCount == null || errorCount == 0) { + long errorCount = review.getViolationCount(Severity.ERROR); + if (errorCount == 0) { log.info("Adding passing score {} for no errors found", passingScore); review.setScores(passingScore); return; diff --git a/src/main/java/pl/touk/sputnik/review/Comment.java b/src/main/java/pl/touk/sputnik/review/Comment.java index baab4151..05f7c2e9 100644 --- a/src/main/java/pl/touk/sputnik/review/Comment.java +++ b/src/main/java/pl/touk/sputnik/review/Comment.java @@ -10,4 +10,5 @@ public class Comment { private final int line; private final String message; + private final Severity severity; } diff --git a/src/main/java/pl/touk/sputnik/review/Review.java b/src/main/java/pl/touk/sputnik/review/Review.java index cb12c780..21391916 100644 --- a/src/main/java/pl/touk/sputnik/review/Review.java +++ b/src/main/java/pl/touk/sputnik/review/Review.java @@ -13,7 +13,7 @@ import pl.touk.sputnik.review.transformer.FileTransformer; import java.util.ArrayList; -import java.util.EnumMap; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -24,8 +24,6 @@ @ToString public class Review { private List files; - private Map violationCount = new EnumMap<>(Severity.class); - private int totalViolationCount = 0; /** * Report problems with configuration, processors and other. @@ -80,14 +78,20 @@ public void addError(String source, Violation violation) { } private void addError(@NotNull ReviewFile reviewFile, @NotNull String source, int line, @Nullable String message, Severity severity) { - reviewFile.getComments().add(new Comment(line, formatter.formatComment(source, severity, message))); - incrementCounters(severity); + reviewFile.getComments().add(new Comment(line, formatter.formatComment(source, severity, message), severity)); } - private void incrementCounters(Severity severity) { - totalViolationCount += 1; - Integer currentCount = violationCount.get(severity); - violationCount.put(severity, currentCount == null ? 1 : currentCount + 1); + public long getTotalViolationCount() { + return files.stream().map(ReviewFile::getComments) + .mapToLong(Collection::size) + .sum(); + } + + public long getViolationCount(Severity severity) { + return files.stream().map(ReviewFile::getComments) + .flatMap(Collection::stream) + .filter(comment -> comment.getSeverity() == severity) + .count(); } @NoArgsConstructor diff --git a/src/test/java/pl/touk/sputnik/ReviewBuilder.java b/src/test/java/pl/touk/sputnik/ReviewBuilder.java index 64bef8e0..99a717ad 100644 --- a/src/test/java/pl/touk/sputnik/ReviewBuilder.java +++ b/src/test/java/pl/touk/sputnik/ReviewBuilder.java @@ -5,6 +5,7 @@ import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; import pl.touk.sputnik.review.ReviewFormatterFactory; +import pl.touk.sputnik.review.Severity; import java.util.Arrays; import java.util.List; @@ -14,16 +15,15 @@ public class ReviewBuilder { public static Review buildReview(Configuration configuration) { List reviewFiles = Arrays.asList(buildReviewFile(1), buildReviewFile(2), buildReviewFile(3), buildReviewFile(4)); Review review = new Review(reviewFiles, ReviewFormatterFactory.get(configuration)); - review.setTotalViolationCount(8); review.getMessages().add("Total 8 violations found"); review.getScores().put("Code-Review", (short) 1); return review; } - public static ReviewFile buildReviewFile(int i) { + private static ReviewFile buildReviewFile(int i) { ReviewFile reviewFile = new ReviewFile("filename" + i); - reviewFile.getComments().add(new Comment(0, "test1")); - reviewFile.getComments().add(new Comment(1, "test2")); + reviewFile.getComments().add(new Comment(0, "test1", Severity.INFO)); + reviewFile.getComments().add(new Comment(1, "test2", Severity.INFO)); return reviewFile; } } diff --git a/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java b/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java index 7c0af374..f9b1ec10 100644 --- a/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java +++ b/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java @@ -17,6 +17,7 @@ import pl.touk.sputnik.review.Comment; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; +import pl.touk.sputnik.review.Severity; import java.io.IOException; import java.util.List; @@ -106,8 +107,8 @@ void shouldWarnWithCommentsAndLineNumbers() { ReviewFile review2 = mock(ReviewFile.class); ReviewFile review3 = mock(ReviewFile.class); when(review3.getReviewFilename()).thenReturn("/path/to/file3"); - when(review1.getComments()).thenReturn(ImmutableList.of(new Comment(11, "Comment 1"), new Comment(14, "Comment 2"))); - when(review3.getComments()).thenReturn(ImmutableList.of(new Comment(15, "Comment 3"))); + when(review1.getComments()).thenReturn(ImmutableList.of(new Comment(11, "Comment 1", Severity.INFO), new Comment(14, "Comment 2", Severity.INFO))); + when(review3.getComments()).thenReturn(ImmutableList.of(new Comment(15, "Comment 3", Severity.INFO))); when(review.getMessages()).thenReturn(ImmutableList.of("message 1", "message 2")); when(review.getFiles()).thenReturn(ImmutableList.of(review1, review2, review3)); diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitorTest.java b/src/test/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitorTest.java index a7e2ead5..5ebe8968 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitorTest.java +++ b/src/test/java/pl/touk/sputnik/engine/visitor/SummaryMessageVisitorTest.java @@ -1,55 +1,71 @@ package pl.touk.sputnik.engine.visitor; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import pl.touk.sputnik.TestEnvironment; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import pl.touk.sputnik.review.Review; +import java.util.ArrayList; +import java.util.List; + +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; -class SummaryMessageVisitorTest extends TestEnvironment { +@ExtendWith(MockitoExtension.class) +class SummaryMessageVisitorTest { private static final String TOTAL_8_VIOLATIONS_FOUND = "Total 8 violations found"; private static final String PROBLEM_SOURCE = "PMD"; private static final String PROBLEM_MESSAGE = "configuration error"; private static final String PROBLEM_FORMATTED_MESSAGE = "There is a problem with PMD: configuration error"; + private final SummaryMessageVisitor summaryMessageVisitor = new SummaryMessageVisitor("Perfect"); + + @Mock + private Review review; + private List messages = new ArrayList<>(); + + @BeforeEach + void setUp() { + when(review.getMessages()).thenReturn(messages); + } + @Test void shouldAddSummaryMessage() { - Review review = review(); - review.setTotalViolationCount(8); + when(review.getTotalViolationCount()).thenReturn(8L); - new SummaryMessageVisitor("Perfect").afterReview(review); + summaryMessageVisitor.afterReview(review); assertThat(review.getMessages()).containsOnly(TOTAL_8_VIOLATIONS_FOUND); } @Test void shouldAddSummaryMessageWithOneViolation() { - Review review = review(); - review.setTotalViolationCount(1); + when(review.getTotalViolationCount()).thenReturn(1L); - new SummaryMessageVisitor("Perfect").afterReview(review); + summaryMessageVisitor.afterReview(review); assertThat(review.getMessages()).containsOnly("Total 1 violation found"); } @Test void shouldAddPerfectMessageIfThereAreNoViolationsFound() { - Review review = review(); - review.setTotalViolationCount(0); + when(review.getTotalViolationCount()).thenReturn(0L); - new SummaryMessageVisitor("Perfect").afterReview(review); + summaryMessageVisitor.afterReview(review); assertThat(review.getMessages()).containsOnly("Perfect"); } @Test void shouldAddProblemMessagesPerfectMessageIfThereAreNoViolationsFound() { - Review review = review(); - review.setTotalViolationCount(8); - review.addProblem(PROBLEM_SOURCE, PROBLEM_MESSAGE); + when(review.getTotalViolationCount()).thenReturn(8L); + when(review.getProblems()).thenReturn(singletonList("There is a problem with PMD: configuration error")); - new SummaryMessageVisitor("Perfect").afterReview(review); + summaryMessageVisitor.afterReview(review); assertThat(review.getMessages()).containsSequence(TOTAL_8_VIOLATIONS_FOUND, PROBLEM_FORMATTED_MESSAGE); } diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java b/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java index 5fa7024f..a25445fb 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java +++ b/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java @@ -1,17 +1,21 @@ package pl.touk.sputnik.engine.visitor.score; import org.junit.jupiter.api.Test; -import pl.touk.sputnik.TestEnvironment; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import pl.touk.sputnik.review.Review; import static org.assertj.core.api.Assertions.assertThat; -class NoScoreTest extends TestEnvironment { +@ExtendWith(MockitoExtension.class) +class NoScoreTest { + + @Mock + private Review review; @Test void shouldAddNoScoreToReview() { - Review review = review(); - new NoScore().afterReview(review); assertThat(review.getScores()).isEmpty(); diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java index 30d694df..d7f3763c 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java +++ b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java @@ -1,19 +1,38 @@ package pl.touk.sputnik.engine.visitor.score; import com.google.common.collect.ImmutableMap; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import pl.touk.sputnik.TestEnvironment; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import pl.touk.sputnik.review.Review; +import pl.touk.sputnik.review.ReviewFile; +import pl.touk.sputnik.review.ReviewFormatter; + +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; -class ScoreAlwaysPassTest extends TestEnvironment { +@ExtendWith(MockitoExtension.class) +class ScoreAlwaysPassTest { + + private Review review; + + @Mock + private List files; + + @Mock + private ReviewFormatter reviewFormatter; + + @BeforeEach + void setUp() { + review = new Review(files, reviewFormatter); + } @Test void shouldAddScoreToReview() { - Review review = review(); - new ScoreAlwaysPass(ImmutableMap.of("Sputnik-Pass", (short) 1)).afterReview(review); assertThat(review.getScores()).containsOnly(entry("Sputnik-Pass", (short) 1)); diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java index da3f65d5..73c6abe9 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java +++ b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java @@ -2,23 +2,27 @@ import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import pl.touk.sputnik.review.Review; import java.util.Map; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@ExtendWith(MockitoExtension.class) class ScorePassIfEmptyTest { private static final Map PASSING_SCORE = ImmutableMap.of("Sputnik-Pass", (short) 1); private static final Map FAILING_SCORE = ImmutableMap.of("Code-Review", (short) -2); - private Review reviewMock = mock(Review.class); + @Mock + private Review reviewMock; @Test void shouldPassIfViolationsIsEmpty() { - when(reviewMock.getTotalViolationCount()).thenReturn(0); + when(reviewMock.getTotalViolationCount()).thenReturn(0L); new ScorePassIfEmpty(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); @@ -27,7 +31,7 @@ void shouldPassIfViolationsIsEmpty() { @Test void shouldFailIfViolationsIsNotEmpty() { - when(reviewMock.getTotalViolationCount()).thenReturn(1); + when(reviewMock.getTotalViolationCount()).thenReturn(1L); new ScorePassIfEmpty(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java index a44f609b..85ac934d 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java +++ b/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java @@ -2,34 +2,28 @@ import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.Severity; import java.util.Map; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@ExtendWith(MockitoExtension.class) class ScorePassIfNoErrorsTest { private static final Map PASSING_SCORE = ImmutableMap.of("Sputnik-Pass", (short) 1); private static final Map FAILING_SCORE = ImmutableMap.of("Code-Review", (short) -2); - private Review reviewMock = mock(Review.class, RETURNS_DEEP_STUBS); - - @Test - void shouldPassIfErrorCountIsNull() { - when(reviewMock.getViolationCount().get(Severity.ERROR)).thenReturn(null); - - new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); - - verify(reviewMock).setScores(PASSING_SCORE); - } + @Mock + private Review reviewMock; @Test void shouldPassIfErrorCountIsZero() { - when(reviewMock.getViolationCount().get(Severity.ERROR)).thenReturn(0); + when(reviewMock.getViolationCount(Severity.ERROR)).thenReturn(0L); new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); @@ -38,7 +32,7 @@ void shouldPassIfErrorCountIsZero() { @Test void shouldFailIfErrorCountIsNotZero() { - when(reviewMock.getViolationCount().get(Severity.ERROR)).thenReturn(1); + when(reviewMock.getViolationCount(Severity.ERROR)).thenReturn(1L); new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); diff --git a/src/test/java/pl/touk/sputnik/review/ReviewTest.java b/src/test/java/pl/touk/sputnik/review/ReviewTest.java new file mode 100644 index 00000000..3c731e35 --- /dev/null +++ b/src/test/java/pl/touk/sputnik/review/ReviewTest.java @@ -0,0 +1,67 @@ +package pl.touk.sputnik.review; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ReviewTest { + + @Mock + private ReviewFile file1; + + @Mock + private ReviewFile file2; + + @Mock + private ReviewFormatter reviewFormatter; + + private Review review; + + @BeforeEach + void setUp() { + review = new Review(asList(file1, file2), reviewFormatter); + } + + @Test + void shouldCountTotalViolationCountFromCommentsSize() { + when(file1.getComments()).thenReturn(asList(mockComment(), mockComment())); + when(file2.getComments()).thenReturn(singletonList(mockComment())); + + long totalViolationCount = review.getTotalViolationCount(); + + assertThat(totalViolationCount).isEqualTo(3); + } + + @Test + void shouldCountViolationsPerSeverity() { + Comment errorComment = mockComment(Severity.ERROR); + Comment infoComment1 = mockComment(Severity.INFO); + Comment infoComment2 = mockComment(Severity.INFO); + when(file1.getComments()).thenReturn(asList(errorComment, infoComment1)); + when(file2.getComments()).thenReturn(singletonList(infoComment2)); + + long totalViolationCount = review.getViolationCount(Severity.INFO); + + assertThat(totalViolationCount).isEqualTo(2); + } + + private Comment mockComment() { + return mock(Comment.class); + } + + private Comment mockComment(Severity severity) { + Comment comment = mock(Comment.class); + when(comment.getSeverity()).thenReturn(severity); + return comment; + } + +} \ No newline at end of file