Skip to content

Commit

Permalink
Exit sputnik with error code if checks fail
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Marquis Wong authored and Marquis Wong committed Nov 11, 2019
1 parent e1344e1 commit 0355151
Show file tree
Hide file tree
Showing 25 changed files with 377 additions and 330 deletions.
6 changes: 5 additions & 1 deletion src/main/java/pl/touk/sputnik/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, Short> getReviewLabel(Score score) {
switch (score) {
case PASS:
return Collections.singletonMap(scorePassingKey, scorePassingValue);
case FAIL:
return Collections.singletonMap(scoreFailingKey, scoreFailingValue);
default:
return Collections.emptyMap();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<String, Short> reviewLabel = scoreLabeler.getReviewLabel(review.getScore());
reviewInput.labels = reviewLabel;
if (StringUtils.isNotBlank(tag)) {
reviewInput.tag = tag;
}
Expand Down
32 changes: 31 additions & 1 deletion src/main/java/pl/touk/sputnik/engine/Engine.java
Original file line number Diff line number Diff line change
@@ -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 {

Expand All @@ -26,8 +33,9 @@ public Engine(ConnectorFacade facade, ReviewPublisher reviewPublisher, Configura
this.config = configuration;
}

public void run() {
public Score run() {
List<ReviewFile> reviewFiles = facade.listFiles();
log.debug(reviewFiles.toString());
Review review = new Review(reviewFiles, ReviewFormatterFactory.get(config));

for (BeforeReviewVisitor beforeReviewVisitor : new VisitorBuilder().buildBeforeReviewVisitors(config)) {
Expand All @@ -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;
}
}
46 changes: 0 additions & 46 deletions src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<BeforeReviewVisitor> buildBeforeReviewVisitors(Configuration configuration) {
List<BeforeReviewVisitor> beforeReviewVisitors = new ArrayList<>();
Expand Down Expand Up @@ -65,40 +52,7 @@ public List<AfterReviewVisitor> buildAfterReviewVisitors(Configuration configura
afterReviewVisitors.add(new LimitCommentVisitor(maxNumberOfComments));
}

afterReviewVisitors.add(buildScoreAfterReviewVisitor(configuration));

return afterReviewVisitors;
}

@NotNull
private AfterReviewVisitor buildScoreAfterReviewVisitor(Configuration configuration) {
Map<String, Short> passingScore = ImmutableMap.<String, Short>of(
configuration.getProperty(GeneralOption.SCORE_PASSING_KEY),
Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE))
);
Map<String, Short> failingScore = ImmutableMap.<String, Short>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);
}
}
}
9 changes: 9 additions & 0 deletions src/main/java/pl/touk/sputnik/engine/score/Score.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pl.touk.sputnik.engine.score;

public enum Score {
PASS,
FAIL,
NONE


}
46 changes: 46 additions & 0 deletions src/main/java/pl/touk/sputnik/engine/score/ScoreStrategies.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
82 changes: 82 additions & 0 deletions src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java
Original file line number Diff line number Diff line change
@@ -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<String> 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);
}
Loading

0 comments on commit 0355151

Please sign in to comment.