Skip to content

Commit

Permalink
Test names in score config are now trimmed (since we already trim nam…
Browse files Browse the repository at this point in the history
…es when tests are being updated).
  • Loading branch information
Martin Krulis committed Apr 19, 2019
1 parent e1e7c63 commit 36e6d28
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 25 deletions.
6 changes: 2 additions & 4 deletions app/V1Module/presenters/ExercisesConfigPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -653,12 +653,10 @@ public function actionSetScoreConfig(string $id) {

// validate score configuration
$calculator = $this->calculators->getCalculator($exercise->getScoreCalculator());
if (!$calculator->isScoreConfigValid($config)) {
throw new ExerciseConfigException("Exercise score configuration is not valid");
}
$normalizedConfig = $calculator->validateAndNormalizeScore($config); // throws if validation fails

$exercise->updatedNow();
$exercise->setScoreConfig($config);
$exercise->setScoreConfig($normalizedConfig);
$this->exercises->flush();

// check exercise configuration
Expand Down
10 changes: 10 additions & 0 deletions app/helpers/Evaluation/ScoreCalculators/IScoreCalculator.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace App\Helpers\Evaluation;
use App\Exceptions\SubmissionEvaluationFailedException;
use App\Exceptions\ExerciseConfigException;

/**
* Interface for score computations. Purpose is to merge scores of all
Expand All @@ -24,6 +25,15 @@ public function computeScore(string $scoreConfig, array $testResults): float;
*/
public function isScoreConfigValid(string $scoreConfig): bool;

/**
* Performs validation and normalization on config string.
* This should be used instead of validation when the score config is processed as API input.
* @param string $scoreConfig YAML configuration for the score calculator
* @return string Normalized and polished YAML with score configuration
* @throws ExerciseConfigException
*/
public function validateAndNormalizeScore(string $scoreConfig): string;

/**
* Make default configuration for array of test names. Each test will
* have the same priority as others.
Expand Down
62 changes: 43 additions & 19 deletions app/helpers/Evaluation/ScoreCalculators/SimpleScoreCalculator.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Helpers\Evaluation;

use App\Exceptions\SubmissionEvaluationFailedException;
use App\Exceptions\ExerciseConfigException;
use App\Helpers\Evaluation\IScoreCalculator;
use Symfony\Component\Yaml\Yaml;
use Symfony\Component\Yaml\Exception\ParseException;
Expand All @@ -17,6 +18,32 @@
* weights can be arbitrary, what matters are ratios.
*/
class SimpleScoreCalculator implements IScoreCalculator {
/**
* Internal function that safely retrieves score config weights.
* @param string $scoreConfig
* @return array|null Null if the config is invalid, name => weight array otherwise.
*/
private function getTestWeights(string $scoreConfig): ?array {
try {
$config = Yaml::parse($scoreConfig);
$normalizedWeights = [];

if (isset($config['testWeights']) && is_array($config['testWeights'])) {
foreach ($config['testWeights'] as $name => $value) {
if (!is_integer($value)) {
return null;
}
$normalizedWeights[trim($name)] = $value;
}
} else {
return null;
}
} catch (ParseException $e) {
return null;
}

return $normalizedWeights;
}

/**
* Function that computes the resulting score from simple YML config and test results score
Expand All @@ -26,13 +53,11 @@ class SimpleScoreCalculator implements IScoreCalculator {
* @throws SubmissionEvaluationFailedException
*/
public function computeScore(string $scoreConfig, array $testResults): float {
if (!$this->isScoreConfigValid($scoreConfig)) {
$weights = $this->getTestWeights($scoreConfig);
if ($weights === null) {
throw new SubmissionEvaluationFailedException("Assignment score configuration is invalid");
}

$config = Yaml::parse($scoreConfig);
$weights = $config['testWeights'];

// assign zero ratio to all tests which does not have specified value
foreach ($testResults as $name => $score) {
if (!array_key_exists($name, $weights)) {
Expand All @@ -57,23 +82,22 @@ public function computeScore(string $scoreConfig, array $testResults): float {
* @return bool If the configuration is valid or not
*/
public function isScoreConfigValid(string $scoreConfig): bool {
try {
$config = Yaml::parse($scoreConfig);
return $this->getTestWeights($scoreConfig) !== null;
}

if (isset($config['testWeights']) && is_array($config['testWeights'])) {
foreach ($config['testWeights'] as $value) {
if (!is_integer($value)) {
return false;
}
}
} else {
return false;
}
} catch (ParseException $e) {
return false;
/**
* Performs validation and normalization on config string.
* This should be used instead of validation when the score config is processed as API input.
* @param string $scoreConfig YAML configuration for the score calculator
* @return string Normalized and polished YAML with score configuration
* @throws ExerciseConfigException
*/
public function validateAndNormalizeScore(string $scoreConfig): string {
$weights = $this->getTestWeights($scoreConfig);
if ($weights === null) {
throw new ExerciseConfigException("Exercise score configuration is not valid");
}

return true;
return Yaml::dump([ 'testWeights' => $weights ]);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions tests/Presenters/ExercisesConfigPresenter.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use App\Model\Entity\ExerciseTest;
use App\Model\Entity\HardwareGroup;
use App\V1Module\Presenters\ExercisesConfigPresenter;
use Tester\Assert;
use Symfony\Component\Yaml\Yaml;

/**
* @testCase
Expand Down Expand Up @@ -515,8 +516,9 @@ class TestExercisesConfigPresenter extends Tester\TestCase
$result = $response->getPayload();
Assert::equal(200, $result['code']);

$payload = $result['payload'];
Assert::equal("testWeights:\n \"Test 1\": 100\n \"Test 2\": 100\n \"Test 3\": 100", $payload);
$payload = Yaml::parse($result['payload']);
$expected = Yaml::parse($config);
Assert::equal($expected, $payload);
}

public function testGetTests() {
Expand Down

0 comments on commit 36e6d28

Please sign in to comment.