From 97e3b9d657ad8a7305b8c94c254055b79e31b616 Mon Sep 17 00:00:00 2001 From: Matthias Opitz Date: Wed, 14 Feb 2024 11:48:37 +0000 Subject: [PATCH] MDL-76024 calculated questions: cast float type before comparison * added calculated question for multiplication to tests/helper.php. * added test_grading_of_negative_responses() test to question/type/calculated/tests/question_test.php --- question/type/calculated/questiontype.php | 4 +- question/type/calculated/tests/helper.php | 145 +++++++++++++++++- .../type/calculated/tests/question_test.php | 40 +++++ 3 files changed, 186 insertions(+), 3 deletions(-) diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php index 9708f9d44dd..556a4352121 100644 --- a/question/type/calculated/questiontype.php +++ b/question/type/calculated/questiontype.php @@ -1121,8 +1121,8 @@ class qtype_calculated extends question_type { $correcttrue = new stdClass(); $correcttrue->correct = $formattedanswer->answer; $correcttrue->true = ''; - if ($formattedanswer->answer < $answer->min || - $formattedanswer->answer > $answer->max) { + if ((float) $formattedanswer->answer < $answer->min || + (float) $formattedanswer->answer > $answer->max) { $comment->outsidelimit = true; $comment->answers[$key] = $key; $comment->stranswers[$key] .= diff --git a/question/type/calculated/tests/helper.php b/question/type/calculated/tests/helper.php index 623ed446fd0..7cfc172540b 100644 --- a/question/type/calculated/tests/helper.php +++ b/question/type/calculated/tests/helper.php @@ -41,7 +41,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); */ class qtype_calculated_test_helper extends question_test_helper { public function get_test_questions() { - return array('sum'); + return array('sum', 'mult'); } /** @@ -189,6 +189,149 @@ class qtype_calculated_test_helper extends question_test_helper { return $fromform; } + + /** + * Makes a calculated question about multiplying two numbers. + * + * @return qtype_calculated_question + * @throws coding_exception + */ + public function make_calculated_question_mult() { + question_bank::load_question_definition_classes('calculated'); + $q = new qtype_calculated_question(); + test_question_maker::initialise_a_question($q); + $q->name = 'Simple multiplication'; + $q->questiontext = 'What is {a} * {b}?'; + $q->generalfeedback = 'Generalfeedback: {={a} * {b}} is the right answer.'; + + $q->answers = [ + 13 => new \qtype_calculated\qtype_calculated_answer(13, '{a} * {b}', 1.0, 'Positive.', FORMAT_HTML, 0), + 14 => new \qtype_calculated\qtype_calculated_answer(14, '-{a} * {b}', 0.0, 'Negative.', FORMAT_HTML, 0), + ]; + foreach ($q->answers as $answer) { + $answer->correctanswerlength = 2; + $answer->correctanswerformat = 1; + } + + $q->qtype = question_bank::get_qtype('calculated'); + $q->unitdisplay = qtype_numerical::UNITOPTIONAL; + $q->unitgradingtype = 0; + $q->unitpenalty = 0; + $q->unit = 'cm'; + $q->ap = new qtype_numerical_answer_processor([]); + $q->synchronised = false; + + $q->datasetloader = new qtype_calculated_test_dataset_loader(0, [ + ['a' => 1, 'b' => 5], + ['a' => 3, 'b' => 4], + ['a' => 3, 'b' => 0.01416], + ['a' => 31, 'b' => 0.01416], + ]); + + return $q; + } + + /** + * Makes a calculated question about multiplying two numbers. + * + * @return qtype_calculated_question + * @throws coding_exception + */ + public function get_calculated_question_data_mult() { + question_bank::load_question_definition_classes('calculated'); + $qdata = new stdClass(); + test_question_maker::initialise_question_data($qdata); + + $qdata->qtype = 'calculated'; + $qdata->name = 'Simple multiplication'; + $qdata->questiontext = 'What is {a} * {b}?'; + $qdata->generalfeedback = 'Generalfeedback: {={a} * {b}} is the right answer.'; + $qdata->status = \core_question\local\bank\question_version_status::QUESTION_STATUS_READY; + + $qdata->options = new stdClass(); + $qdata->options->unitgradingtype = 0; + $qdata->options->unitpenalty = 0.0; + $qdata->options->showunits = qtype_numerical::UNITOPTIONAL; + $qdata->unit = 'cm'; + $qdata->options->unitsleft = 0; + $qdata->options->synchronize = 0; + + $qdata->options->answers = [ + 13 => new \qtype_calculated\qtype_calculated_answer(13, '{a} * {b}', 1.0, 'Positive.', FORMAT_HTML, 0.001), + 14 => new \qtype_calculated\qtype_calculated_answer(14, '-{a} * {b}', 0.0, 'Negative.', FORMAT_HTML, 0.001), + ]; + foreach ($qdata->options->answers as $answer) { + $answer->correctanswerlength = 2; + $answer->correctanswerformat = 1; + } + + $qdata->options->units = []; + + return $qdata; + } + + /** + * Makes a calculated question about multiplying two numbers. + * + * @return qtype_calculated_question + * @throws coding_exception + */ + public function get_calculated_question_form_data_mult() { + question_bank::load_question_definition_classes('calculated'); + $fromform = new stdClass(); + + $fromform->name = 'Simple multiplication'; + $fromform->questiontext = 'What is {a} * {b}?'; + $fromform->defaultmark = 1.0; + $fromform->generalfeedback = 'Generalfeedback: {={a} * {b}} is the right answer.'; + + $fromform->unitrole = '0'; + $fromform->unitpenalty = 0.1; + $fromform->unitgradingtypes = '1'; + $fromform->unitsleft = '0'; + $fromform->nounits = 1; + $fromform->multiplier = []; + $fromform->multiplier[0] = '1.0'; + $fromform->synchronize = 0; + $fromform->answernumbering = 0; + $fromform->shuffleanswers = 0; + + $fromform->noanswers = 6; + $fromform->answer = []; + $fromform->answer[0] = '{a} * {b}'; + $fromform->answer[1] = '-{a} * {b}'; + + $fromform->fraction = []; + $fromform->fraction[0] = '1.0'; + $fromform->fraction[1] = '0.0'; + + $fromform->tolerance = []; + $fromform->tolerance[0] = 0.001; + $fromform->tolerance[1] = 0.001; + + $fromform->tolerancetype[0] = 1; + $fromform->tolerancetype[1] = 1; + + $fromform->correctanswerlength[0] = 2; + $fromform->correctanswerlength[1] = 2; + + $fromform->correctanswerformat[0] = 1; + $fromform->correctanswerformat[1] = 1; + + $fromform->feedback = []; + $fromform->feedback[0] = []; + $fromform->feedback[0]['format'] = FORMAT_HTML; + $fromform->feedback[0]['text'] = 'Positive.'; + + $fromform->feedback[1] = []; + $fromform->feedback[1]['format'] = FORMAT_HTML; + $fromform->feedback[1]['text'] = 'Negative.'; + + $fromform->status = \core_question\local\bank\question_version_status::QUESTION_STATUS_READY; + + return $fromform; + } + } diff --git a/question/type/calculated/tests/question_test.php b/question/type/calculated/tests/question_test.php index 0d6db2f3c28..70a2ee22c99 100644 --- a/question/type/calculated/tests/question_test.php +++ b/question/type/calculated/tests/question_test.php @@ -16,7 +16,9 @@ namespace qtype_calculated; +use qtype_calculated; use question_attempt_step; +use question_bank; use question_classified_response; use question_display_options; use question_state; @@ -196,4 +198,42 @@ class question_test extends \advanced_testcase { $this->assertEquals(qtype_numerical::UNITNONE, $options['unitdisplay']); $this->assertEmpty($options['unitsleft']); } + + /** + * Test that the grading of negative responses does not show false ERROR. + * + * @return void + * @throws \coding_exception + * @throws \dml_exception + */ + public function test_grading_of_negative_responses(): void { + global $DB; + + $this->resetAfterTest(); + + $qtype = new qtype_calculated(); + + // Create a question. + $q = \test_question_maker::get_question_data('calculated', 'mult'); + $q->id = 99; + + // Add units for the question. The issue to test only applies if the answer contains a unit string. + $units = []; + $unit = new \stdClass(); + $unit->question = $q->id; + $unit->multiplier = 1.0; + $unit->unit = "cm"; + $units[] = $unit; + $DB->insert_records("question_numerical_units", $units); + + $qtypeobj = question_bank::get_qtype($qtype->name()); + $fakedata = ["a" => "5.7", "b" => "3.3"]; + + $result = $qtype->comment_on_datasetitems($qtypeobj, $q->id, $q->questiontext, $q->options->answers, $fakedata, 1); + + // Make sure "ERROR" is not part of the answers. + foreach ($result->stranswers as $answer) { + $this->assertFalse(strstr($answer, "ERROR"), "Assert that 'ERROR' is not part of the answer!"); + } + } }