From 0452275566e1f9689734dba54c49831b6e19036c Mon Sep 17 00:00:00 2001 From: Philipp Imhof <52650214+PhilippImhof@users.noreply.github.com> Date: Sun, 25 May 2025 09:54:33 +0200 Subject: [PATCH 1/2] increase test coverage --- tests/backup_restore_test.php | 108 +++++++++++++++++++++- tests/edit_formulas_form_test.php | 149 ++++++++++++++++++++++++++++++ tests/evaluator_test.php | 4 + 3 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 tests/edit_formulas_form_test.php diff --git a/tests/backup_restore_test.php b/tests/backup_restore_test.php index 1908b0cc..60230b2f 100644 --- a/tests/backup_restore_test.php +++ b/tests/backup_restore_test.php @@ -125,7 +125,6 @@ public function test_backup_and_restore(string $questionname): void { // Create a new course and restore the backup. $newcourse = $generator->create_course(); - $context = context_course::instance($newcourse->id); $rc = new restore_controller( $backupid, $newcourse->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, $USER->id, backup::TARGET_NEW_COURSE ); @@ -361,6 +360,113 @@ public function test_restore_quiz_if_field_is_missing_in_backup(string $which): $this->assertEquals($quizstructure[1]->questionid, $restoredquizstructure[1]->questionid); } + public function test_restore_of_legacy_backup_with_missing_fields(): void { + global $DB, $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course->id, 'editingteacher'); + $coursecontext = \context_course::instance($course->id); + /** @var \core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a quiz with a multipart Formulas question. + $quiz = $this->create_test_quiz($course); + $question = $questiongenerator->create_question('formulas', 'testmethodsinparts', ['category' => $cat->id]); + quiz_add_quiz_question($question->id, $quiz, 0); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Delete requested entry from questions.xml file in the backup. + $xmlfile = $bc->get_plan()->get_basepath() . '/questions.xml'; + $xml = file_get_contents($xmlfile); + + // List of fields that can be missing from older backups. We use the default + // value that will be assigned in restore_qtype_formulas_plugin.class.php. + $fieldstoremove = [ + 'answernotunique' => '1', + 'shownumcorrect' => 0, + 'answernumbering' => 'none', + 'feedbackformat' => FORMAT_HTML, + 'partindex' => null, + 'correctfeedback' => '', + 'partiallycorrectfeedback' => '', + 'incorrectfeedback' => '', + 'partcorrectfb' => '', + 'partpartiallycorrectfb' => '', + 'partincorrectfb' => '', + ]; + // Remove these fields from the backup file. + foreach (array_keys($fieldstoremove) as $field) { + $xml = preg_replace("#<$field( format=\"html\")?>[^<]+#", '', $xml); + } + file_put_contents($xmlfile, $xml); + + // Delete the current course to make sure there is no data. + delete_course($course, false); + + // Create a new course and restore the backup. + $newcourse = $generator->create_course(); + $rc = new restore_controller( + $backupid, $newcourse->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, $USER->id, backup::TARGET_NEW_COURSE + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Fetch the quiz and question ID. + $modules = get_fast_modinfo($newcourse->id)->get_instances_of('quiz'); + $quiz = reset($modules); + $structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz->instance, $quiz->context); + $question = reset($structure); + + // Fetch the question and its additional data (random vars, global vars, parts) from the DB. + $questionrecord = $DB->get_record('question', ['id' => $question->questionid], '*', MUST_EXIST); + $qtype = new qtype_formulas(); + $qtype->get_question_options($questionrecord); + + // Check whether the previously removed fields have been populated with their + // respective default values. + foreach ($fieldstoremove as $field => $defaultvalue) { + // The fields 'shownumcorrect', 'answernumbering' and '...feedback' (combined + // feedback) are stored at the options level. + if ($field == 'shownumcorrect' || $field == 'answernumbering' || preg_match('/feedback$/', $field)) { + self::assertEquals($defaultvalue, $questionrecord->options->$field, $field); + if (strstr($field, 'num') === false) { + $format = $field . 'format'; + self::assertEquals(FORMAT_HTML, $questionrecord->options->$format, $format); + } + } else { + // All other fields are stored at the answers level. We check all parts. The partindex + // will be assigned according to the appearance. For the other fields, we use the default + // value. The combined feedback fields are stored as part....fb. + foreach ($questionrecord->options->answers as $i => $answer) { + if ($field === 'partindex') { + self::assertEquals($i, $answer->$field, $field); + } else { + self::assertEquals($defaultvalue, $answer->$field, $field); + } + if (strstr($field, 'fb') !== false) { + $format = $field . 'format'; + self::assertEquals(FORMAT_HTML, $answer->$format, $format); + } + } + } + } + } + /** * Restore a quiz with duplicate questions (same stamp and questions) into the same course. * This is a contrived case, but this test serves as a control for the other tests in this class, proving diff --git a/tests/edit_formulas_form_test.php b/tests/edit_formulas_form_test.php new file mode 100644 index 00000000..28b29d59 --- /dev/null +++ b/tests/edit_formulas_form_test.php @@ -0,0 +1,149 @@ +. + +/** + * Unit tests for (some of) question/type/formulas/edit_formulas_form.php. + * + * @package qtype_formulas + * @copyright 2025 Philipp Imhof + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace qtype_formulas; + +use qtype_formulas_edit_form; +use qtype_formulas; +use qtype_formulas_question; +use qtype_formulas_test_helper; +use test_question_maker; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); +require_once($CFG->dirroot . '/question/type/formulas/questiontype.php'); +require_once($CFG->dirroot . '/question/type/formulas/tests/helper.php'); +require_once($CFG->dirroot . '/question/type/formulas/edit_formulas_form.php'); + +/** + * Unit tests for question/type/formulas/edit_formulas_form.php. + * + * @copyright 2025 Philipp Imhof + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \qtype_formulas_edit_form + */ +final class edit_formulas_form_test extends \advanced_testcase { + + /** @var formulas instance of the question type class to test. */ + protected $qtype; + + /** + * Create a question object of a certain type, as defined in the helper.php file. + * + * @param string|null $which the test question name + * @return qtype_formulas_question + */ + protected function get_test_formulas_question($which = null) { + return test_question_maker::make_question('formulas', $which); + } + + protected function setUp(): void { + $this->qtype = new qtype_formulas(); + + parent::setUp(); + } + + protected function tearDown(): void { + $this->qtype = null; + + parent::tearDown(); + } + + public function test_data_preprocessing(): void { + global $DB, $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course->id, 'editingteacher'); + $coursecontext = \context_course::instance($course->id); + $contexts = new \core_question\local\bank\question_edit_contexts($coursecontext); + /** @var \core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + $question = $questiongenerator->create_question('formulas', 'testmethodsinparts', ['category' => $category->id]); + $questionrecord = $DB->get_record('question', ['id' => $question->id], '*', MUST_EXIST); + $this->qtype->get_question_options($questionrecord); + + $questionrecord->formoptions = new \stdClass(); + $questionrecord->formoptions->canedit = true; + $questionrecord->formoptions->canmove = true; + $questionrecord->formoptions->cansaveasnew = true; + $questionrecord->formoptions->repeatelements = true; + $questionrecord->beingcopied = false; + + $form = $this->qtype->create_editing_form('question.php', $questionrecord, $category, $contexts, true); + + // Use reflection to access protected method. + $method = new \ReflectionMethod($form, 'data_preprocessing'); + $method->setAccessible(true); + $processedquestion = $method->invoke($form, $questionrecord); + + $helper = new qtype_formulas_test_helper(); + $formdata = $helper->get_formulas_question_form_data_testmethodsinparts(); + + // First, we want to make sure that the ruleid and unitpenalty values are moved from the parts (where they) + // are stored in the DB, to the global form fields. + $globalfields = ['globalruleid', 'globalunitpenalty']; + foreach ($globalfields as $field) { + self::assertObjectHasProperty($field, $processedquestion); + self::assertEquals($formdata->$field, $processedquestion->$field); + + } + + // Now, check the per-part fields, with exception of the unitpenalty and ruleid mentioned above. + $numparts = count($questionrecord->options->answers); + foreach ($this->qtype::PART_BASIC_FIELDS as $field) { + if ($field === 'unitpenalty' || $field === 'ruleid') { + continue; + } + for ($i = 0; $i < $numparts; $i++) { + $formfieldname = $field . "[{$i}]"; + self::assertEquals($formdata->{$field}[$i], $processedquestion->$formfieldname); + } + } + + // Finally, check the textual fields, i. e. subqtext, general feedback and combined feedback. + // They all have a text, a format and an item ID. For the item ID, we just check it is there. + $textfields = ['subqtext', 'feedback', 'partcorrectfb', 'partpartiallycorrectfb', 'partincorrectfb']; + foreach ($textfields as $field) { + for ($i = 0; $i < $numparts; $i++) { + $formfieldname = $field . "[{$i}]"; + self::assertEquals($formdata->{$field}[$i]['text'], $processedquestion->{$formfieldname}['text']); + self::assertEquals($formdata->{$field}[$i]['format'], $processedquestion->{$formfieldname}['format']); + self::assertArrayHasKey('itemid', $processedquestion->$formfieldname); + } + } + } + + + +} diff --git a/tests/evaluator_test.php b/tests/evaluator_test.php index e15334cc..369e4f9e 100644 --- a/tests/evaluator_test.php +++ b/tests/evaluator_test.php @@ -1630,6 +1630,10 @@ public static function provide_invalid_assignments(): array { 'Scalar value expected, found list.', 'a = "a" + [1, 2, 3]', ], + 'invalid lb' => [ + 'lb() expects its argument to be a positive number.', + 'a = lb(-5)', + ], 'invalid 0^0' => [ 'Power 0^0 is not defined.', 'a = 0 ** 0', From f37bbe04dc1da86e7be4a824ee50aaa256d87586 Mon Sep 17 00:00:00 2001 From: Philipp Imhof <52650214+PhilippImhof@users.noreply.github.com> Date: Sun, 25 May 2025 10:26:58 +0200 Subject: [PATCH 2/2] compatibility with PHPUnit 9.5 --- tests/edit_formulas_form_test.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/edit_formulas_form_test.php b/tests/edit_formulas_form_test.php index 28b29d59..a77b6f97 100644 --- a/tests/edit_formulas_form_test.php +++ b/tests/edit_formulas_form_test.php @@ -114,7 +114,12 @@ public function test_data_preprocessing(): void { // are stored in the DB, to the global form fields. $globalfields = ['globalruleid', 'globalunitpenalty']; foreach ($globalfields as $field) { - self::assertObjectHasProperty($field, $processedquestion); + // For backwards compatibility with PHPUnit 9.5, used in Moodle 4.1 and 4.2. + if (method_exists(__CLASS__, 'assertObjectHasProperty')) { + self::assertObjectHasProperty($field, $processedquestion); + } else { + self::assertObjectHasAttribute($field, $processedquestion); + } self::assertEquals($formdata->$field, $processedquestion->$field); }