diff --git a/src/main/java/pl/touk/sputnik/Main.java b/src/main/java/pl/touk/sputnik/Main.java index 9f6db727..7127488b 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 == Score.FAIL) { + System.exit(1); + } } private static ConnectorFacade getConnectorFacade(Configuration configuration) { diff --git a/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacade.java b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacade.java index 5dc95bac..c7714314 100644 --- a/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacade.java +++ b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacade.java @@ -26,7 +26,7 @@ public class GerritFacade implements ConnectorFacade, ConnectorValidator, Review private final GerritApi gerritApi; private final GerritPatchset gerritPatchset; - private final CommentFilter commentFilter; + private final ReviewInputBuilder reviewInputBuilder; @NotNull @Override @@ -62,7 +62,7 @@ private boolean isDeleted(FileInfo fileInfo) { public void publish(@NotNull Review review) { try { log.debug("Set review in Gerrit: {}", review); - ReviewInput reviewInput = new ReviewInputBuilder(commentFilter).toReviewInput(review, gerritPatchset.getTag()); + ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review, gerritPatchset.getTag()); gerritApi.changes() .id(gerritPatchset.getChangeId()) .revision(gerritPatchset.getRevisionId()) diff --git a/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacadeBuilder.java b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacadeBuilder.java index da2cda5a..a2c1505c 100644 --- a/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacadeBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritFacadeBuilder.java @@ -47,7 +47,18 @@ public HttpClientBuilder extend(HttpClientBuilder httpClientBuilder, GerritAuthD CommentFilter commentFilter = buildCommentFilter(configuration, gerritPatchset, gerritApi); - return new GerritFacade(gerritApi, gerritPatchset, commentFilter); + GerritScoreLabeler scoreLabeler = scoreLabeler(configuration); + ReviewInputBuilder reviewInputBuilder = new ReviewInputBuilder(commentFilter, scoreLabeler); + return new GerritFacade(gerritApi, gerritPatchset, reviewInputBuilder); + } + + @NotNull + private GerritScoreLabeler scoreLabeler(Configuration configuration) { + String scorePassingKey = configuration.getProperty(GeneralOption.SCORE_PASSING_KEY); + String scoreFailingKey = configuration.getProperty(GeneralOption.SCORE_FAILING_KEY); + short scorePassingValue = Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE)); + short scoreFailingValue = Short.valueOf(configuration.getProperty(GeneralOption.SCORE_FAILING_VALUE)); + return new GerritScoreLabeler(scorePassingKey, scoreFailingKey, scorePassingValue, scoreFailingValue); } @NotNull diff --git a/src/main/java/pl/touk/sputnik/connector/gerrit/GerritScoreLabeler.java b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritScoreLabeler.java new file mode 100644 index 00000000..d0f49c8a --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/gerrit/GerritScoreLabeler.java @@ -0,0 +1,28 @@ +package pl.touk.sputnik.connector.gerrit; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import pl.touk.sputnik.engine.score.Score; + +import java.util.Collections; +import java.util.Map; + +@AllArgsConstructor +@Getter +class GerritScoreLabeler { + private final String scorePassingKey; + private final String scoreFailingKey; + private final short scorePassingValue; + private final short scoreFailingValue; + + Map getReviewLabel(Score score) { + switch (score) { + case PASS: + return Collections.singletonMap(scorePassingKey, scorePassingValue); + case FAIL: + return Collections.singletonMap(scoreFailingKey, scoreFailingValue); + default: + return Collections.emptyMap(); + } + } +} 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 5cd5cedf..41da7212 100644 --- a/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java @@ -11,8 +11,8 @@ import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; -import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; @AllArgsConstructor @@ -22,12 +22,14 @@ public class ReviewInputBuilder { private static final String MESSAGE_SEPARATOR = ". "; private final CommentFilter commentFilter; + private final GerritScoreLabeler scoreLabeler; @NotNull public ReviewInput toReviewInput(@NotNull Review review, @Nullable String tag) { ReviewInput reviewInput = new ReviewInput(); reviewInput.message = Joiner.on(MESSAGE_SEPARATOR).join(review.getMessages()); - reviewInput.labels = new HashMap<>(review.getScores()); + Map reviewLabel = scoreLabeler.getReviewLabel(review.getScore()); + reviewInput.labels = reviewLabel; if (StringUtils.isNotBlank(tag)) { reviewInput.tag = tag; } diff --git a/src/main/java/pl/touk/sputnik/engine/Engine.java b/src/main/java/pl/touk/sputnik/engine/Engine.java index 5b3d0715..59ddcbcd 100644 --- a/src/main/java/pl/touk/sputnik/engine/Engine.java +++ b/src/main/java/pl/touk/sputnik/engine/Engine.java @@ -1,18 +1,25 @@ package pl.touk.sputnik.engine; import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.configuration.GeneralOption; import pl.touk.sputnik.connector.ConnectorFacade; import pl.touk.sputnik.connector.ReviewPublisher; +import pl.touk.sputnik.engine.score.Score; +import pl.touk.sputnik.engine.score.ScoreStrategy; import pl.touk.sputnik.engine.visitor.AfterReviewVisitor; import pl.touk.sputnik.engine.visitor.BeforeReviewVisitor; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; import pl.touk.sputnik.review.ReviewFormatterFactory; import pl.touk.sputnik.review.ReviewProcessor; +import pl.touk.sputnik.review.Severity; import java.util.List; +import static org.apache.commons.lang3.Validate.notBlank; + @Slf4j public class Engine { @@ -26,8 +33,9 @@ public Engine(ConnectorFacade facade, ReviewPublisher reviewPublisher, Configura this.config = configuration; } - public void run() { + public Score run() { List reviewFiles = facade.listFiles(); + log.debug(reviewFiles.toString()); Review review = new Review(reviewFiles, ReviewFormatterFactory.get(config)); for (BeforeReviewVisitor beforeReviewVisitor : new VisitorBuilder().buildBeforeReviewVisitors(config)) { @@ -44,6 +52,28 @@ public void run() { afterReviewVisitor.afterReview(review); } + String scoreStrategyName = scoreStrategyName(); + if (!ScoreStrategy.isValidScoreStrategy(scoreStrategyName)) { + log.warn("Score strategy {} not found, using default ScoreAlwaysPass", scoreStrategyName); + } + + Score score = ScoreStrategy.of(scoreStrategyName).score(review); + review.setScore(score); + log.info("{} violations found, {} errors. Adding score {}", + review.getTotalViolationCount(), + review.getViolationCount().get(Severity.ERROR), + score); + reviewPublisher.publish(review); + + return score; + } + + @NotNull + private String scoreStrategyName() { + String scoreStrategyName = config.getProperty(GeneralOption.SCORE_STRATEGY); + notBlank(scoreStrategyName); + scoreStrategyName = scoreStrategyName.toUpperCase(); + return scoreStrategyName; } } 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/Score.java b/src/main/java/pl/touk/sputnik/engine/score/Score.java new file mode 100644 index 00000000..88101e11 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/Score.java @@ -0,0 +1,9 @@ +package pl.touk.sputnik.engine.score; + +public enum Score { + PASS, + FAIL, + NONE + + +} diff --git a/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategies.java b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategies.java new file mode 100644 index 00000000..7fcc13bc --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategies.java @@ -0,0 +1,46 @@ +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 ScoreStrategies { + 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) { + String scoreStrategy = configuration.getProperty(GeneralOption.SCORE_STRATEGY); + notBlank(scoreStrategy); + + String myS = scoreStrategy.toUpperCase(); + return getScoreStrategy(scoreStrategy, myS); + } + + @NotNull + private ScoreStrategy getScoreStrategy(String aScoreStrategy, String aS) { + switch(aS) { + case NOSCORE: + return ScoreStrategy.NO_SCORE; + + case SCOREALWAYSPASS: + return ScoreStrategy.ALWAYS_PASS; + + case SCOREPASSIFEMPTY: + return ScoreStrategy.PASS_IF_EMPTY; + + case SCOREPASSIFNOERRORS: + return ScoreStrategy.PASS_IF_NO_ERRORS; + + default: + log.warn("Score strategy {} not found, using default ScoreAlwaysPass", aScoreStrategy); + return ScoreStrategy.ALWAYS_PASS; + } + } +} 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..2fc8cdd7 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java @@ -0,0 +1,82 @@ +package pl.touk.sputnik.engine.score; + +import com.google.common.collect.ImmutableSet; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.review.Review; +import pl.touk.sputnik.review.Severity; + +import java.util.Set; + +public enum ScoreStrategy { + NO_SCORE { + @Override + public Score score(@NotNull Review review) { + return Score.NONE; + } + }, + + ALWAYS_PASS { + @Override + public Score score(@NotNull Review review) { + return Score.PASS; + } + }, + + PASS_IF_EMPTY { + @Override + public Score score(@NotNull Review review) { + if (review.getTotalViolationCount() == 0) { + return Score.PASS; + } + + return Score.FAIL; + } + }, + + PASS_IF_NO_ERRORS { + @Override + public Score score(@NotNull Review review) { + Integer errorCount = review.getViolationCount().get(Severity.ERROR); + if (errorCount == null || errorCount == 0) { + return Score.PASS; + } + + return Score.FAIL; + } + }; + + + private static final String NOSCORE = "NOSCORE"; + private static final String SCOREALWAYSPASS = "SCOREALWAYSPASS"; + private static final String SCOREPASSIFEMPTY = "SCOREPASSIFEMPTY"; + private static final String SCOREPASSIFNOERRORS = "SCOREPASSIFNOERRORS"; + + private static final Set SCORE_STRATEGY_KEYS = ImmutableSet.of( + NOSCORE, SCOREALWAYSPASS, SCOREPASSIFEMPTY, SCOREPASSIFNOERRORS); + + public static boolean isValidScoreStrategy(String strategy) { + return SCORE_STRATEGY_KEYS.contains(strategy); + } + + @NotNull + public static ScoreStrategy of(String strategy) { + switch(strategy) { + case NOSCORE: + return ScoreStrategy.NO_SCORE; + + case SCOREALWAYSPASS: + return ScoreStrategy.ALWAYS_PASS; + + case SCOREPASSIFEMPTY: + return ScoreStrategy.PASS_IF_EMPTY; + + case SCOREPASSIFNOERRORS: + return ScoreStrategy.PASS_IF_NO_ERRORS; + + default: + return ScoreStrategy.ALWAYS_PASS; + } + } + + abstract public Score score(@NotNull Review review); +} 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/engine/visitor/score/ScorePassIfEmpty.java b/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java deleted file mode 100644 index 444e4979..00000000 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java +++ /dev/null @@ -1,30 +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 ScorePassIfEmpty implements AfterReviewVisitor { - private final Map passingScore; - private final Map failingScore; - - @Override - public void afterReview(@NotNull Review review) { - if (review.getTotalViolationCount() == 0) { - log.info("Adding passing score {} for no violation(s) found", passingScore); - review.setScores(passingScore); - return; - } - - log.info("Adding failing score {} for {} violations found", failingScore, review.getTotalViolationCount()); - review.setScores(failingScore); - } -} 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 deleted file mode 100644 index 362bafd7..00000000 --- a/src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java +++ /dev/null @@ -1,32 +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 pl.touk.sputnik.review.Severity; - -import java.util.Map; - -@Slf4j -@Getter -@AllArgsConstructor -public class ScorePassIfNoErrors implements AfterReviewVisitor { - private final Map passingScore; - private final Map failingScore; - - @Override - public void afterReview(@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; - } - - log.info("Adding failing score {} for {} errors found", failingScore, errorCount); - review.setScores(failingScore); - } -} 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/connector/gerrit/ReviewInputBuilderTest.java b/src/test/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilderTest.java index f1fad3f7..f4c442c7 100644 --- a/src/test/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilderTest.java +++ b/src/test/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilderTest.java @@ -39,7 +39,7 @@ void shouldBuildReviewInput() { Review review = ReviewBuilder.buildReview(config); when(commentFilter.include(eq("filename1"), anyInt())).thenReturn(true); - ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review, TAG); + ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review); assertThat(reviewInput.message).isEqualTo("Total 8 violations found"); assertThat(reviewInput.comments).hasSize(4); @@ -56,7 +56,7 @@ void shouldBuildReviewInputWithCommentFilter() { when(commentFilter.include("filename1", 0)).thenReturn(false); when(commentFilter.include("filename1", 1)).thenReturn(true); - ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review, TAG); + ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review); assertThat(reviewInput.message).isEqualTo("Total 8 violations found"); assertThat(reviewInput.comments).hasSize(4); @@ -72,7 +72,7 @@ void shouldNotSetEmptyOrNullTag(String tag) { Configuration config = ConfigurationBuilder.initFromResource("test.properties"); Review review = ReviewBuilder.buildReview(config); - ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review, tag); + ReviewInput reviewInput = reviewInputBuilder.toReviewInput(review); assertThat(reviewInput.tag).isNull(); } 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 58% 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..a3263fd2 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,13 @@ 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(); + } + + @Test + public void athateh() { + System.out.println(new Score("1", 1)); } } \ 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..a3eb848b --- /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 ScoreStrategies().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 ScoreStrategies().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 ScoreStrategies().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 ScoreStrategies().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 ScoreStrategies().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