Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement penalty code checks #210

Merged
merged 8 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* [Mutation coverage](#mutation-coverage)
* [Meta tests](#meta-tests)
* [Code checks](#code-checks)
* [Required code checks](#required-code-checks)
* [Penalty code checks](#penalty-code-checks)
* [Success message](#success-message)
* [External process](#external-process)
* [Team](#team)
Expand Down Expand Up @@ -370,25 +370,27 @@ should have properties: FAIL (weight: 2)
either make use of Arbitraries or JQWik IntRange-like annotations: FAIL (weight: 8)
```

#### Required code checks
#### Penalty code checks

Required code checks work in a similar way to regular code checks. However, they are not considered a grading component. Instead, if one or more required code checks fail, the final grade is overridden to 0, irrespective of the score of any of the grading components.
Penalty code checks work in a similar way to regular code checks. However, they are not considered a grading component. Instead, if a penalty code check passes, this has no effect on the final score. However, if it fails, its weight is subtracted from the final grade.

They can be defined as follows:

```java
@Override
public CheckScript requiredCheckScript() {
public CheckScript penaltyCheckScript() {
return new CheckScript(List.of(
new SingleCheck("Trip Repository should be mocked",
// this check deducts 10 points if it fails
new SingleCheck(10, "Trip Repository should be mocked",
new MockClass("TripRepository")),
new SingleCheck("Reservation Repository should be mocked",
// this check overrides the grade to 0 if it fails
new SingleCheck(100, "Reservation Repository should be mocked",
new MockClass("ReservationRepository"))
));
}
```

If this method is not overridden, required code checks are disabled.
If this method is not overridden, penalty code checks are disabled.
martinmladenov marked this conversation as resolved.
Show resolved Hide resolved

#### Success message

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public CheckScript checkScript() {
return new CheckScript(Collections.emptyList());
}

public CheckScript requiredCheckScript() {
public CheckScript penaltyCheckScript() {
return new CheckScript(Collections.emptyList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public static List<ExecutionStep> fullMode() {
new RunJUnitTestsStep(),
new CollectCoverageInformationStep(),
new RunPitestStep(),
new RunRequiredCodeChecksStep(),
new RunPenaltyCodeChecksStep(),
new RunCodeChecksStep(),
new KillExternalProcessStep(),
new RunMetaTestsStep()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@

import static nl.tudelft.cse1110.andy.utils.FilesUtils.findSolution;

public class RunRequiredCodeChecksStep implements ExecutionStep {
public class RunPenaltyCodeChecksStep implements ExecutionStep {
@Override
public void execute(Context ctx, ResultBuilder result) {
DirectoryConfiguration dirCfg = ctx.getDirectoryConfiguration();
RunConfiguration runCfg = ctx.getRunConfiguration();

CheckScript script = runCfg.requiredCheckScript();
CheckScript script = runCfg.penaltyCheckScript();
try {
script.runChecks(findSolution(dirCfg.getWorkingDir()));
} catch (FileNotFoundException e) {
throw new RuntimeException(e);
}

result.logRequiredCodeChecks(script);
result.logPenaltyCodeChecks(script);
}

@Override
public boolean equals(Object other) {
return other instanceof RunRequiredCodeChecksStep;
return other instanceof RunPenaltyCodeChecksStep;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ public int calculateFinalGrade(GradeValues gradeValues, GradeWeight weights) {
if(finalGrade < 0 || finalGrade > 100)
throw new RuntimeException("Invalid grade calculation");

if(gradeValues.getPenalty() < 0){
martinmladenov marked this conversation as resolved.
Show resolved Hide resolved
throw new RuntimeException("Negative penalty: " + gradeValues.getPenalty());
}

// Apply penalty
finalGrade -= gradeValues.getPenalty();

// Grade should not go below 0
if(finalGrade < 0) finalGrade = 0;

// Grades between 99.5 and 100 should be rounded down to 99 instead of up
if (finalGrade == 100 && hasIncompleteComponents(gradeValues, weights)) {
finalGrade = 99;
Expand Down
17 changes: 15 additions & 2 deletions andy/src/main/java/nl/tudelft/cse1110/andy/grade/GradeValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public class GradeValues {
private int checksPassed;
private int totalChecks;

private int penalty;

public int getCoveredBranches() {
return coveredBranches;
}
Expand Down Expand Up @@ -75,14 +77,25 @@ public void setCheckGrade(int checksPassed, int totalChecks) {
this.totalChecks = totalChecks;
}

public static GradeValues fromResults(CoverageResult coverageResults, CodeChecksResult codeCheckResults, MutationTestingResult mutationResults, MetaTestsResult metaTestResults) {
public int getPenalty() {
return penalty;
}

public GradeValues setPenalty(int penalty) {
this.penalty = penalty;
return this;
}

public static GradeValues fromResults(CoverageResult coverageResults, CodeChecksResult codeCheckResults, MutationTestingResult mutationResults, MetaTestsResult metaTestResults, CodeChecksResult penaltyCodeCheckResults) {
GradeValues grades = new GradeValues();
grades.setBranchGrade(coverageResults.getCoveredBranches(), coverageResults.getTotalNumberOfBranches());
grades.setCheckGrade(codeCheckResults.getNumberOfPassedChecks(), codeCheckResults.getTotalNumberOfChecks());
grades.setMutationGrade(mutationResults.getKilledMutants(), mutationResults.getTotalNumberOfMutants());
grades.setMetaGrade(metaTestResults.getPassedMetaTests(), metaTestResults.getTotalTests());

// penalty is equal to the sum of the weights of all failed penalty code checks
grades.setPenalty(penaltyCodeCheckResults.getCheckResults().stream().mapToInt(check -> check.passed() ? 0 : check.getWeight()).sum());

return grades;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,21 @@ public static CodeChecksResult build(List<CodeCheckResult> checkResults) {
}

public int getNumberOfPassedChecks() {
return checkResults.stream().mapToInt(check -> check.passed() ? check.getWeight() : 0).sum();
return getNumberOfPassedChecks(true);
}

public int getNumberOfPassedChecks(boolean includeWeight) {
martinmladenov marked this conversation as resolved.
Show resolved Hide resolved
return checkResults.stream().mapToInt(check -> check.passed() ?
(includeWeight ? check.getWeight() : 1) :
0).sum();
}

public int getTotalNumberOfChecks() {
return checkResults.stream().mapToInt(c -> c.getWeight()).sum();
return getTotalNumberOfChecks(true);
}

public int getTotalNumberOfChecks(boolean includeWeight) {
return checkResults.stream().mapToInt(c -> (includeWeight ? c.getWeight() : 1)).sum();
}

public List<CodeCheckResult> getCheckResults() {
Expand All @@ -40,10 +50,6 @@ public boolean wasExecuted() {
return wasExecuted;
}

public boolean allChecksPass() {
return getNumberOfPassedChecks() == getTotalNumberOfChecks();
}

@Override
public String toString() {
return "CodeChecksResult{" +
Expand Down
18 changes: 12 additions & 6 deletions andy/src/main/java/nl/tudelft/cse1110/andy/result/Result.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,25 @@ public class Result {
private final UnitTestsResult tests;
private final MutationTestingResult mutationTesting;
private final CodeChecksResult codeChecks;
private final CodeChecksResult requiredCodeChecks;
private final CodeChecksResult penaltyCodeChecks;
private final CoverageResult coverage;
private final MetaTestsResult metaTests;
private final int penalty;
private final int finalGrade;
private final GenericFailure genericFailure;
private final double timeInSeconds;
private final GradeWeight weights;
private final String successMessage;

public Result(CompilationResult compilation, UnitTestsResult tests, MutationTestingResult mutationTesting, CodeChecksResult codeChecks, CodeChecksResult requiredCodeChecks, CoverageResult coverage, MetaTestsResult metaTests, int finalGrade, GenericFailure genericFailure, double timeInSeconds, GradeWeight weights, String successMessage) {
public Result(CompilationResult compilation, UnitTestsResult tests, MutationTestingResult mutationTesting, CodeChecksResult codeChecks, CodeChecksResult penaltyCodeChecks, CoverageResult coverage, MetaTestsResult metaTests, int penalty, int finalGrade, GenericFailure genericFailure, double timeInSeconds, GradeWeight weights, String successMessage) {
this.compilation = compilation;
this.tests = tests;
this.mutationTesting = mutationTesting;
this.codeChecks = codeChecks;
this.requiredCodeChecks = requiredCodeChecks;
this.penaltyCodeChecks = penaltyCodeChecks;
this.coverage = coverage;
this.metaTests = metaTests;
this.penalty = penalty;
this.finalGrade = finalGrade;
this.genericFailure = genericFailure;
this.timeInSeconds = timeInSeconds;
Expand All @@ -38,7 +40,7 @@ public Result(CompilationResult compilation, UnitTestsResult tests, MutationTest
}

public Result(CompilationResult compilation, double timeInSeconds) {
this(compilation, UnitTestsResult.empty(), MutationTestingResult.empty(), CodeChecksResult.empty(), CodeChecksResult.empty(), CoverageResult.empty(), MetaTestsResult.empty(), 0, GenericFailure.noFailure(), timeInSeconds, null, null);
this(compilation, UnitTestsResult.empty(), MutationTestingResult.empty(), CodeChecksResult.empty(), CodeChecksResult.empty(), CoverageResult.empty(), MetaTestsResult.empty(), 0, 0, GenericFailure.noFailure(), timeInSeconds, null, null);
}

public CompilationResult getCompilation() {
Expand All @@ -53,8 +55,8 @@ public MutationTestingResult getMutationTesting() {
return mutationTesting;
}

public CodeChecksResult getRequiredCodeChecks() {
return requiredCodeChecks;
public CodeChecksResult getPenaltyCodeChecks() {
return penaltyCodeChecks;
}

public CodeChecksResult getCodeChecks() {
Expand All @@ -69,6 +71,10 @@ public MetaTestsResult getMetaTests() {
return metaTests;
}

public int getPenalty() {
return penalty;
}

public int getFinalGrade() {
return finalGrade;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class ResultBuilder {
private UnitTestsResult testResults = UnitTestsResult.empty();
private MutationTestingResult mutationResults = MutationTestingResult.empty();
private CodeChecksResult codeCheckResults = CodeChecksResult.empty();
private CodeChecksResult requiredCodeCheckResults = CodeChecksResult.empty();
private CodeChecksResult penaltyCodeCheckResults = CodeChecksResult.empty();
private CoverageResult coverageResults = CoverageResult.empty();
private MetaTestsResult metaTestResults = MetaTestsResult.empty();

Expand Down Expand Up @@ -168,10 +168,10 @@ public void logCodeChecks(CheckScript script) {
}

/*
* Required code checks
* Penalty code checks (subtract points from the final score if they fail)
*/
public void logRequiredCodeChecks(CheckScript script) {
this.requiredCodeCheckResults = buildCodeChecksResult(script);
public void logPenaltyCodeChecks(CheckScript script) {
this.penaltyCodeCheckResults = buildCodeChecksResult(script);
}

private static CodeChecksResult buildCodeChecksResult(CheckScript script) {
Expand Down Expand Up @@ -270,21 +270,23 @@ public Result build() {
if(!compilation.successful()) {
return new Result(compilation, timeInSeconds);
} else {
GradeValues grades = GradeValues.fromResults(coverageResults, codeCheckResults, mutationResults, metaTestResults);
GradeValues grades = GradeValues.fromResults(coverageResults, codeCheckResults, mutationResults, metaTestResults, penaltyCodeCheckResults);
GradeWeight weights = GradeWeight.fromConfig(ctx.getRunConfiguration().weights());
String successMessage = ctx.getRunConfiguration().successMessage();

this.checkExternalProcessExit();

int finalGrade = calculateFinalGrade(grades, weights);
final int finalGrade = calculateFinalGrade(grades, weights);
final int penalty = grades.getPenalty();

return new Result(compilation,
testResults,
mutationResults,
codeCheckResults,
requiredCodeCheckResults,
penaltyCodeCheckResults,
coverageResults,
metaTestResults,
penalty,
finalGrade,
genericFailureObject,
timeInSeconds,
Expand Down Expand Up @@ -332,9 +334,8 @@ public boolean hasFailed() {
boolean compilationFailed = compilation!=null && !compilation.successful();
boolean unitTestsFailed = testResults != null && testResults.didNotGoWell();
boolean hasGenericFailure = genericFailureObject.hasFailure();
boolean requiredCodeChecksFailed = !requiredCodeCheckResults.allChecksPass();

return compilationFailed || unitTestsFailed || hasGenericFailure || requiredCodeChecksFailed;
return compilationFailed || unitTestsFailed || hasGenericFailure;
}

public UnitTestsResult getTestResults() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ private void writeStdOutFile(Context ctx, Result result) {
printTestResults(result.getTests());
printCoverageResults(result.getCoverage());
printMutationTestingResults(result.getMutationTesting());
printRequiredCodeCheckResults(ctx, result.getRequiredCodeChecks());
printCodeCheckResults(ctx, result.getCodeChecks());
printCodeCheckResults(ctx, result.getCodeChecks(), result.getPenaltyCodeChecks());
printMetaTestResults(ctx, result.getMetaTests());
printFinalGrade(ctx, result);
printModeAndTimeToRun(ctx, result.getTimeInSeconds());
Expand Down Expand Up @@ -176,6 +175,11 @@ private void printFinalGrade(Context ctx, Result result) {
printGradeCalculationDetails("Mutation coverage", result.getMutationTesting().getKilledMutants(), result.getMutationTesting().getTotalNumberOfMutants(), result.getWeights().getMutationCoverageWeight());
printGradeCalculationDetails("Code checks", result.getCodeChecks().getNumberOfPassedChecks(), result.getCodeChecks().getTotalNumberOfChecks(), result.getWeights().getCodeChecksWeight());
printGradeCalculationDetails("Meta tests", result.getMetaTests().getPassedMetaTests(), result.getMetaTests().getTotalTests(), result.getWeights().getMetaTestsWeight());
}

// print penalty
if(result.getPenalty() != 0){
l(String.format("Penalty: %d", result.getPenalty()));
l("");
}

Expand Down Expand Up @@ -287,8 +291,8 @@ private void printCoverageResults(CoverageResult coverage) {

}

private void printCodeCheckResults(Context ctx, CodeChecksResult codeChecks) {
if(!codeChecks.wasExecuted())
private void printCodeCheckResults(Context ctx, CodeChecksResult codeChecks, CodeChecksResult penaltyCodeChecks) {
if(!codeChecks.wasExecuted() && !penaltyCodeChecks.wasExecuted())
return;

boolean allHints = modeActionSelector(ctx).shouldShowFullHints();
Expand All @@ -297,36 +301,16 @@ private void printCodeCheckResults(Context ctx, CodeChecksResult codeChecks) {
if(!allHints && !onlyResult)
return;

if(codeChecks.hasChecks()) {
if(codeChecks.hasChecks() || penaltyCodeChecks.hasChecks()) {
l("\n--- Code checks");
printCodeCheckOutput(codeChecks, allHints);
}

}

private void printRequiredCodeCheckResults(Context ctx, CodeChecksResult codeChecks) {
if(!codeChecks.wasExecuted())
return;

boolean allHints = modeActionSelector(ctx).shouldShowFullHints();
boolean onlyResult = modeActionSelector(ctx).shouldShowPartialHints();

if(!allHints && !onlyResult)
return;

if(codeChecks.hasChecks()) {
l("\n--- Required code checks");
printCodeCheckOutput(codeChecks, allHints);

if(!codeChecks.allChecksPass()){
l("\nSome required code checks failed. Stopping the assessment.");
}
printCodeCheckOutput(codeChecks, penaltyCodeChecks, allHints);
}

}

private void printCodeCheckOutput(CodeChecksResult codeChecks, boolean allHints) {
l(String.format("%d/%d passed", codeChecks.getNumberOfPassedChecks(), codeChecks.getTotalNumberOfChecks()));
private void printCodeCheckOutput(CodeChecksResult codeChecks, CodeChecksResult penaltyCodeChecks, boolean allHints) {
l(String.format("%d/%d passed", codeChecks.getNumberOfPassedChecks() + penaltyCodeChecks.getNumberOfPassedChecks(false),
codeChecks.getTotalNumberOfChecks() + penaltyCodeChecks.getTotalNumberOfChecks(false)));

if(allHints) {
for (CodeCheckResult result : codeChecks.getCheckResults()) {
Expand All @@ -335,6 +319,12 @@ private void printCodeCheckOutput(CodeChecksResult codeChecks, boolean allHints)
result.passed() ? "PASS" : "FAIL",
result.getWeight()));
}
for (CodeCheckResult result : penaltyCodeChecks.getCheckResults()) {
l(String.format("%s: %s (penalty: %d)",
result.getDescription(),
result.passed() ? "PASS" : "FAIL",
result.getWeight()));
}

}
}
Expand Down
Loading