moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-05 08:56:36 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-69246 questions: allow for float issues when validating manual marks

Browse source 
As well as fixing the bug, I also rewrote the test to use
data providers, which should lead to more useful failure messages.

And, I moved the magic number we used as the float tolerence to
be a named constant.
This commit is contained in:
Tim Hunt 2020-07-09 22:30:18 +01:00
parent b3b04e083b
commit 10f03d2a99
4 changed files with 54 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

2
question/behaviour/behaviourbase.php
View file

@ -784,7 +784,7 @@ abstract class question_cbm {
// errors for people who have used TGM's patches. // errors for people who have used TGM's patches.
return 0; return 0;
} }
if ($fraction <= 0.00000005) { if ($fraction <= question_utils::MARK_TOLERANCE) {
return self::$wrongscore[$certainty]; return self::$wrongscore[$certainty];
} else { } else {
return self::$rightscore[$certainty] * $fraction; return self::$rightscore[$certainty] * $fraction;

10
question/engine/lib.php
View file

@ -790,6 +790,16 @@ class question_out_of_sequence_exception extends moodle_exception {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/ */
abstract class question_utils { abstract class question_utils {
/**
* @var float tolerance to use when comparing question mark/fraction values.
*
* When comparing floating point numbers in a computer, the representation is not
* necessarily exact. Therefore, we need to allow a tolerance.
* Question marks are stored in the database as decimal numbers with 7 decimal places.
* Therefore, this is the appropriate tolerance to use.
*/
const MARK_TOLERANCE = 0.00000005;
/** /**
* Tests to see whether two arrays have the same keys, with the same values * Tests to see whether two arrays have the same keys, with the same values
* (as compared by ===) for each key. However, the order of the arrays does * (as compared by ===) for each key. However, the order of the arrays does

5
question/engine/questionattempt.php
View file

@ -695,7 +695,7 @@ class question_attempt {
/** @return bool whether this question attempt has a non-zero maximum mark. */ /** @return bool whether this question attempt has a non-zero maximum mark. */
public function has_marks() { public function has_marks() {
// Since grades are stored in the database as NUMBER(12,7). // Since grades are stored in the database as NUMBER(12,7).
return $this->maxmark >= 0.00000005; return $this->maxmark >= question_utils::MARK_TOLERANCE;
} }
/** /**
@ -1158,7 +1158,8 @@ class question_attempt {
} }
$maxmark = $this->get_max_mark(); $maxmark = $this->get_max_mark();
if ($mark > $maxmark * $this->get_max_fraction() || $mark < $maxmark * $this->get_min_fraction()) { if ($mark > $maxmark * $this->get_max_fraction() + question_utils::MARK_TOLERANCE ||
$mark < $maxmark * $this->get_min_fraction() - question_utils::MARK_TOLERANCE) {
return get_string('manualgradeoutofrange', 'question'); return get_string('manualgradeoutofrange', 'question');
} }

54
question/engine/tests/questionattempt_with_steps_test.php
View file

@ -170,19 +170,45 @@ class question_attempt_with_steps_test extends advanced_testcase {
$qa->get_max_fraction(); $qa->get_max_fraction();
} }
public function test_validate_manual_mark() { /**
$this->qa->set_min_fraction(0); * Test cases for {@see test_validate_manual_mark()}.
$this->qa->set_max_fraction(1); *
$this->assertSame('', $this->qa->validate_manual_mark(null)); * @return array test cases
$this->assertSame('', $this->qa->validate_manual_mark('')); */
$this->assertSame('', $this->qa->validate_manual_mark('0')); public function validate_manual_mark_cases(): array {
$this->assertSame('', $this->qa->validate_manual_mark('0.0')); // Recall, the DB schema stores question grade information to 7 decimal places.
$this->assertSame('', $this->qa->validate_manual_mark('2,0')); return [
$this->assertSame(get_string('manualgradeinvalidformat', 'question'), [0, 1, 2, null, ''],
$this->qa->validate_manual_mark('frog')); [0, 1, 2, '', ''],
$this->assertSame(get_string('manualgradeoutofrange', 'question'), [0, 1, 2, '0', ''],
$this->qa->validate_manual_mark('2.1')); [0, 1, 2, '0.0', ''],
$this->assertSame(get_string('manualgradeoutofrange', 'question'), [0, 1, 2, '2,0', ''],
$this->qa->validate_manual_mark('-0,01')); [0, 1, 2, 'frog', get_string('manualgradeinvalidformat', 'question')],
[0, 1, 2, '2.1', get_string('manualgradeoutofrange', 'question')],
[0, 1, 2, '-0,01', get_string('manualgradeoutofrange', 'question')],
[-0.3333333, 1, 0.75, '0.75', ''],
[-0.3333333, 1, 0.75, '0.7500001', get_string('manualgradeoutofrange', 'question')],
[-0.3333333, 1, 0.75, '-0.25', ''],
[-0.3333333, 1, 0.75, '-0.2500001', get_string('manualgradeoutofrange', 'question')],
];
}
/**
* Test validate_manual_mark.
*
* @dataProvider validate_manual_mark_cases
*
* @param float $minfraction minimum fraction for the question being attempted.
* @param float $maxfraction maximum fraction for the question being attempted.
* @param float $maxmark marks for the question attempt.
* @param string|null $currentmark submitted mark.
* @param string $expectederror expected error, if any.
*/
public function test_validate_manual_mark(float $minfraction, float $maxfraction,
float $maxmark, ?string $currentmark, string $expectederror) {
$this->qa->set_min_fraction($minfraction);
$this->qa->set_max_fraction($maxfraction);
$this->qa->set_max_mark($maxmark);
$this->assertSame($expectederror, $this->qa->validate_manual_mark($currentmark));
} }
} }