diff --git a/src/main/java/edu/hm/hafner/grading/AnalysisConfiguration.java b/src/main/java/edu/hm/hafner/grading/AnalysisConfiguration.java index ffd915b4..5ba94290 100644 --- a/src/main/java/edu/hm/hafner/grading/AnalysisConfiguration.java +++ b/src/main/java/edu/hm/hafner/grading/AnalysisConfiguration.java @@ -18,6 +18,7 @@ public final class AnalysisConfiguration extends Configuration { @Serial private static final long serialVersionUID = 3L; + private static final String ANALYSIS_ID = "analysis"; /** @@ -57,6 +58,12 @@ public boolean isPositive() { return errorImpact >= 0 && highImpact >= 0 && normalImpact >= 0 && lowImpact >= 0; } + @Override + @JsonIgnore + public boolean hasImpact() { + return errorImpact != 0 || highImpact != 0 || normalImpact != 0 || lowImpact != 0; + } + public int getErrorImpact() { return errorImpact; } diff --git a/src/main/java/edu/hm/hafner/grading/Configuration.java b/src/main/java/edu/hm/hafner/grading/Configuration.java index 923f819a..7cab97b0 100644 --- a/src/main/java/edu/hm/hafner/grading/Configuration.java +++ b/src/main/java/edu/hm/hafner/grading/Configuration.java @@ -23,7 +23,6 @@ * * @author Ullrich Hafner */ -// TODO: make sure that the configuration is valid public abstract class Configuration implements Serializable { @Serial private static final long serialVersionUID = 3L; @@ -35,11 +34,8 @@ static List extractConfigurations( var configurations = jackson.readJson(json); if (configurations.has(id)) { var deserialized = deserialize(id, type, configurations, jackson); - if (deserialized.isEmpty()) { - throw new IllegalArgumentException("Configuration ID '" + id + "' is empty in JSON: " + json); - } deserialized.forEach(Configuration::validateDefaults); -// deserialized.forEach(Configuration::validate); + deserialized.forEach(Configuration::validate); return deserialized; } return Collections.emptyList(); @@ -119,16 +115,37 @@ public List getTools() { private void validateDefaults() { if (tools.isEmpty()) { - throw new IllegalArgumentException("Configuration ID '" + getId() + "' has no tools"); + throwIllegalArgumentException("Configuration ID '" + getId() + "' has no tools"); } + if (getMaxScore() == 0 && hasImpact()) { + throwIllegalArgumentException("When configuring impacts then the score must not be zero."); + } + if (getMaxScore() > 0 && !hasImpact()) { + throwIllegalArgumentException( + "When configuring a max score than an impact must be defined as well."); + } + } + + private void throwIllegalArgumentException(final String errorMessage) { + throw new IllegalArgumentException(errorMessage + "\nConfiguration: " + this); } -// /** -// * Validates this configuration. -// * -// * @throws IllegalArgumentException if this configuration is invalid -// */ -// protected abstract void validate(); + /** + * Returns whether the specified configuration has impact properties defined, or not. + * + * @return {@code true} if the configuration has impact properties, {@code false} if not + */ + protected abstract boolean hasImpact(); + + /** + * Validates this configuration. This default implementation does nothing. Overwrite this method in subclasses to + * add specific validation logic. + * + * @throws IllegalArgumentException if this configuration is invalid + */ + protected void validate() { + // empty default implementation + } @Override @Generated diff --git a/src/main/java/edu/hm/hafner/grading/CoverageConfiguration.java b/src/main/java/edu/hm/hafner/grading/CoverageConfiguration.java index 8c1d9a6d..657d5f3e 100644 --- a/src/main/java/edu/hm/hafner/grading/CoverageConfiguration.java +++ b/src/main/java/edu/hm/hafner/grading/CoverageConfiguration.java @@ -20,14 +20,16 @@ public final class CoverageConfiguration extends Configuration { @Serial private static final long serialVersionUID = 3L; + private static final String COVERAGE_ID = "coverage"; private static final String[] MUTATION_IDS = {"pitest", "mutation", "pit"}; + static final String CODE_COVERAGE = "Code Coverage"; /** * Converts the specified JSON object to a list of {@link CoverageConfiguration} instances. * * @param json - * the json object to convert + * the JSON object to convert * * @return the corresponding {@link CoverageConfiguration} instances */ @@ -35,10 +37,10 @@ public static List from(final String json) { return extractConfigurations(json, COVERAGE_ID, CoverageConfiguration.class); } - @SuppressWarnings("unused") // Json property + @SuppressWarnings("unused") // JSON property private int coveredPercentageImpact; - @SuppressWarnings("unused") // Json property + @SuppressWarnings("unused") // JSON property private int missedPercentageImpact; private CoverageConfiguration() { @@ -52,10 +54,7 @@ protected String getDefaultId() { @Override protected String getDefaultName() { - if (StringUtils.containsAnyIgnoreCase(getId(), MUTATION_IDS)) { - return "Mutation Coverage"; - } - return "Code Coverage"; + return CODE_COVERAGE; } @Override @@ -64,6 +63,12 @@ public boolean isPositive() { return coveredPercentageImpact >= 0 && missedPercentageImpact >= 0; } + @Override + @JsonIgnore + public boolean hasImpact() { + return coveredPercentageImpact != 0 || missedPercentageImpact != 0; + } + public int getCoveredPercentageImpact() { return coveredPercentageImpact; } diff --git a/src/main/java/edu/hm/hafner/grading/TestConfiguration.java b/src/main/java/edu/hm/hafner/grading/TestConfiguration.java index 96bd2d4b..2b578015 100644 --- a/src/main/java/edu/hm/hafner/grading/TestConfiguration.java +++ b/src/main/java/edu/hm/hafner/grading/TestConfiguration.java @@ -4,6 +4,8 @@ import java.util.List; import java.util.Objects; +import com.fasterxml.jackson.annotation.JsonIgnore; + import edu.hm.hafner.util.Generated; /** @@ -15,13 +17,14 @@ public final class TestConfiguration extends Configuration { @Serial private static final long serialVersionUID = 3L; + private static final String TEST_ID = "tests"; /** * Converts the specified JSON object to a list of {@link TestConfiguration} instances. * * @param json - * the json object to convert + * the JSON object to convert * * @return the corresponding {@link TestConfiguration} instances */ @@ -33,6 +36,9 @@ public static List from(final String json) { private int passedImpact; private int skippedImpact; + private int successRateImpact; + private int failureRateImpact; + private TestConfiguration() { super(); // Instances are created via JSON deserialization } @@ -47,9 +53,36 @@ protected String getDefaultName() { return "Tests"; } + /** + * Returns whether this configuration defines relative impacts. + * + * @return {@code true} if this configuration defines relative impacts, {@code false} if it defines absolute impacts + */ + @JsonIgnore + public boolean isRelative() { + return getFailureRateImpact() != 0 || getSuccessRateImpact() != 0; + } + + /** + * Returns whether this configuration defines absolute impacts. + * + * @return {@code true} if this configuration defines absolute impacts, {@code false} if it defines relative impacts + */ + @JsonIgnore + public boolean isAbsolute() { + return getPassedImpact() != 0 || getFailureImpact() != 0 || getSkippedImpact() != 0; + } + @Override + @JsonIgnore public boolean isPositive() { - return passedImpact >= 0 && failureImpact >= 0 && skippedImpact >= 0; + return getPassedImpact() >= 0 && getFailureImpact() >= 0 && getSkippedImpact() >= 0; + } + + @Override + @JsonIgnore + protected boolean hasImpact() { + return isRelative() || isAbsolute(); } public int getSkippedImpact() { @@ -64,6 +97,22 @@ public int getPassedImpact() { return passedImpact; } + public int getSuccessRateImpact() { + return successRateImpact; + } + + public int getFailureRateImpact() { + return failureRateImpact; + } + + @Override + protected void validate() { + if (isRelative() && isAbsolute()) { + throw new IllegalArgumentException( + "Test configuration must either define an impact for absolute or relative metrics only."); + } + } + @Override @Generated public boolean equals(final Object o) { @@ -79,12 +128,15 @@ public boolean equals(final Object o) { var that = (TestConfiguration) o; return failureImpact == that.failureImpact && passedImpact == that.passedImpact - && skippedImpact == that.skippedImpact; + && skippedImpact == that.skippedImpact + && successRateImpact == that.successRateImpact + && failureRateImpact == that.failureRateImpact; } @Override @Generated public int hashCode() { - return Objects.hash(super.hashCode(), failureImpact, passedImpact, skippedImpact); + return Objects.hash(super.hashCode(), failureImpact, passedImpact, skippedImpact, successRateImpact, + failureRateImpact); } } diff --git a/src/main/java/edu/hm/hafner/grading/TestScore.java b/src/main/java/edu/hm/hafner/grading/TestScore.java index c6945240..9a3d8cba 100644 --- a/src/main/java/edu/hm/hafner/grading/TestScore.java +++ b/src/main/java/edu/hm/hafner/grading/TestScore.java @@ -37,6 +37,7 @@ public final class TestScore extends Score { private final int passedSize; private final int failedSize; private final int skippedSize; + private transient Node report; // do not persist the tree of nodes private TestScore(final String id, final String name, final TestConfiguration configuration, @@ -100,13 +101,39 @@ public int getImpact() { int change = 0; - change = change + configuration.getPassedImpact() * getPassedSize(); - change = change + configuration.getFailureImpact() * getFailedSize(); - change = change + configuration.getSkippedImpact() * getSkippedSize(); - + if (configuration.isAbsolute()) { + change = change + configuration.getPassedImpact() * getPassedSize(); + change = change + configuration.getFailureImpact() * getFailedSize(); + change = change + configuration.getSkippedImpact() * getSkippedSize(); + } + else { + change = change + configuration.getSuccessRateImpact() * getSuccessRate(); + change = change + configuration.getFailureRateImpact() * getFailureRate(); + } return change; } + /** + * Returns the success rate of the tests. + * + * @return the success rate, i.e., the number of passed tests in percent with respect to the total number of tests + */ + public int getSuccessRate() { + var rate = getRateOf(getPassedSize()); + if (rate == 100 && getFailedSize() > 0) { + return 99; // 100% success rate is only possible if there are no failed tests + } + return rate; + } + + public int getFailureRate() { + return getRateOf(getFailedSize()); + } + + private int getRateOf(final int achieved) { + return Math.toIntExact(Math.round(achieved * 100.0 / getTotalSize())); + } + public int getPassedSize() { return passedSize; } diff --git a/src/test/java/edu/hm/hafner/grading/AbstractConfigurationTest.java b/src/test/java/edu/hm/hafner/grading/AbstractConfigurationTest.java new file mode 100644 index 00000000..e412205b --- /dev/null +++ b/src/test/java/edu/hm/hafner/grading/AbstractConfigurationTest.java @@ -0,0 +1,38 @@ +package edu.hm.hafner.grading; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.databind.JsonMappingException; + +import static org.assertj.core.api.Assertions.*; + +abstract class AbstractConfigurationTest { + @Test + void shouldReportInvalidConfigurations() { + assertThatIllegalArgumentException() + .isThrownBy(() -> fromJson(getInvalidJson())) + .withMessageContaining("Can't convert JSON") + .withCauseInstanceOf(JsonParseException.class); + } + + @Test + void shouldReportEmptyConfigurations() { + assertThatIllegalArgumentException() + .isThrownBy(() -> fromJson(""" + { + "tests": false, + "analysis": false, + "coverage": false + } + """)) + .withMessageContaining("Can't convert JSON") + .withCauseInstanceOf(JsonMappingException.class); + } + + protected abstract List fromJson(String json); + + protected abstract String getInvalidJson(); +} diff --git a/src/test/java/edu/hm/hafner/grading/AnalysisConfigurationTest.java b/src/test/java/edu/hm/hafner/grading/AnalysisConfigurationTest.java index c720db80..9283bf17 100644 --- a/src/test/java/edu/hm/hafner/grading/AnalysisConfigurationTest.java +++ b/src/test/java/edu/hm/hafner/grading/AnalysisConfigurationTest.java @@ -1,17 +1,112 @@ package edu.hm.hafner.grading; +import java.util.List; +import java.util.stream.Stream; + import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; import static edu.hm.hafner.grading.assertions.Assertions.*; -class AnalysisConfigurationTest { +class AnalysisConfigurationTest extends AbstractConfigurationTest { + @Override + protected List fromJson(final String json) { + return AnalysisConfiguration.from(json); + } + + @Override + protected String getInvalidJson() { + return """ + { + "analysis": { + "name": "Checkstyle and SpotBugs", + "maxScore": 50, + "errorImpact": 1 + }, + }; + """; + } + + @ParameterizedTest(name = "{index} => Invalid configuration: {2}") + @MethodSource + @DisplayName("should throw exceptions for invalid configurations") + void shouldReportNotConsistentConfiguration(final String json, final String errorMessage, + @SuppressWarnings("unused") final String displayName) { + assertThatIllegalArgumentException() + .isThrownBy(() -> fromJson(json)) + .withMessageContaining(errorMessage) + .withNoCause(); + } + + public static Stream shouldReportNotConsistentConfiguration() { + return Stream.of( + Arguments.of(""" + { + "analysis": [ + { + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle" + } + ], + "errorImpact": 1, + "highImpact": 2, + "normalImpact": 3, + "lowImpact": 4, + "maxScore": 0 + } + ] + } + """, "When configuring impacts then the score must not be zero.", + "an impact requires a positive score"), + Arguments.of(""" + { + "analysis": [ + { + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle" + } + ], + "errorImpact": 0, + "highImpact": 0, + "normalImpact": 0, + "lowImpact": 0, + "maxScore": 100 + } + ] + } + """, "When configuring a max score than an impact must be defined as well", + "a score requires an impact"), + Arguments.of(""" + { + "analysis": [ + { + "errorImpact": 1, + "highImpact": 2, + "normalImpact": 3, + "lowImpact": 4, + "maxScore": 20 + } + ] + } + """, "Configuration ID 'analysis' has no tools", + "empty tools configuration") + ); + } + @Test void shouldConvertObjectConfigurationFromJson() { - var configurations = AnalysisConfiguration.from(""" + var configurations = fromJson(""" { "analysis": { "name": "Checkstyle and SpotBugs", @@ -32,20 +127,17 @@ void shouldConvertObjectConfigurationFromJson() { """); assertThat(configurations).hasSize(1).first().satisfies(configuration -> assertThat(configuration) - .hasErrorImpact(1) - .hasHighImpact(0) - .hasNormalImpact(0) - .hasLowImpact(0) + .hasErrorImpact(1).hasHighImpact(0).hasNormalImpact(0).hasLowImpact(0) .hasMaxScore(50) .hasName("Checkstyle and SpotBugs") - .isPositive() + .isPositive().hasImpact() .hasOnlyTools(new ToolConfiguration("checkstyle", "", "target/checkstyle.xml", StringUtils.EMPTY), new ToolConfiguration("spotbugs", "", "target/spotbugsXml.xml", StringUtils.EMPTY))); } @Test void shouldConvertSingleArrayElementConfigurationFromJson() { - var configurations = AnalysisConfiguration.from(""" + var configurations = fromJson(""" { "analysis": [{ "tools": [ @@ -74,7 +166,7 @@ void shouldConvertSingleArrayElementConfigurationFromJson() { @Test void shouldConvertMultipleElementsConfigurationsFromJson() { - var configurations = AnalysisConfiguration.from(""" + var configurations = fromJson(""" { "analysis": [ { @@ -94,8 +186,7 @@ void shouldConvertMultipleElementsConfigurationsFromJson() { "highImpact": 2, "normalImpact": 3, "lowImpact": 4, - "maxScore": 5, - "positive": true + "maxScore": 5 }, { "tools": [ @@ -126,6 +217,7 @@ private void verifyFirstConfiguration(final AnalysisConfiguration configuration) .hasLowImpact(4) .hasMaxScore(5) .isPositive() + .hasImpact() .hasOnlyTools(new ToolConfiguration("checkstyle", "Checkstyle", "target/checkstyle.xml", StringUtils.EMPTY), new ToolConfiguration("spotbugs", "SpotBugs", "target/spotbugsXml.xml", StringUtils.EMPTY)); @@ -139,21 +231,93 @@ private void verifyLastConfiguration(final AnalysisConfiguration configuration) .hasLowImpact(-14) .hasMaxScore(-15) .isNotPositive() + .hasImpact() .hasOnlyTools(new ToolConfiguration("pmd", "PMD", "target/pmd.xml", StringUtils.EMPTY)); } - @Test - void shouldHandleUndefinedTools() { - assertThatIllegalArgumentException().isThrownBy( - () -> AnalysisConfiguration.from(""" + @ParameterizedTest(name = "{index} => Positive configuration: {1}") + @MethodSource + @DisplayName("should identify positive configurations") + void shouldIdentifyPositiveValues(final String json, @SuppressWarnings("unused") final String displayName) { + var configurations = fromJson(json); + + assertThat(configurations).hasSize(1).first().satisfies(configuration -> + assertThat(configuration).isNotPositive()); + } + + public static Stream shouldIdentifyPositiveValues() { + return Stream.of(Arguments.of(""" { - "analysis": { - "name": "Checkstyle and SpotBugs", - "maxScore": 50, - "errorImpact": 1 - } + "analysis": [{ + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle", + "pattern": "target/checkstyle.xml" + } + ], + "errorImpact": -1, + "highImpact": 0, + "normalImpact": 0, + "lowImpact": 0, + "maxScore": 10 + }] + } + """, "error impact is negative"), + Arguments.of(""" + { + "analysis": [{ + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle", + "pattern": "target/checkstyle.xml" + } + ], + "errorImpact": 0, + "highImpact": -1, + "normalImpact": 0, + "lowImpact": 0, + "maxScore": 10 + }] + } + """, "high impact is negative"), + Arguments.of(""" + { + "analysis": [{ + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle", + "pattern": "target/checkstyle.xml" + } + ], + "errorImpact": 0, + "highImpact": 0, + "normalImpact": -1, + "lowImpact": 0, + "maxScore": 10 + }] + } + """, "normal impact is negative"), + Arguments.of(""" + { + "analysis": [{ + "tools": [ + { + "id": "checkstyle", + "name": "Checkstyle", + "pattern": "target/checkstyle.xml" + } + ], + "errorImpact": 1, + "highImpact": 0, + "normalImpact": 0, + "lowImpact": -1, + "maxScore": 10 + }] } - """)).withMessage("Configuration ID 'analysis' has no tools"); + """, "low impact is negative")); } @Test diff --git a/src/test/java/edu/hm/hafner/grading/CoverageConfigurationTest.java b/src/test/java/edu/hm/hafner/grading/CoverageConfigurationTest.java index 65b26f0d..c4d7339b 100644 --- a/src/test/java/edu/hm/hafner/grading/CoverageConfigurationTest.java +++ b/src/test/java/edu/hm/hafner/grading/CoverageConfigurationTest.java @@ -1,7 +1,14 @@ package edu.hm.hafner.grading; +import java.util.List; +import java.util.stream.Stream; + import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import edu.hm.hafner.coverage.Metric; @@ -10,10 +17,91 @@ import static edu.hm.hafner.grading.assertions.Assertions.*; -class CoverageConfigurationTest { +class CoverageConfigurationTest extends AbstractConfigurationTest { + @Override + protected List fromJson(final String json) { + return CoverageConfiguration.from(json); + } + + @Override + protected String getInvalidJson() { + return """ + { + "coverage": { + "name": "Coverage", + "maxScore": 50, + "coveredPercentageImpact": 1 + }, + }; + """; + } + + @ParameterizedTest(name = "{index} => Invalid configuration: {2}") + @MethodSource + @DisplayName("should throw exceptions for invalid configurations") + void shouldReportNotConsistentConfiguration(final String json, final String errorMessage, + @SuppressWarnings("unused") final String displayName) { + assertThatIllegalArgumentException() + .isThrownBy(() -> fromJson(json)) + .withMessageContaining(errorMessage) + .withNoCause(); + } + + public static Stream shouldReportNotConsistentConfiguration() { + return Stream.of( + Arguments.of(""" + { + "coverage": { + "name": "JaCoCo and PIT", + "tools": [ + { + "id": "jacoco", + "metric": "line", + "pattern": "target/jacoco.xml" + } + ], + "maxScore": 0, + "coveredPercentageImpact": 1, + "missedPercentageImpact": 2 + } + } + """, "When configuring impacts then the score must not be zero.", + "an impact requires a positive score"), + Arguments.of(""" + { + "coverage": { + "name": "JaCoCo and PIT", + "tools": [ + { + "id": "jacoco", + "metric": "line", + "pattern": "target/jacoco.xml" + } + ], + "maxScore": 100, + "coveredPercentageImpact": 0, + "missedPercentageImpact": 0 + } + } + """, "When configuring a max score than an impact must be defined as well", + "a score requires an impact"), + Arguments.of(""" + { + "coverage": { + "name": "JaCoCo and PIT", + "maxScore": 100, + "coveredPercentageImpact": 1, + "missedPercentageImpact": 1 + } + } + """, "Configuration ID 'coverage' has no tools", + "empty tools configuration") + ); + } + @Test void shouldConvertObjectConfigurationFromJson() { - var configurations = CoverageConfiguration.from(""" + var configurations = fromJson(""" { "coverage": { "name": "JaCoCo and PIT", @@ -37,18 +125,17 @@ void shouldConvertObjectConfigurationFromJson() { """); assertThat(configurations).hasSize(1).first().satisfies(configuration -> assertThat(configuration) - .hasCoveredPercentageImpact(1) - .hasMissedPercentageImpact(2) + .hasCoveredPercentageImpact(1).hasMissedPercentageImpact(2) .hasMaxScore(50) .hasName("JaCoCo and PIT") - .isPositive() + .isPositive().hasImpact() .hasOnlyTools(new ToolConfiguration("jacoco", "", "target/jacoco.xml", "", getMetricName(Metric.LINE)), new ToolConfiguration("pit", "", "target/pit.xml", "", getMetricName(Metric.MUTATION)))); } @Test void shouldConvertSingleArrayElementConfigurationFromJson() { - var configurations = CoverageConfiguration.from(""" + var configurations = fromJson(""" { "coverage": [{ "tools": [ @@ -75,7 +162,7 @@ void shouldConvertSingleArrayElementConfigurationFromJson() { @Test void shouldConvertMultipleElementsConfigurationsFromJson() { - var configurations = CoverageConfiguration.from(""" + var configurations = fromJson(""" { "coverage": [ { @@ -121,6 +208,7 @@ private void verifyFirstConfiguration(final CoverageConfiguration configuration) .hasMissedPercentageImpact(2) .hasMaxScore(50) .isPositive() + .hasImpact() .hasOnlyTools(new ToolConfiguration("jacoco", "", "target/jacoco.xml", "", getMetricName(Metric.LINE)), new ToolConfiguration("pit", "", "target/pit.xml", "", getMetricName(Metric.MUTATION))); } @@ -135,10 +223,58 @@ private void verifyLastConfiguration(final CoverageConfiguration configuration) .hasMissedPercentageImpact(-10) .hasMaxScore(100) .isNotPositive() + .hasImpact() .hasOnlyTools(new ToolConfiguration("cobertura", "", "target/cobertura.xml", "", getMetricName(Metric.BRANCH))); } + @ParameterizedTest(name = "{index} => Positive configuration: {1}") + @MethodSource + @DisplayName("should identify positive configurations") + void shouldIdentifyPositiveValues(final String json, @SuppressWarnings("unused") final String displayName) { + var configurations = fromJson(json); + + assertThat(configurations).hasSize(1).first().satisfies(configuration -> + assertThat(configuration).isNotPositive().hasName(CoverageConfiguration.CODE_COVERAGE)); + } + + public static Stream shouldIdentifyPositiveValues() { + return Stream.of(Arguments.of(""" + { + "coverage": + { + "tools": [ + { + "id": "jacoco", + "metric": "line" + } + ], + "coveredPercentageImpact": 0, + "missedPercentageImpact": -1, + "maxScore": 50 + } + } + """, "missed impact is negative"), + Arguments.of(""" + { + "coverage": + { + "tools": [ + { + "id": "jacoco", + "metric": "line", + "pattern": "target/jacoco.xml" + } + ], + "coveredPercentageImpact": -1, + "missedPercentageImpact": 0, + "maxScore": 50 + } + } + """, "covered impact is negative") + ); + } + @Test void shouldAdhereToEquals() { EqualsVerifier.forClass(CoverageConfiguration.class) diff --git a/src/test/java/edu/hm/hafner/grading/TestConfigurationTest.java b/src/test/java/edu/hm/hafner/grading/TestConfigurationTest.java index 3899e5c8..cfccff63 100644 --- a/src/test/java/edu/hm/hafner/grading/TestConfigurationTest.java +++ b/src/test/java/edu/hm/hafner/grading/TestConfigurationTest.java @@ -1,17 +1,249 @@ package edu.hm.hafner.grading; +import java.util.List; +import java.util.stream.Stream; + import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; import static edu.hm.hafner.grading.assertions.Assertions.*; -class TestConfigurationTest { +class TestConfigurationTest extends AbstractConfigurationTest { + @Override + protected List fromJson(final String json) { + return TestConfiguration.from(json); + } + + @Override + protected String getInvalidJson() { + return """ + { + "tests": { + "name": "Unit Tests", + } + } + """; + } + + @ParameterizedTest(name = "{index} => Invalid configuration: {2}") + @MethodSource + @DisplayName("should throw exceptions for invalid configurations") + void shouldReportNotConsistentConfiguration(final String json, final String errorMessage, + @SuppressWarnings("unused") final String displayName) { + assertThatIllegalArgumentException() + .isThrownBy(() -> fromJson(json)) + .withMessageContaining(errorMessage) + .withNoCause(); + } + + public static Stream shouldReportNotConsistentConfiguration() { + return Stream.of( + Arguments.of(""" + { + "tests": { + "name": "Unit Tests", + "maxScore": 100, + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "passedImpact": 0, + "failureImpact": -5, + "skippedImpact": -1, + "successRateImpact": 1, + "failureRateImpact": -1 + } + } + """, "absolute or relative metrics", "absolute and relative values used"), + Arguments.of(""" + { + "tests": { + "name": "Unit Tests", + "maxScore": 0, + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "passedImpact": 0, + "failureImpact": 0, + "skippedImpact": 0, + "successRateImpact": 1, + "failureRateImpact": 0 + } + } + """, "When configuring impacts then the score must not be zero.", + "relative impact requires positive score"), + Arguments.of(""" + { + "tests": { + "name": "Unit Tests", + "maxScore": 0, + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "passedImpact": 0, + "failureImpact": 1, + "skippedImpact": 0, + "successRateImpact": 0, + "failureRateImpact": 0 + } + } + """, "When configuring impacts then the score must not be zero.", + "absolute impact requires positive score"), + Arguments.of(""" + { + "tests": { + "name": "Unit Tests", + "maxScore": 100, + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ] + } + } + """, "When configuring a max score than an impact must be defined as well", + "a score requires an impact"), + Arguments.of(""" + { + "tests": { + "name": "Unit Tests", + "maxScore": 100, + "passedImpact": 1 + } + } + """, "Configuration ID 'tests' has no tools", + "empty tools configuration") + ); + } + + @ParameterizedTest(name = "{index} => Positive configuration: {1}") + @MethodSource + @DisplayName("should identify positive configurations") + void shouldIdentifyPositiveValues(final String json, @SuppressWarnings("unused") final String displayName) { + var configurations = fromJson(json); + + assertThat(configurations).hasSize(1).first().satisfies(configuration -> + assertThat(configuration).isNotPositive()); + } + + public static Stream shouldIdentifyPositiveValues() { + return Stream.of(Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": 0, + "failureImpact": -5, + "skippedImpact": -1 + } + } + """, "passed impact is zero"), + Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": -1, + "failureImpact": 0, + "skippedImpact": -1 + } + } + """, "failure impact is zero"), + Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": -1, + "failureImpact": -5, + "skippedImpact": 0 + } + } + """, "skipped impact is zero"), + Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": 0, + "failureImpact": 0, + "skippedImpact": -1 + } + } + """, "skipped impact is negative"), + Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": 0, + "failureImpact": -5, + "skippedImpact": 0 + } + } + """, "failure impact is negative"), + Arguments.of(""" + { + "tests": { + "tools": [ + { + "id": "junit", + "pattern": "target/junit.xml" + } + ], + "maxScore": 50, + "passedImpact": -1, + "failureImpact": 0, + "skippedImpact": 0 + } + } + """, "passed impact is negative")); + } + @Test void shouldConvertObjectConfigurationFromJson() { - var configurations = TestConfiguration.from(""" + var configurations = fromJson(""" { "tests": { "name": "Unit Tests", @@ -29,19 +261,19 @@ void shouldConvertObjectConfigurationFromJson() { } """); - assertThat(configurations).hasSize(1).first().satisfies(configuration -> assertThat(configuration) - .hasPassedImpact(0) - .hasFailureImpact(-5) - .hasSkippedImpact(-1) - .hasMaxScore(50) - .hasName("Unit Tests") - .isNotPositive() - .hasOnlyTools(new ToolConfiguration("junit", "", "target/junit.xml", StringUtils.EMPTY))); + assertThat(configurations).hasSize(1).first().satisfies(configuration -> + assertThat(configuration) + .hasPassedImpact(0).hasFailureImpact(-5).hasSkippedImpact(-1) + .hasSuccessRateImpact(0).hasFailureRateImpact(0) + .hasMaxScore(50) + .hasName("Unit Tests") + .isNotPositive().isAbsolute().isNotRelative() + .hasOnlyTools(new ToolConfiguration("junit", "", "target/junit.xml", StringUtils.EMPTY))); } @Test void shouldConvertSingleArrayElementConfigurationFromJson() { - var configurations = TestConfiguration.from(""" + var configurations = fromJson(""" { "tests": [{ "name": "Unit Tests", @@ -70,7 +302,7 @@ void shouldConvertSingleArrayElementConfigurationFromJson() { @Test void shouldConvertMultipleElementsConfigurationsFromJson() { - var configurations = TestConfiguration.from(""" + var configurations = fromJson(""" { "tests": [ { @@ -119,6 +351,8 @@ private void verifyFirstConfiguration(final TestConfiguration configuration) { .hasSkippedImpact(1) .hasMaxScore(50) .isPositive() + .isAbsolute() + .isNotRelative() .hasOnlyTools(new ToolConfiguration("junit", "Junit tests", "target/junit.xml", StringUtils.EMPTY), new ToolConfiguration("jest", "JEST", "target/jest.xml", StringUtils.EMPTY)); } @@ -130,6 +364,8 @@ private void verifyLastConfiguration(final TestConfiguration configuration) { .hasSkippedImpact(-1) .hasMaxScore(500) .isNotPositive() + .isNotRelative() + .isAbsolute() .hasOnlyTools(new ToolConfiguration("junit", "", "target/junit.xml", StringUtils.EMPTY)); } diff --git a/src/test/java/edu/hm/hafner/grading/TestScoreTest.java b/src/test/java/edu/hm/hafner/grading/TestScoreTest.java index 886be753..121a6853 100644 --- a/src/test/java/edu/hm/hafner/grading/TestScoreTest.java +++ b/src/test/java/edu/hm/hafner/grading/TestScoreTest.java @@ -18,6 +18,86 @@ class TestScoreTest { private static final String NAME = "Tests"; private static final String ID = "tests"; + @Test + void shouldUseRelativeValues() { + var configuration = createConfiguration(""" + { + "tests": { + "tools": [ + { + "id": "tests", + "name": "Tests", + "pattern": "target/tests.xml" + } + ], + "maxScore": 100, + "successRateImpact": 1, + "failureRateImpact": 0 + } + } + """); + + var builder = new TestScoreBuilder() + .withId(ID) + .withName(NAME) + .withConfiguration(configuration); + var twenty = builder.withReport(createTestReport(2, 3, 5)).build(); + assertThat(twenty) + .hasId(ID).hasName(NAME).hasConfiguration(configuration) + .hasFailedSize(5).hasSkippedSize(3).hasTotalSize(10) + .hasMaxScore(100) + .hasImpact(20) + .hasValue(20); + assertThat(twenty.toString()).startsWith("{").endsWith("}") + .containsIgnoringWhitespaces("\"impact\":20"); + + assertThat(builder.withReport(createTestReport(12, 3, 5)).build()) + .hasImpact(60).hasValue(60); + assertThat(builder.withReport(createTestReport(95, 5, 0)).build()) + .hasImpact(95).hasValue(95); + assertThat(builder.withReport(createTestReport(100, 0, 0)).build()) + .hasImpact(100).hasValue(100); + + // Check rounding + assertThat(builder.withReport(createTestReport(197, 0, 3)).build()) + .hasImpact(99).hasValue(99); + assertThat(builder.withReport(createTestReport(198, 0, 2)).build()) + .hasImpact(99).hasValue(99); + assertThat(builder.withReport(createTestReport(199, 0, 1)).build()) + .hasImpact(99).hasValue(99); + assertThat(builder.withReport(createTestReport(200, 0, 0)).build()) + .hasImpact(100).hasValue(100); + } + + @Test + void shouldUseRelativeFailureValues() { + var configuration = createConfiguration(""" + { + "tests": { + "tools": [ + { + "id": "tests", + "name": "Tests", + "pattern": "target/tests.xml" + } + ], + "maxScore": 100, + "failureRateImpact": -1 + } + } + """); + + var builder = new TestScoreBuilder() + .withId(ID) + .withName(NAME) + .withConfiguration(configuration); + var score = builder.withReport(createTestReport(2, 1, 7)).build(); + assertThat(score) + .hasMaxScore(100) + .hasImpact(-70) + .hasValue(30); + } + @Test void shouldCalculateImpactAndScoreWithNegativeValues() { var configuration = createConfiguration("""