From ff82354abcd06177ee7866a586aa149eab468bbc Mon Sep 17 00:00:00 2001 From: mkassaei Date: Mon, 16 Oct 2023 16:23:00 +0100 Subject: [PATCH] PHPUNIT tests and some tidying up --- edit_oumatrix_form.php | 15 ++- questiontype.php | 89 +++++++++----- tests/edit_oumatrix_form_test.php | 188 ++++++++++++++++++++++++++++++ tests/helper.php | 26 +++-- tests/questiontype_test.php | 26 ++--- 5 files changed, 285 insertions(+), 59 deletions(-) create mode 100644 tests/edit_oumatrix_form_test.php diff --git a/edit_oumatrix_form.php b/edit_oumatrix_form.php index 18dc403..9fe9a95 100644 --- a/edit_oumatrix_form.php +++ b/edit_oumatrix_form.php @@ -68,7 +68,7 @@ protected function definition_inner($mform) { 'multiple' => get_string('answermodemultiple', 'qtype_oumatrix'), ]; $mform->addElement('select', 'inputtype', get_string('answermode', 'qtype_oumatrix'), $answermodemenu); - $mform->setDefault('inputtype', $this->get_default_value('single', + $mform->setDefault('inputtype', $this->get_default_value('inputtype', get_config('qtype_oumatrix', 'inputtype'))); $grademethod = [ @@ -77,14 +77,14 @@ protected function definition_inner($mform) { ]; $mform->addElement('select', 'grademethod', get_string('grademethod', 'qtype_oumatrix'), $grademethod); $mform->addHelpButton('grademethod', 'grademethod', 'qtype_oumatrix'); - $mform->setDefault('grademethod', $this->get_default_value( - 'grademethod', get_config('qtype_oumatrix', 'grademethod'))); + $mform->setDefault('grademethod', $this->get_default_value('grademethod', + get_config('qtype_oumatrix', 'grademethod'))); $mform->disabledIf('grademethod', 'inputtype', 'eq', 'single'); $mform->addElement('selectyesno', 'shuffleanswers', get_string('shuffleanswers', 'qtype_oumatrix')); $mform->addHelpButton('shuffleanswers', 'shuffleanswers', 'qtype_oumatrix'); - $mform->setDefault('shuffleanswers', $this->get_default_value( - 'shuffleanswers', get_config('qtype_oumatrix', 'shuffleanswers'))); + $mform->setDefault('shuffleanswers', $this->get_default_value('shuffleanswers', + get_config('qtype_oumatrix', 'shuffleanswers'))); // Add update field. $mform->addElement('submit', 'updateform', get_string('updateform', 'qtype_oumatrix')); @@ -242,8 +242,7 @@ private function data_preprocessing_rows($question) { public function validation($data, $files) { $errors = parent::validation($data, $files); - // Validate required number of min and max columns. - // Ignore the blank columns. + // Validate minimum required number of columns. $filteredcolscount = count(array_filter($data['columnname'])); if ($filteredcolscount < column::MIN_NUMBER_OF_COLUMNS) { $errors['columnname[' . $filteredcolscount . ']'] = get_string('notenoughanswercols', 'qtype_oumatrix', @@ -281,7 +280,7 @@ public function validation($data, $files) { } } - // Validate required number of min and max rows. + // Validate minimum required number of rows. $countrows = count(array_filter($data['rowname'])); if ($countrows < row::MIN_NUMBER_OF_ROWS) { $errors['rowoptions[' . $countrows . ']'] = get_string('notenoughquestionrows', 'qtype_oumatrix', diff --git a/questiontype.php b/questiontype.php index 29bcaaa..ff6f899 100644 --- a/questiontype.php +++ b/questiontype.php @@ -58,7 +58,7 @@ public function get_question_options($question) { /** * Create a default options object for the provided question. * - * @param object $question The queston we are working with. + * @param object $question The question we are working with. * @return object The options object. */ protected function create_default_options($question) { @@ -89,11 +89,6 @@ public function save_defaults_for_new_questions(stdClass $fromform): void { $this->set_default_value('shownumcorrect', $fromform->shownumcorrect); } - public function save_question($question, $form) { - $question = parent::save_question($question, $form); - return $question; - } - public function save_question_options($question) { global $DB; $context = $question->context; @@ -124,11 +119,12 @@ public function save_question_options($question) { } /** + * Save the question columns and return a list of columns to be used in the save_rows function. * * @param object $question This holds the information from the editing form. * @return array The list of columns created. */ - public function save_columns($question) { + public function save_columns(object $question): array { global $DB; $numcolumns = count($question->columnname); $columnslist = []; @@ -146,14 +142,14 @@ public function save_columns($question) { } /** + * Save the question rows. * * @param object $question This holds the information from the editing form * @param array $columnslist */ - public function save_rows($question, $columnslist) { + public function save_rows(object $question, array $columnslist) { global $DB; $context = $question->context; - $result = new stdClass(); $numrows = count($question->rowname); // Insert row input data. @@ -186,7 +182,7 @@ public function save_rows($question, $columnslist) { if ($question->feedback[$i]['text'] != '') { $questionrow->feedback = $this->import_or_save_files($question->feedback[$i], - $context, 'qtype_oumatrix', 'feedback', $questionrow->id); + $context, 'qtype_oumatrix', 'feedback', $questionrow->id); $questionrow->feedbackformat = $question->feedback[$i]['format']; $DB->update_record('qtype_oumatrix_rows', $questionrow); @@ -222,19 +218,30 @@ public function delete_question($questionid, $contextid) { parent::delete_question($questionid, $contextid); } - protected function get_num_correct_choices($questiondata) { + public function get_num_correct_choices($questiondata) { $numright = 0; foreach ($questiondata->rows as $row) { $rowanwers = json_decode($row->correctanswers); - foreach ($rowanwers as $key => $value) { - if ((int) $value === 1) { - $numright += 1; - } - } + $numright += count((array)$rowanwers); } return $numright; } + /** + * Return total number if choices for both (single, multiple) matrix choices. + * @param object $questiondata + * @return int + */ + public function get_total_number_of_choices(object $questiondata):? int { + // if rows or columns are not set return null; + if (sizeof($questiondata->columns) === 0 || sizeof($questiondata->rows) === 0) { + return null; + } + // Total number of choices for each row is the number of columns, + // therefore the total number of choices for the question is + return count($questiondata->columns) * count($questiondata->rows); + } + public function get_random_guess_score($questiondata) { // We compute the randome guess score here on the assumption we are using // the deferred feedback behaviour, and the question text tells the @@ -242,9 +249,36 @@ public function get_random_guess_score($questiondata) { // Amazingly, the forumla for this works out to be // # correct choices / total # choices in all cases. - //TODO: improve this. - return $this->get_num_correct_choices($questiondata) / - count($questiondata->rows); + if ($this->get_total_number_of_choices($questiondata) === null) { + return null; + } + return $this->get_num_correct_choices($questiondata) / $this->get_total_number_of_choices($questiondata); + } + + public function get_possible_responses($questiondata) { + if ($questiondata->options->single) { + $responses = array(); + + // TODO: Sort out this funtion to work with rows and columns, etc. + foreach ($questiondata->options->answers as $aid => $answer) { + $responses[$aid] = new question_possible_response( + question_utils::to_plain_text($answer->answer, $answer->answerformat), + $answer->fraction); + } + + $responses[null] = question_possible_response::no_response(); + return array($questiondata->id => $responses); + } else { + $parts = array(); + + foreach ($questiondata->options->answers as $aid => $answer) { + $parts[$aid] = array($aid => new question_possible_response( + question_utils::to_plain_text($answer->answer, $answer->answerformat), + $answer->fraction)); + } + + return $parts; + } } /** @@ -296,7 +330,7 @@ public function make_row($rowdata) { explode(',', $rowdata->correctanswers), $rowdata->feedback, $rowdata->feedbackformat); } - public function import_from_xml($data, $question, qformat_xml $format, $extra = null) { + public function import_from_xml($data, $question, qformat_xml $format, $extra = null): object { if (!isset($data['@']['type']) || $data['@']['type'] != 'oumatrix') { return false; } @@ -337,7 +371,7 @@ public function import_from_xml($data, $question, qformat_xml $format, $extra = return $question; } - public function import_columns(qformat_xml $format, stdClass $question, array $columns): void { + public function import_columns(qformat_xml $format, stdClass $question, array $columns) { foreach ($columns as $column) { static $indexno = 0; $question->columns[$indexno]['name'] = @@ -350,7 +384,7 @@ public function import_columns(qformat_xml $format, stdClass $question, array $c } } - public function import_rows(qformat_xml $format, stdClass $question, array $rows): void { + public function import_rows(qformat_xml $format, stdClass $question, array $rows) { foreach ($rows as $row) { static $indexno = 0; $question->rows[$indexno]['id'] = $format->getpath($row, ['@', 'key'], $indexno); @@ -448,7 +482,7 @@ public function move_files($questionid, $oldcontextid, $newcontextid) { $fs = get_file_storage(); parent::move_files($questionid, $oldcontextid, $newcontextid); - $this->move_files_in_rowanswers($questionid, $oldcontextid, $newcontextid); + $this->move_files_in_row_feedback($questionid, $oldcontextid, $newcontextid); $this->move_files_in_hints($questionid, $oldcontextid, $newcontextid); $fs->move_area_files_to_new_context($oldcontextid, @@ -460,15 +494,14 @@ public function move_files($questionid, $oldcontextid, $newcontextid) { } /** - * Move all the files belonging to each rows question answers when the question - * is moved from one context to another. + * Move all the feedback files belonging to each sub-question + * when the question is moved from one context to another. * * @param int $questionid the question being moved. * @param int $oldcontextid the context it is moving from. * @param int $newcontextid the context it is moving to. */ - protected function move_files_in_rowanswers($questionid, $oldcontextid, - $newcontextid) { + protected function move_files_in_row_feedback(int $questionid, int $oldcontextid, int $newcontextid) { global $DB; $fs = get_file_storage(); @@ -497,7 +530,7 @@ protected function delete_files($questionid, $contextid) { * @param int $questionid the question being deleted. * @param int $contextid the context the question is in. */ - protected function delete_files_in_row_feedback($questionid, $contextid) { + protected function delete_files_in_row_feedback(int $questionid, int $contextid) { global $DB; $fs = get_file_storage(); diff --git a/tests/edit_oumatrix_form_test.php b/tests/edit_oumatrix_form_test.php new file mode 100644 index 0000000..5906e1b --- /dev/null +++ b/tests/edit_oumatrix_form_test.php @@ -0,0 +1,188 @@ +. + +namespace qtype_oumatrix; + +use stdClass; +use qtype_oumatrix; +use qtype_oumatrix_edit_form; +use question_possible_response; +use qtype_oumatrix_test_helper; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); +require_once($CFG->dirroot . '/question/type/oumatrix/tests/helper.php'); +require_once($CFG->dirroot . '/question/type/oumatrix/questiontype.php'); +require_once($CFG->dirroot . '/question/type/edit_question_form.php'); +require_once($CFG->dirroot . '/question/type/oumatrix/edit_oumatrix_form.php'); + +/** + * Unit tests for the OU matrix question definition class. + * + * @package qtype_oumatrix + * @copyright 2023 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_oumatrix + */ + +/** + * Unit tests for oumatrix question edit form. + * + * @package qtype_oumatrix + * @copyright 2023 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class edit_oumatrix_form_test extends \advanced_testcase { + + /** + * Helper method. + * + * @param string $classname the question form class to instantiate. + * + * @return array with two elements: + * question_edit_form great a question form instance that can be tested. + * stdClass the question category. + */ + protected function get_form($classname) { + $this->setAdminUser(); + $this->resetAfterTest(); + $syscontext = \context_system::instance(); + $category = question_make_default_categories(array($syscontext)); + $fakequestion = new stdClass(); + $fakequestion->qtype = 'oumatrix'; + $fakequestion->contextid = $syscontext->id; + $fakequestion->createdby = 2; + $fakequestion->category = $category->id; + $fakequestion->questiontext = 'Animal classification. Please answer the sub questions in all 4 rows.'; + $fakequestion->options = new stdClass(); + $fakequestion->options->answers = array(); + $fakequestion->formoptions = new stdClass(); + $fakequestion->formoptions->movecontext = null; + $fakequestion->formoptions->repeatelements = true; + $fakequestion->inputs = null; + + $form = new $classname(new \moodle_url('/'), $fakequestion, $category, + new \core_question\local\bank\question_edit_contexts($syscontext)); + + return [$form, $category]; + } + + /** + * Test the form correctly validates minimum requirements of rows and columns. + */ + public function test_validation_cols_rows_minimum() { + [$form, $category] = $this->get_form('qtype_oumatrix_edit_form'); + $helper = new qtype_oumatrix_test_helper(); + $formdata = $helper->get_test_question_form_data('animals_single'); + $formdata['category'] = $category->id; + + // Minumum number of columns. + $testdata = $formdata; + $testdata['columnname'][1] = ''; + $testdata['columnname'][2] = ''; + $testdata['columnname'][3] = ''; + $expected = get_string('notenoughanswercols', 'qtype_oumatrix', column::MIN_NUMBER_OF_COLUMNS); + $errors = $form->validation($testdata, []); + $this->assertArrayNotHasKey('columnname[0]', $errors); + $this->assertArrayHasKey('columnname[1]', $errors); + $this->assertArrayNotHasKey('columnname[2]', $errors); + $this->assertArrayNotHasKey('columnname[3]', $errors); + $this->assertEquals($expected, $errors['columnname[1]']); + + // Minumum number of rows. + $testdata = $formdata; + $testdata['rowname'][1] = ''; + $testdata['rowname'][2] = ''; + $testdata['rowname'][3] = ''; + $errors = $form->validation($testdata, []); + $expected = get_string('notenoughquestionrows', 'qtype_oumatrix', row::MIN_NUMBER_OF_ROWS); + $this->assertEquals($expected, $errors['rowoptions[1]']); + } + + /** + * Test the form correctly validates duplicates of rows and columns. + */ + public function test_validation_cols_rows_duplicates() { + [$form, $category] = $this->get_form('qtype_oumatrix_edit_form'); + $helper = new qtype_oumatrix_test_helper(); + $formdata = $helper->get_test_question_form_data('animals_single'); + $formdata['category'] = $category->id; + + // Duplicate a column name. + $testdata = $formdata; + $testdata['columnname'][1] = 'Insects'; + $expected = get_string('duplicates', 'qtype_oumatrix', 'Insects'); + $errors = $form->validation($testdata, []); + $this->assertArrayNotHasKey('columnname[0]', $errors); + $this->assertArrayHasKey('columnname[1]', $errors); + $this->assertArrayNotHasKey('columnname[2]', $errors); + $this->assertArrayNotHasKey('columnname[3]', $errors); + $this->assertEquals($expected, $errors['columnname[1]']); + + // Duplicate a column name. + $testdata = $formdata; + $testdata['rowname'][1] = 'Bee'; + $errors = $form->validation($testdata, []); + $expected = get_string('duplicates', 'qtype_oumatrix', 'Bee'); + $this->assertEquals($expected, $errors['rowoptions[1]']); + } + + /** + * Test the form correctly validates if there are empty columns in between. + */ + public function test_validation_column_names_empty() { + [$form, $category] = $this->get_form('qtype_oumatrix_edit_form'); + $helper = new qtype_oumatrix_test_helper(); + $formdata = $helper->get_test_question_form_data('animals_single'); + $formdata['category'] = $category->id; + + // Empty columns names (second and third columns are empty). + $testdata = $formdata; + $testdata['columnname'][1] = ''; + $testdata['columnname'][2] = ''; + $errors = $form->validation($testdata, []); + $this->assertArrayNotHasKey('columnname[0]', $errors); + $this->assertArrayHasKey('columnname[1]', $errors); + $this->assertArrayHasKey('columnname[2]', $errors); + $this->assertArrayNotHasKey('columnname[3]', $errors); + $expectedcol1 = $errors['columnname[1]'] = get_string('blankcolumnsnotallowed', 'qtype_oumatrix'); + $expectedcol2 = $errors['columnname[2]'] = get_string('blankcolumnsnotallowed', 'qtype_oumatrix'); + $this->assertEquals($expectedcol1, $errors['columnname[1]']); + $this->assertEquals($expectedcol2, $errors['columnname[2]']); + } + + /** + * Test the form correctly validates if correct answers have been input. + */ + public function test_validation_rowanswers() { + [$form, $category] = $this->get_form('qtype_oumatrix_edit_form'); + $helper = new qtype_oumatrix_test_helper(); + $formdata = $helper->get_test_question_form_data('animals_single'); + $formdata['category'] = $category->id; + + // Rows without chosen answer(s) are not valid. + $testdata = $formdata; + $testdata['rowanswers'][1] = '0'; + $testdata['rowanswers'][2] = '0'; + $errors = $form->validation($testdata, []); + $expectedanswer1 = $errors['rowoptions[1]'] = get_string('noinputanswer', 'qtype_oumatrix'); + $expectedanswer2 = $errors['rowoptions[2]'] = get_string('noinputanswer', 'qtype_oumatrix'); + $this->assertEquals($expectedanswer1, $errors['rowoptions[1]']); + $this->assertEquals($expectedanswer2, $errors['rowoptions[2]']); + } +} diff --git a/tests/helper.php b/tests/helper.php index a171a37..4ea171e 100644 --- a/tests/helper.php +++ b/tests/helper.php @@ -96,7 +96,7 @@ public function get_oumatrix_question_data_animals_single() { 'name' => 'Bee', 'feedback' => 'Fly, Bee and spider are insects.', 'feedbackformat' => FORMAT_HTML, - 'correctanswers' => '{"Insects":"1"}', + 'correctanswers' => '{"11":"1"}', ], 12 => (object) [ 'id' => 12, @@ -104,7 +104,7 @@ public function get_oumatrix_question_data_animals_single() { 'name' => 'Salmon', 'feedback' => 'Cod, Salmon and Trout are fish.', 'feedbackformat' => FORMAT_HTML, - 'correctanswers' => '{"Fish":"1"}', + 'correctanswers' => '{"12":"1"}', ], 13 => (object) [ 'id' => 13, @@ -112,7 +112,7 @@ public function get_oumatrix_question_data_animals_single() { 'name' => 'Seagull', 'feedback' => 'Gulls and Owls are birds.', 'feedbackformat' => FORMAT_HTML, - 'correctanswers' => '{"Birds":"1"}', + 'correctanswers' => '{"13":"1"}', ], 14 => (object) [ 'id' => 14, @@ -120,7 +120,7 @@ public function get_oumatrix_question_data_animals_single() { 'name' => 'Dog', 'feedback' => 'Cow, Dog and Horse are mammals.', 'feedbackformat' => FORMAT_HTML, - 'correctanswers' => '{"Mammals":"1"}', + 'correctanswers' => '{"14":"1"}', ], ]; @@ -301,7 +301,7 @@ public function get_oumatrix_question_data_food_multiple() { 'id' => 21, 'number' => 0, 'name' => 'Proteins', - 'correctanswers' => '{"Chicken breast":"1","Salmon fillet":"1","Steak":"1"}', + 'correctanswers' => '{"21":"1","23":"1","26":"1"}', 'feedback' => 'Chicken, fish and red meat containing proteins.', 'feedbackformat' => FORMAT_HTML, ], @@ -309,7 +309,7 @@ public function get_oumatrix_question_data_food_multiple() { 'id' => 22, 'number' => 1, 'name' => 'Vegetables', - 'correctanswers' => '{"Carrot":"1","Asparagus":"1","Potato":"1"}', + 'correctanswers' => '{"22":"1","24":"1","27":"1"}', 'feedback' => 'Carrot, Asparagus, Potato are vegetables.', 'feedbackformat' => FORMAT_HTML, ], @@ -317,7 +317,7 @@ public function get_oumatrix_question_data_food_multiple() { 'id' => 23, 'number' => 2, 'name' => 'Fats', - 'correctanswers' => '{"Olive oil":"1"}', + 'correctanswers' => '{"25":"1"}', 'feedback' => 'Olive oil contains fat.', 'feedbackformat' => FORMAT_HTML, ], @@ -434,7 +434,15 @@ public static function get_oumatrix_question_form_data_food_multiple() { return $qfdata; } - public function get_test_question_data($witch) { - return \test_question_maker::get_question_data('oumatrix', $witch); + public function get_test_question_data($which) { + return \test_question_maker::get_question_data('oumatrix', $which); + } + + public function get_test_question_form_data($which) { + return (array)\test_question_maker::get_question_form_data('oumatrix', $which); + } + + public function get_test_question_form_data($witch) { + return (array)\test_question_maker::get_question_form_data('oumatrix', $witch); } } diff --git a/tests/questiontype_test.php b/tests/questiontype_test.php index 0d39a24..9a48c5b 100644 --- a/tests/questiontype_test.php +++ b/tests/questiontype_test.php @@ -55,7 +55,7 @@ public function test_name() { public function test_initialise_question_instance() { $h = new qtype_oumatrix_test_helper(); - $qdata = $h->get_oumatrix_question_data_animals_single(); + $qdata = $h->get_test_question_data('animals_single'); $expected = $h->get_test_question_data('animals_single'); $this->assertEquals(0.5, $this->qtype->get_random_guess_score($qdata)); @@ -73,22 +73,20 @@ public function test_initialise_question_instance() { } public function test_get_random_guess_score() { - $h = new qtype_oumatrix_test_helper(); - - $qsingle = $h->get_oumatrix_question_data_animals_single(); - $this->assertEquals(0.5, $this->qtype->get_random_guess_score($qsingle)); - - $qmultiple = $h->get_oumatrix_question_data_oumatrix_multiple(); - $this->assertEquals(0.5, $this->qtype->get_random_guess_score($qmultiple)); + $helper = new qtype_oumatrix_test_helper(); + $qdata = $helper->get_test_question_data('animals_single'); + $expected = $this->qtype->get_num_correct_choices($qdata) / $this->qtype->get_total_number_of_choices($qdata); + $this->assertEquals($expected, $this->qtype->get_random_guess_score($qdata)); } public function test_get_random_guess_score_broken_question() { - $q = $this->get_test_question_data(); - $q->rows = []; + $helper = new qtype_oumatrix_test_helper(); + $q = $helper->get_test_question_data('animals_single'); + $q->columns = []; $this->assertNull($this->qtype->get_random_guess_score($q)); } - public static function get_save_question_which(): array { + public function get_save_question_which() { return [['animals_single'], ['oumatrix_multiple']]; } @@ -97,12 +95,12 @@ public static function get_save_question_which(): array { * @dataProvider get_save_question_which * @param $which */ - public function test_save_question($which) { + public function test_save_question() { $this->resetAfterTest(true); $this->setAdminUser(); - $questiondata = \test_question_maker::get_question_data('oumatrix', $which); - $formdata = \test_question_maker::get_question_form_data('oumatrix', $which); + $questiondata = \test_question_maker::get_question_data('oumatrix', 'animals_single'); + $formdata = \test_question_maker::get_question_form_data('oumatrix', 'animals_single'); $generator = $this->getDataGenerator()->get_plugin_generator('core_question');