From 8a27c638a69f5d3c2ecb2f04903c0c99ecb01238 Mon Sep 17 00:00:00 2001 From: Marquis Wong Date: Tue, 15 Oct 2019 09:50:59 -0500 Subject: [PATCH] Exit sputnik with error code if checks fail This allows scripts/tools to take the return result into account. Furthermore, Extensions of sputnik like the sputnik-maven-plugin can use a return value of the Engine to communicate the result of a run and do something like fail a build. Implement this by adding a Score object (with the review label and the score value as fields) and return that in Engine.run(). If a failing score value (< 0) is returned, then call System.exit with the score as the error status. --- src/main/java/pl/touk/sputnik/Main.java | 6 +- .../connector/gerrit/ReviewInputBuilder.java | 5 +- .../java/pl/touk/sputnik/engine/Engine.java | 10 +- .../touk/sputnik/engine/VisitorBuilder.java | 46 -------- .../pl/touk/sputnik/engine/score/NoScore.java | 17 +++ .../pl/touk/sputnik/engine/score/Score.java | 13 +++ .../sputnik/engine/score/ScoreAlwaysPass.java | 22 ++++ .../{visitor => }/score/ScorePassIfEmpty.java | 20 ++-- .../score/ScorePassIfNoErrors.java | 20 ++-- .../sputnik/engine/score/ScoreStrategy.java | 8 ++ .../engine/score/ScoreStrategyFactory.java | 48 +++++++++ .../sputnik/engine/visitor/score/NoScore.java | 15 --- .../engine/visitor/score/ScoreAlwaysPass.java | 23 ---- .../java/pl/touk/sputnik/review/Review.java | 4 +- .../java/pl/touk/sputnik/ReviewBuilder.java | 3 +- .../sputnik/engine/VisitorBuilderTest.java | 101 +----------------- .../{visitor => }/score/NoScoreTest.java | 6 +- .../engine/score/ScoreAlwaysPassTest.java | 21 ++++ .../engine/score/ScorePassIfEmptyTest.java | 35 ++++++ .../score/ScorePassIfNoErrorsTest.java | 24 ++--- .../score/ScoreStrategyFactoryTest.java | 71 ++++++++++++ .../visitor/score/ScoreAlwaysPassTest.java | 22 ---- .../visitor/score/ScorePassIfEmptyTest.java | 37 ------- 23 files changed, 294 insertions(+), 283 deletions(-) create mode 100644 src/main/java/pl/touk/sputnik/engine/score/NoScore.java create mode 100644 src/main/java/pl/touk/sputnik/engine/score/Score.java create mode 100644 src/main/java/pl/touk/sputnik/engine/score/ScoreAlwaysPass.java rename src/main/java/pl/touk/sputnik/engine/{visitor => }/score/ScorePassIfEmpty.java (52%) rename src/main/java/pl/touk/sputnik/engine/{visitor => }/score/ScorePassIfNoErrors.java (56%) create mode 100644 src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java create mode 100644 src/main/java/pl/touk/sputnik/engine/score/ScoreStrategyFactory.java delete mode 100644 src/main/java/pl/touk/sputnik/engine/visitor/score/NoScore.java delete mode 100644 src/main/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPass.java rename src/test/java/pl/touk/sputnik/engine/{visitor => }/score/NoScoreTest.java (69%) create mode 100644 src/test/java/pl/touk/sputnik/engine/score/ScoreAlwaysPassTest.java create mode 100644 src/test/java/pl/touk/sputnik/engine/score/ScorePassIfEmptyTest.java rename src/test/java/pl/touk/sputnik/engine/{visitor => }/score/ScorePassIfNoErrorsTest.java (50%) create mode 100644 src/test/java/pl/touk/sputnik/engine/score/ScoreStrategyFactoryTest.java delete mode 100644 src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java delete mode 100644 src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java diff --git a/src/main/java/pl/touk/sputnik/Main.java b/src/main/java/pl/touk/sputnik/Main.java index 9f6db727..1fd24356 100644 --- a/src/main/java/pl/touk/sputnik/Main.java +++ b/src/main/java/pl/touk/sputnik/Main.java @@ -14,6 +14,7 @@ import pl.touk.sputnik.connector.ConnectorFacadeFactory; import pl.touk.sputnik.connector.ConnectorType; import pl.touk.sputnik.engine.Engine; +import pl.touk.sputnik.engine.score.Score; public final class Main { private static final String SPUTNIK = "sputnik"; @@ -38,7 +39,10 @@ public static void main(String[] args) { configuration.updateWithCliOptions(commandLine); ConnectorFacade facade = getConnectorFacade(configuration); - new Engine(facade, facade, configuration).run(); + Score runResult = new Engine(facade, facade, configuration).run(); + if (runResult.getScore() < 0) { + System.exit(runResult.getScore()); + } } private static ConnectorFacade getConnectorFacade(Configuration configuration) { diff --git a/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java b/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java index e4facca9..a6c36cee 100644 --- a/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java @@ -10,6 +10,7 @@ import pl.touk.sputnik.review.ReviewFile; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -23,7 +24,9 @@ public class ReviewInputBuilder { public ReviewInput toReviewInput(@NotNull Review review) { ReviewInput reviewInput = new ReviewInput(); reviewInput.message = Joiner.on(". ").join(review.getMessages()); - reviewInput.labels = new HashMap(review.getScores()); + reviewInput.labels = review.getScore() == null + ? Collections.emptyMap() + : Collections.singletonMap(review.getScore().getLabel(), (short) review.getScore().getScore()); reviewInput.comments = new HashMap>(); for (ReviewFile file : review.getFiles()) { List comments = new ArrayList(); diff --git a/src/main/java/pl/touk/sputnik/engine/Engine.java b/src/main/java/pl/touk/sputnik/engine/Engine.java index 5b3d0715..343bfb3b 100644 --- a/src/main/java/pl/touk/sputnik/engine/Engine.java +++ b/src/main/java/pl/touk/sputnik/engine/Engine.java @@ -6,6 +6,9 @@ import pl.touk.sputnik.connector.ReviewPublisher; import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; import pl.touk.sputnik.engine.visitor.BeforeReviewVisitor; +import pl.touk.sputnik.engine.score.ScoreStrategy; +import pl.touk.sputnik.engine.score.ScoreStrategyFactory; +import pl.touk.sputnik.engine.score.Score; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; import pl.touk.sputnik.review.ReviewFormatterFactory; @@ -26,7 +29,7 @@ public Engine(ConnectorFacade facade, ReviewPublisher reviewPublisher, Configura this.config = configuration; } - public void run() { + public Score run() { List reviewFiles = facade.listFiles(); Review review = new Review(reviewFiles, ReviewFormatterFactory.get(config)); @@ -44,6 +47,11 @@ public void run() { afterReviewVisitor.afterReview(review); } + ScoreStrategy scoreStrategy = new ScoreStrategyFactory().buildScoreStrategy(config); + Score score = scoreStrategy.score(review); + review.setScore(score); reviewPublisher.publish(review); + + return score; } } diff --git a/src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java b/src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java index 43d74a12..845940f4 100644 --- a/src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java +++ b/src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java @@ -1,6 +1,5 @@ package pl.touk.sputnik.engine; -import com.google.common.collect.ImmutableMap; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.math.NumberUtils; @@ -14,24 +13,12 @@ import pl.touk.sputnik.engine.visitor.LimitCommentVisitor; import pl.touk.sputnik.engine.visitor.RegexFilterFilesVisitor; import pl.touk.sputnik.engine.visitor.SummaryMessageVisitor; -import pl.touk.sputnik.engine.visitor.score.NoScore; -import pl.touk.sputnik.engine.visitor.score.ScoreAlwaysPass; -import pl.touk.sputnik.engine.visitor.score.ScorePassIfEmpty; -import pl.touk.sputnik.engine.visitor.score.ScorePassIfNoErrors; import java.util.ArrayList; import java.util.List; -import java.util.Map; - -import static org.apache.commons.lang3.Validate.notBlank; @Slf4j public class VisitorBuilder { - private static final String NOSCORE = "NOSCORE"; - private static final String SCOREALWAYSPASS = "SCOREALWAYSPASS"; - private static final String SCOREPASSIFEMPTY = "SCOREPASSIFEMPTY"; - private static final String SCOREPASSIFNOERRORS = "SCOREPASSIFNOERRORS"; - @NotNull public List buildBeforeReviewVisitors(Configuration configuration) { List beforeReviewVisitors = new ArrayList<>(); @@ -65,40 +52,7 @@ public List buildAfterReviewVisitors(Configuration configura afterReviewVisitors.add(new LimitCommentVisitor(maxNumberOfComments)); } - afterReviewVisitors.add(buildScoreAfterReviewVisitor(configuration)); - return afterReviewVisitors; } - @NotNull - private AfterReviewVisitor buildScoreAfterReviewVisitor(Configuration configuration) { - Map passingScore = ImmutableMap.of( - configuration.getProperty(GeneralOption.SCORE_PASSING_KEY), - Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE)) - ); - Map failingScore = ImmutableMap.of( - configuration.getProperty(GeneralOption.SCORE_FAILING_KEY), - Short.valueOf(configuration.getProperty(GeneralOption.SCORE_FAILING_VALUE)) - ); - String scoreStrategy = configuration.getProperty(GeneralOption.SCORE_STRATEGY); - notBlank(scoreStrategy); - - switch(scoreStrategy.toUpperCase()) { - case NOSCORE: - return new NoScore(); - - case SCOREALWAYSPASS: - return new ScoreAlwaysPass(passingScore); - - case SCOREPASSIFEMPTY: - return new ScorePassIfEmpty(passingScore, failingScore); - - case SCOREPASSIFNOERRORS: - return new ScorePassIfNoErrors(passingScore, failingScore); - - default: - log.warn("Score strategy {} not found, using default ScoreAlwaysPass", scoreStrategy); - return new ScoreAlwaysPass(passingScore); - } - } } diff --git a/src/main/java/pl/touk/sputnik/engine/score/NoScore.java b/src/main/java/pl/touk/sputnik/engine/score/NoScore.java new file mode 100644 index 00000000..f33a86f3 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/NoScore.java @@ -0,0 +1,17 @@ +package pl.touk.sputnik.engine.score; + +import lombok.EqualsAndHashCode; +import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.review.Review; + +@Slf4j +@EqualsAndHashCode +public class NoScore implements ScoreStrategy { + + @Override + public Score score(@NotNull Review review) { + log.info("No score for review"); + return null; + } +} diff --git a/src/main/java/pl/touk/sputnik/engine/score/Score.java b/src/main/java/pl/touk/sputnik/engine/score/Score.java new file mode 100644 index 00000000..982d0016 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/Score.java @@ -0,0 +1,13 @@ +package pl.touk.sputnik.engine.score; + +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; + +@AllArgsConstructor +@Getter +@EqualsAndHashCode +public class Score { + private final String label; + private final int score; +} diff --git a/src/main/java/pl/touk/sputnik/engine/score/ScoreAlwaysPass.java b/src/main/java/pl/touk/sputnik/engine/score/ScoreAlwaysPass.java new file mode 100644 index 00000000..3d97b946 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/ScoreAlwaysPass.java @@ -0,0 +1,22 @@ +package pl.touk.sputnik.engine.score; + +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.review.Review; + +@Slf4j +@Getter +@EqualsAndHashCode +@AllArgsConstructor +public class ScoreAlwaysPass implements ScoreStrategy { + private final Score passingScore; + + @Override + public Score score(@NotNull Review review) { + log.info("Adding static passing score {} to review", passingScore); + return passingScore; + } +} diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java b/src/main/java/pl/touk/sputnik/engine/score/ScorePassIfEmpty.java similarity index 52% rename from src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java rename to src/main/java/pl/touk/sputnik/engine/score/ScorePassIfEmpty.java index 444e4979..41b6015a 100644 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java +++ b/src/main/java/pl/touk/sputnik/engine/score/ScorePassIfEmpty.java @@ -1,30 +1,28 @@ -package pl.touk.sputnik.engine.visitor.score; +package pl.touk.sputnik.engine.score; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; import pl.touk.sputnik.review.Review; -import java.util.Map; - @Slf4j @Getter +@EqualsAndHashCode @AllArgsConstructor -public class ScorePassIfEmpty implements AfterReviewVisitor { - private final Map passingScore; - private final Map failingScore; +public class ScorePassIfEmpty implements ScoreStrategy { + private final Score passingScore; + private final Score failingScore; @Override - public void afterReview(@NotNull Review review) { + public Score score(@NotNull Review review) { if (review.getTotalViolationCount() == 0) { log.info("Adding passing score {} for no violation(s) found", passingScore); - review.setScores(passingScore); - return; + return passingScore; } log.info("Adding failing score {} for {} violations found", failingScore, review.getTotalViolationCount()); - review.setScores(failingScore); + return failingScore; } } diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java b/src/main/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrors.java similarity index 56% rename from src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java rename to src/main/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrors.java index 362bafd7..83eba355 100644 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java +++ b/src/main/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrors.java @@ -1,32 +1,30 @@ -package pl.touk.sputnik.engine.visitor.score; +package pl.touk.sputnik.engine.score; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.Severity; -import java.util.Map; - @Slf4j @Getter +@EqualsAndHashCode @AllArgsConstructor -public class ScorePassIfNoErrors implements AfterReviewVisitor { - private final Map passingScore; - private final Map failingScore; +public class ScorePassIfNoErrors implements ScoreStrategy { + private final Score passingScore; + private final Score failingScore; @Override - public void afterReview(@NotNull Review review) { + public Score score(@NotNull Review review) { Integer errorCount = review.getViolationCount().get(Severity.ERROR); if (errorCount == null || errorCount == 0) { log.info("Adding passing score {} for no errors found", passingScore); - review.setScores(passingScore); - return; + return passingScore; } log.info("Adding failing score {} for {} errors found", failingScore, errorCount); - review.setScores(failingScore); + return failingScore; } } diff --git a/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java new file mode 100644 index 00000000..a4e410f0 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java @@ -0,0 +1,8 @@ +package pl.touk.sputnik.engine.score; + +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.review.Review; + +public interface ScoreStrategy { + Score score(@NotNull Review review); +} diff --git a/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategyFactory.java b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategyFactory.java new file mode 100644 index 00000000..35805983 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategyFactory.java @@ -0,0 +1,48 @@ +package pl.touk.sputnik.engine.score; + +import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.configuration.GeneralOption; + +import static org.apache.commons.lang3.Validate.notBlank; + +@Slf4j +public class ScoreStrategyFactory { + private static final String NOSCORE = "NOSCORE"; + private static final String SCOREALWAYSPASS = "SCOREALWAYSPASS"; + private static final String SCOREPASSIFEMPTY = "SCOREPASSIFEMPTY"; + private static final String SCOREPASSIFNOERRORS = "SCOREPASSIFNOERRORS"; + + @NotNull + public ScoreStrategy buildScoreStrategy(Configuration configuration) { + Score passingScore = new Score( + configuration.getProperty(GeneralOption.SCORE_PASSING_KEY), + Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE)) + ); + Score failingScore = new Score( + configuration.getProperty(GeneralOption.SCORE_FAILING_KEY), + Short.valueOf(configuration.getProperty(GeneralOption.SCORE_FAILING_VALUE)) + ); + String scoreStrategy = configuration.getProperty(GeneralOption.SCORE_STRATEGY); + notBlank(scoreStrategy); + + switch(scoreStrategy.toUpperCase()) { + case NOSCORE: + return new NoScore(); + + case SCOREALWAYSPASS: + return new ScoreAlwaysPass(passingScore); + + case SCOREPASSIFEMPTY: + return new ScorePassIfEmpty(passingScore, failingScore); + + case SCOREPASSIFNOERRORS: + return new ScorePassIfNoErrors(passingScore, failingScore); + + default: + log.warn("Score strategy {} not found, using default ScoreAlwaysPass", scoreStrategy); + return new ScoreAlwaysPass(passingScore); + } + } +} diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/score/NoScore.java b/src/main/java/pl/touk/sputnik/engine/visitor/score/NoScore.java deleted file mode 100644 index 9ac81762..00000000 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/NoScore.java +++ /dev/null @@ -1,15 +0,0 @@ -package pl.touk.sputnik.engine.visitor.score; - -import lombok.extern.slf4j.Slf4j; -import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; -import pl.touk.sputnik.review.Review; - -@Slf4j -public class NoScore implements AfterReviewVisitor { - - @Override - public void afterReview(@NotNull Review review) { - log.info("No score for review"); - } -} diff --git a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPass.java b/src/main/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPass.java deleted file mode 100644 index b2b6162d..00000000 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPass.java +++ /dev/null @@ -1,23 +0,0 @@ -package pl.touk.sputnik.engine.visitor.score; - -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.extern.slf4j.Slf4j; -import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; -import pl.touk.sputnik.review.Review; - -import java.util.Map; - -@Slf4j -@Getter -@AllArgsConstructor -public class ScoreAlwaysPass implements AfterReviewVisitor { - private final Map passingScore; - - @Override - public void afterReview(@NotNull Review review) { - log.info("Adding static passing score {} to review", passingScore); - review.getScores().putAll(passingScore); - } -} diff --git a/src/main/java/pl/touk/sputnik/review/Review.java b/src/main/java/pl/touk/sputnik/review/Review.java index cb12c780..6bf013b6 100644 --- a/src/main/java/pl/touk/sputnik/review/Review.java +++ b/src/main/java/pl/touk/sputnik/review/Review.java @@ -9,12 +9,12 @@ import lombok.extern.slf4j.Slf4j; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import pl.touk.sputnik.engine.score.Score; import pl.touk.sputnik.review.filter.FileFilter; import pl.touk.sputnik.review.transformer.FileTransformer; import java.util.ArrayList; import java.util.EnumMap; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -38,7 +38,7 @@ public class Review { * Messages that will be displayed on review summary with your code-review tool */ private List messages = new ArrayList<>(); - private Map scores = new HashMap<>(); + private Score score; private final ReviewFormatter formatter; diff --git a/src/test/java/pl/touk/sputnik/ReviewBuilder.java b/src/test/java/pl/touk/sputnik/ReviewBuilder.java index 64bef8e0..a29dd63e 100644 --- a/src/test/java/pl/touk/sputnik/ReviewBuilder.java +++ b/src/test/java/pl/touk/sputnik/ReviewBuilder.java @@ -1,6 +1,7 @@ package pl.touk.sputnik; import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.engine.score.Score; import pl.touk.sputnik.review.Comment; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; @@ -16,7 +17,7 @@ public static Review buildReview(Configuration configuration) { 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); + review.setScore(new Score("Code-Review", 1)); return review; } diff --git a/src/test/java/pl/touk/sputnik/engine/VisitorBuilderTest.java b/src/test/java/pl/touk/sputnik/engine/VisitorBuilderTest.java index d374520c..99bb4cf5 100644 --- a/src/test/java/pl/touk/sputnik/engine/VisitorBuilderTest.java +++ b/src/test/java/pl/touk/sputnik/engine/VisitorBuilderTest.java @@ -6,21 +6,14 @@ import pl.touk.sputnik.configuration.Configuration; import pl.touk.sputnik.configuration.ConfigurationSetup; import pl.touk.sputnik.configuration.GeneralOption; -import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; import pl.touk.sputnik.engine.visitor.FilterOutTestFilesVisitor; import pl.touk.sputnik.engine.visitor.LimitCommentVisitor; import pl.touk.sputnik.engine.visitor.RegexFilterFilesVisitor; import pl.touk.sputnik.engine.visitor.SummaryMessageVisitor; -import pl.touk.sputnik.engine.visitor.score.NoScore; -import pl.touk.sputnik.engine.visitor.score.ScoreAlwaysPass; -import pl.touk.sputnik.engine.visitor.score.ScorePassIfEmpty; -import pl.touk.sputnik.engine.visitor.score.ScorePassIfNoErrors; import java.util.Collections; -import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.data.MapEntry.entry; class VisitorBuilderTest { @@ -69,9 +62,9 @@ void shouldBuildAfterVisitors() { Configuration config = new ConfigurationSetup().setUp(Collections.emptyMap()); assertThat(new VisitorBuilder().buildAfterReviewVisitors(config)) - .hasSize(2) + .hasSize(1) .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScoreAlwaysPass.class); + .containsExactly(SummaryMessageVisitor.class); } @Test @@ -81,9 +74,9 @@ void shouldNotBuildDisabledAfterVisitors() { )); assertThat(new VisitorBuilder().buildAfterReviewVisitors(config)) - .hasSize(2) + .hasSize(1) .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScoreAlwaysPass.class); + .containsExactly(SummaryMessageVisitor.class); } @Test @@ -93,93 +86,9 @@ void shouldBuildFilterOutCommentVisitor() { )); assertThat(new VisitorBuilder().buildAfterReviewVisitors(config)) - .hasSize(3) - .extracting("class") - .containsExactly(SummaryMessageVisitor.class, LimitCommentVisitor.class, ScoreAlwaysPass.class); - } - - @Test - void shouldBuildNoScoreVisitor() { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( - GeneralOption.SCORE_STRATEGY.getKey(), "NOscore" - )); - - assertThat(new VisitorBuilder().buildAfterReviewVisitors(config)) - .hasSize(2) - .extracting("class") - .containsExactly(SummaryMessageVisitor.class, NoScore.class); - } - - @Test - void shouldBuildScoreAlwaysPassVisitor() { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( - GeneralOption.SCORE_STRATEGY.getKey(), "scoreAlwaysPass", - GeneralOption.SCORE_PASSING_KEY.getKey(), "Verified", - GeneralOption.SCORE_PASSING_VALUE.getKey(), "2" - )); - - List afterReviewVisitors = new VisitorBuilder().buildAfterReviewVisitors(config); - - assertThat(afterReviewVisitors) - .hasSize(2) - .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScoreAlwaysPass.class); - assertThat(((ScoreAlwaysPass) afterReviewVisitors.get(1)).getPassingScore()).containsOnly(entry("Verified", (short) 2)); - } - - @Test - void shouldBuildScorePassIfEmptyVisitor() { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( - GeneralOption.SCORE_STRATEGY.getKey(), "SCOREPASSIFEMPTY", - GeneralOption.SCORE_PASSING_KEY.getKey(), "Verified", - GeneralOption.SCORE_PASSING_VALUE.getKey(), "3", - GeneralOption.SCORE_FAILING_KEY.getKey(), "Verified", - GeneralOption.SCORE_FAILING_VALUE.getKey(), "-3" - )); - - List afterReviewVisitors = new VisitorBuilder().buildAfterReviewVisitors(config); - - assertThat(afterReviewVisitors) - .hasSize(2) - .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScorePassIfEmpty.class); - assertThat(((ScorePassIfEmpty) afterReviewVisitors.get(1)).getPassingScore()).containsOnly(entry("Verified", (short) 3)); - assertThat(((ScorePassIfEmpty) afterReviewVisitors.get(1)).getFailingScore()).containsOnly(entry("Verified", (short) -3)); - } - - @Test - void shouldBuildScorePassIfNoErrorsVisitor() { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( - GeneralOption.SCORE_STRATEGY.getKey(), "SCOREPassIfNoErrors", - GeneralOption.SCORE_PASSING_KEY.getKey(), "Code-Review", - GeneralOption.SCORE_PASSING_VALUE.getKey(), "1", - GeneralOption.SCORE_FAILING_KEY.getKey(), "Code-Review", - GeneralOption.SCORE_FAILING_VALUE.getKey(), "-2" - )); - - List afterReviewVisitors = new VisitorBuilder().buildAfterReviewVisitors(config); - - assertThat(afterReviewVisitors) - .hasSize(2) - .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScorePassIfNoErrors.class); - assertThat(((ScorePassIfNoErrors) afterReviewVisitors.get(1)).getPassingScore()).containsOnly(entry("Code-Review", (short) 1)); - assertThat(((ScorePassIfNoErrors) afterReviewVisitors.get(1)).getFailingScore()).containsOnly(entry("Code-Review", (short) -2)); - } - - @Test - void shouldBuildDefaultScoreAlwaysPassIfStrategyIsUnknown() { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( - GeneralOption.SCORE_STRATEGY.getKey(), "mySimpleStrategy" - )); - - List afterReviewVisitors = new VisitorBuilder().buildAfterReviewVisitors(config); - - assertThat(afterReviewVisitors) .hasSize(2) .extracting("class") - .containsExactly(SummaryMessageVisitor.class, ScoreAlwaysPass.class); - assertThat(((ScoreAlwaysPass) afterReviewVisitors.get(1)).getPassingScore()).containsOnly(entry("Code-Review", (short) 1)); + .containsExactly(SummaryMessageVisitor.class, LimitCommentVisitor.class); } } \ No newline at end of file diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java b/src/test/java/pl/touk/sputnik/engine/score/NoScoreTest.java similarity index 69% rename from src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java rename to src/test/java/pl/touk/sputnik/engine/score/NoScoreTest.java index 5fa7024f..cd3ca285 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/NoScoreTest.java +++ b/src/test/java/pl/touk/sputnik/engine/score/NoScoreTest.java @@ -1,4 +1,4 @@ -package pl.touk.sputnik.engine.visitor.score; +package pl.touk.sputnik.engine.score; import org.junit.jupiter.api.Test; import pl.touk.sputnik.TestEnvironment; @@ -12,8 +12,8 @@ class NoScoreTest extends TestEnvironment { void shouldAddNoScoreToReview() { Review review = review(); - new NoScore().afterReview(review); + Score score = new NoScore().score(review); - assertThat(review.getScores()).isEmpty(); + assertThat(score).isNull(); } } \ No newline at end of file diff --git a/src/test/java/pl/touk/sputnik/engine/score/ScoreAlwaysPassTest.java b/src/test/java/pl/touk/sputnik/engine/score/ScoreAlwaysPassTest.java new file mode 100644 index 00000000..83789f7e --- /dev/null +++ b/src/test/java/pl/touk/sputnik/engine/score/ScoreAlwaysPassTest.java @@ -0,0 +1,21 @@ +package pl.touk.sputnik.engine.score; + +import org.junit.jupiter.api.Test; +import pl.touk.sputnik.TestEnvironment; +import pl.touk.sputnik.review.Review; + +import static org.assertj.core.api.Assertions.assertThat; + +class ScoreAlwaysPassTest extends TestEnvironment { + + @Test + void shouldAddScoreToReview() { + Review review = review(); + + Score myPassingScore = new Score("Sputnik-Pass", 1); + Score myScore = new ScoreAlwaysPass(myPassingScore).score(review); + + assertThat(myScore).isEqualTo(myPassingScore); + } + +} \ No newline at end of file diff --git a/src/test/java/pl/touk/sputnik/engine/score/ScorePassIfEmptyTest.java b/src/test/java/pl/touk/sputnik/engine/score/ScorePassIfEmptyTest.java new file mode 100644 index 00000000..13d366cb --- /dev/null +++ b/src/test/java/pl/touk/sputnik/engine/score/ScorePassIfEmptyTest.java @@ -0,0 +1,35 @@ +package pl.touk.sputnik.engine.score; + +import org.junit.jupiter.api.Test; +import pl.touk.sputnik.review.Review; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class ScorePassIfEmptyTest { + private static final Score PASSING_SCORE = new Score("Sputnik-Pass", 1); + private static final Score FAILING_SCORE = new Score("Code-Review", -2); + + private Review reviewMock = mock(Review.class); + private static final ScorePassIfEmpty SCORE_PASS_IF_EMPTY = new ScorePassIfEmpty(PASSING_SCORE, FAILING_SCORE); + + @Test + void shouldPassIfViolationsIsEmpty() { + when(reviewMock.getTotalViolationCount()).thenReturn(0); + + Score score = SCORE_PASS_IF_EMPTY.score(reviewMock); + + assertThat(score).isEqualTo(PASSING_SCORE); + } + + @Test + void shouldFailIfViolationsIsNotEmpty() { + when(reviewMock.getTotalViolationCount()).thenReturn(1); + + Score score = SCORE_PASS_IF_EMPTY.score(reviewMock); + + assertThat(score).isEqualTo(FAILING_SCORE); + } + +} \ No newline at end of file diff --git a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java b/src/test/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrorsTest.java similarity index 50% rename from src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java rename to src/test/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrorsTest.java index a44f609b..be4c3090 100644 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrorsTest.java +++ b/src/test/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrorsTest.java @@ -1,48 +1,46 @@ -package pl.touk.sputnik.engine.visitor.score; +package pl.touk.sputnik.engine.score; -import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.Severity; -import java.util.Map; - +import static org.assertj.core.api.Assertions.assertThat; 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; 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 static final Score PASSING_SCORE = new Score("Sputnik-Pass", 1); + private static final Score FAILING_SCORE = new Score("Code-Review", -2); + private static final ScorePassIfNoErrors SCORE_PASS_IF_EMPTY = new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE); 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); + Score score = SCORE_PASS_IF_EMPTY.score(reviewMock); - verify(reviewMock).setScores(PASSING_SCORE); + assertThat(score).isEqualTo(PASSING_SCORE); } @Test void shouldPassIfErrorCountIsZero() { when(reviewMock.getViolationCount().get(Severity.ERROR)).thenReturn(0); - new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); + Score score = SCORE_PASS_IF_EMPTY.score(reviewMock); - verify(reviewMock).setScores(PASSING_SCORE); + assertThat(score).isEqualTo(PASSING_SCORE); } @Test void shouldFailIfErrorCountIsNotZero() { when(reviewMock.getViolationCount().get(Severity.ERROR)).thenReturn(1); - new ScorePassIfNoErrors(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); + Score score = SCORE_PASS_IF_EMPTY.score(reviewMock); - verify(reviewMock).setScores(FAILING_SCORE); + assertThat(score).isEqualTo(FAILING_SCORE); } } \ No newline at end of file diff --git a/src/test/java/pl/touk/sputnik/engine/score/ScoreStrategyFactoryTest.java b/src/test/java/pl/touk/sputnik/engine/score/ScoreStrategyFactoryTest.java new file mode 100644 index 00000000..7a2aa439 --- /dev/null +++ b/src/test/java/pl/touk/sputnik/engine/score/ScoreStrategyFactoryTest.java @@ -0,0 +1,71 @@ +package pl.touk.sputnik.engine.score; + +import com.google.common.collect.ImmutableMap; +import org.junit.jupiter.api.Test; +import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.configuration.ConfigurationSetup; +import pl.touk.sputnik.configuration.GeneralOption; + +import static org.assertj.core.api.Assertions.assertThat; + +class ScoreStrategyFactoryTest { + @Test + void shouldBuildNoScoreVisitor() { + Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( + GeneralOption.SCORE_STRATEGY.getKey(), "NOscore" + )); + + assertThat(new ScoreStrategyFactory().buildScoreStrategy(config)) + .isEqualTo(new NoScore()); + } + + @Test + void shouldBuildScoreAlwaysPassVisitor() { + Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( + GeneralOption.SCORE_STRATEGY.getKey(), "scoreAlwaysPass", + GeneralOption.SCORE_PASSING_KEY.getKey(), "Verified", + GeneralOption.SCORE_PASSING_VALUE.getKey(), "2" + )); + + assertThat(new ScoreStrategyFactory().buildScoreStrategy(config)) + .isEqualTo(new ScoreAlwaysPass(new Score("Verified", 2))); + } + + @Test + void shouldBuildScorePassIfEmptyVisitor() { + Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( + GeneralOption.SCORE_STRATEGY.getKey(), "SCOREPASSIFEMPTY", + GeneralOption.SCORE_PASSING_KEY.getKey(), "Verified", + GeneralOption.SCORE_PASSING_VALUE.getKey(), "3", + GeneralOption.SCORE_FAILING_KEY.getKey(), "Verified", + GeneralOption.SCORE_FAILING_VALUE.getKey(), "-3" + )); + + assertThat(new ScoreStrategyFactory().buildScoreStrategy(config)) + .isEqualTo(new ScorePassIfEmpty(new Score("Verified", 3), new Score("Verified", -3))); + } + + @Test + void shouldBuildScorePassIfNoErrorsVisitor() { + Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( + GeneralOption.SCORE_STRATEGY.getKey(), "SCOREPassIfNoErrors", + GeneralOption.SCORE_PASSING_KEY.getKey(), "Code-Review", + GeneralOption.SCORE_PASSING_VALUE.getKey(), "1", + GeneralOption.SCORE_FAILING_KEY.getKey(), "Code-Review", + GeneralOption.SCORE_FAILING_VALUE.getKey(), "-2" + )); + + assertThat(new ScoreStrategyFactory().buildScoreStrategy(config)) + .isEqualTo(new ScorePassIfNoErrors(new Score("Code-Review", 1), new Score("Code-Review", -2))); + } + + @Test + void shouldBuildDefaultScoreAlwaysPassIfStrategyIsUnknown() { + Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of( + GeneralOption.SCORE_STRATEGY.getKey(), "mySimpleStrategy" + )); + + assertThat(new ScoreStrategyFactory().buildScoreStrategy(config)) + .isEqualTo(new ScoreAlwaysPass(new Score("Code-Review", 1))); + } +} \ No newline at end of file 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 deleted file mode 100644 index 30d694df..00000000 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScoreAlwaysPassTest.java +++ /dev/null @@ -1,22 +0,0 @@ -package pl.touk.sputnik.engine.visitor.score; - -import com.google.common.collect.ImmutableMap; -import org.junit.jupiter.api.Test; -import pl.touk.sputnik.TestEnvironment; -import pl.touk.sputnik.review.Review; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.entry; - -class ScoreAlwaysPassTest extends TestEnvironment { - - @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)); - } - -} \ No newline at end of file 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 deleted file mode 100644 index da3f65d5..00000000 --- a/src/test/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmptyTest.java +++ /dev/null @@ -1,37 +0,0 @@ -package pl.touk.sputnik.engine.visitor.score; - -import com.google.common.collect.ImmutableMap; -import org.junit.jupiter.api.Test; -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; - -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); - - @Test - void shouldPassIfViolationsIsEmpty() { - when(reviewMock.getTotalViolationCount()).thenReturn(0); - - new ScorePassIfEmpty(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); - - verify(reviewMock).setScores(PASSING_SCORE); - } - - @Test - void shouldFailIfViolationsIsNotEmpty() { - when(reviewMock.getTotalViolationCount()).thenReturn(1); - - new ScorePassIfEmpty(PASSING_SCORE, FAILING_SCORE).afterReview(reviewMock); - - verify(reviewMock).setScores(FAILING_SCORE); - } - -} \ No newline at end of file