Skip to content

Commit

Permalink
MDL-68149 qtype_match: correctly distinguish 0 and 0.0 answers
Browse files Browse the repository at this point in the history 
Also added a test for backup & restore (which was working).
  • Loading branch information
@timhunt
timhunt committed Mar 11, 2020
1 parent b060e74 commit 9440b54
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 4 deletions.
2 changes: 1 addition & 1 deletion question/engine/tests/helpers.php
View file
Expand Up @@ -409,7 +409,7 @@ public static function make_an_essay_question() {
* Add some standard overall feedback to a question. You need to use these
* specific feedback strings for the corresponding contains_..._feedback
* methods in {@link qbehaviour_walkthrough_test_base} to works.
* @param question_definition $q the question to add the feedback to.
* @param question_definition|stdClass $q the question to add the feedback to.
*/
public static function set_standard_combined_feedback_fields($q) {
$q->correctfeedback = self::STANDARD_OVERALL_CORRECT_FEEDBACK;
Expand Down
3 changes: 1 addition & 2 deletions question/type/match/questiontype.php
View file
Expand Up @@ -124,8 +124,7 @@ protected function initialise_question_instance(question_definition $question, $
$question->right = array();

foreach ($questiondata->options->subquestions as $matchsub) {
$ans = $matchsub->answertext;
$key = array_search($matchsub->answertext, $question->choices);
$key = array_search($matchsub->answertext, $question->choices, true);
if ($key === false) {
$key = $matchsub->id;
$question->choices[$key] = $matchsub->answertext;
Expand Down
87 changes: 87 additions & 0 deletions question/type/match/tests/backup_test.php
View file
@@ -0,0 +1,87 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* Tests for the matching question type backup and restore logic.
*
* @package qtype_match
* @copyright 2020 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later.
*/

defined('MOODLE_INTERNAL') || die();

global $CFG;
require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
require_once($CFG->dirroot . '/course/externallib.php');


/**
* Tests for the matching question type backup and restore logic.
*/
class qtype_match_backup_testcase extends advanced_testcase {

/**
* Duplicate quiz with a matching question, and check it worked.
*/
public function test_duplicate_match_question() {
global $DB;
$this->resetAfterTest();
$this->setAdminUser();

$coregenerator = $this->getDataGenerator();
$questiongenerator = $coregenerator->get_plugin_generator('core_question');

// Create a course with a page that embeds a question.
$course = $coregenerator->create_course();
$quiz = $coregenerator->create_module('quiz', ['course' => $course->id]);
$quizcontext = context_module::instance($quiz->cmid);

$cat = $questiongenerator->create_question_category(['contextid' => $quizcontext->id]);
$question = $questiongenerator->create_question('match', 'trickynums', ['category' => $cat->id]);

// Store some counts.
$numquizzes = count(get_fast_modinfo($course)->instances['quiz']);
$nummatchquestions = $DB->count_records('question', ['qtype' => 'match']);

// Duplicate the page.
duplicate_module($course, get_fast_modinfo($course)->get_cm($quiz->cmid));

// Verify the copied quiz exists.
$this->assertCount($numquizzes + 1, get_fast_modinfo($course)->instances['quiz']);

// Verify the copied question.
$this->assertEquals($nummatchquestions + 1, $DB->count_records('question', ['qtype' => 'match']));
$newmatchid = $DB->get_field_sql("
SELECT MAX(id)
FROM {question}
WHERE qtype = ?
", ['match']);
$matchdata = question_bank::load_question_data($newmatchid);

$subquestions = array_values($matchdata->options->subquestions);

$this->assertSame('System.out.println(0);', $subquestions[0]->questiontext);
$this->assertSame('0', $subquestions[0]->answertext);

$this->assertSame('System.out.println(0.0);', $subquestions[1]->questiontext);
$this->assertSame('0.0', $subquestions[1]->answertext);

$this->assertSame('', $subquestions[2]->questiontext);
$this->assertSame('NULL', $subquestions[2]->answertext);
}
}
115 changes: 114 additions & 1 deletion question/type/match/tests/helper.php
View file
Expand Up @@ -37,7 +37,7 @@
*/
class qtype_match_test_helper extends question_test_helper {
public function get_test_questions() {
return array('foursubq');
return array('foursubq', 'trickynums');
}

/**
Expand Down Expand Up @@ -159,4 +159,117 @@ public static function make_match_question_foursubq() {
return $match;
}

/**
* Makes a matching question with choices including '0' and '0.0'.
*
* @return object the question definition data, as it might be returned from
* get_question_options.
*/
public function get_match_question_data_trickynums() {
global $USER;

$q = new stdClass();
test_question_maker::initialise_question_data($q);
$q->name = 'Java matching';
$q->qtype = 'match';
$q->parent = 0;
$q->questiontext = 'What is the output of each of these lines of code?';
$q->questiontextformat = FORMAT_HTML;
$q->generalfeedback = 'Java has some advantages over PHP I guess!';
$q->generalfeedbackformat = FORMAT_HTML;
$q->defaultmark = 1;
$q->penalty = 0.3333333;
$q->length = 1;
$q->hidden = 0;
$q->createdby = $USER->id;
$q->modifiedby = $USER->id;

$q->options = new stdClass();
$q->options->shuffleanswers = 1;
test_question_maker::set_standard_combined_feedback_fields($q->options);

$q->options->subquestions = array(
14 => (object) array(
'id' => 14,
'questiontext' => 'System.out.println(0);',
'questiontextformat' => FORMAT_HTML,
'answertext' => '0'),
15 => (object) array(
'id' => 15,
'questiontext' => 'System.out.println(0.0);',
'questiontextformat' => FORMAT_HTML,
'answertext' => '0.0'),
16 => (object) array(
'id' => 16,
'questiontext' => '',
'questiontextformat' => FORMAT_HTML,
'answertext' => 'NULL'),
);

return $q;
}

/**
* Makes a match question about completing two blanks in some text.
* @return object the question definition data, as it might be returned from
* the question editing form.
*/
public function get_match_question_form_data_trickynums() {
$q = new stdClass();
$q->name = 'Java matching';
$q->questiontext = ['text' => 'What is the output of each of these lines of code?', 'format' => FORMAT_HTML];
$q->generalfeedback = ['text' => 'Java has some advantages over PHP I guess!', 'format' => FORMAT_HTML];
$q->defaultmark = 1;
$q->penalty = 0.3333333;

$q->shuffleanswers = 1;
test_question_maker::set_standard_combined_feedback_form_data($q);

$q->subquestions = array(
0 => array('text' => 'System.out.println(0);', 'format' => FORMAT_HTML),
1 => array('text' => 'System.out.println(0.0);', 'format' => FORMAT_HTML),
2 => array('text' => '', 'format' => FORMAT_HTML),
);

$q->subanswers = array(
0 => '0',
1 => '0.0',
2 => 'NULL',
);

$q->noanswers = 3;

return $q;
}

/**
* Makes a matching question with choices including '0' and '0.0'.
*
* @return qtype_match_question
*/
public static function make_match_question_trickynums() {
question_bank::load_question_definition_classes('match');
$match = new qtype_match_question();
test_question_maker::initialise_a_question($match);
$match->name = 'Java matching';
$match->questiontext = 'What is the output of each of these lines of code?';
$match->generalfeedback = 'Java has some advantages over PHP I guess!';
$match->qtype = question_bank::get_qtype('match');

$match->shufflestems = 1;

test_question_maker::set_standard_combined_feedback_fields($match);

// Using unset to get 1-based arrays.
$match->stems = array('', 'System.out.println(0);', 'System.out.println(0.0);');
$match->stemformat = array('', FORMAT_HTML, FORMAT_HTML);
$match->choices = array('', '0', '0.0', 'NULL');
$match->right = array('', 1, 2);
unset($match->stems[0]);
unset($match->stemformat[0]);
unset($match->choices[0]);
unset($match->right[0]);

return $match;
}
}
20 changes: 20 additions & 0 deletions question/type/match/tests/questiontype_test.php
View file
Expand Up @@ -112,6 +112,26 @@ public function test_can_analyse_responses() {
$this->assertTrue($this->qtype->can_analyse_responses());
}

public function test_make_question_instance() {
$questiondata = test_question_maker::get_question_data('match', 'trickynums');
$question = question_bank::make_question($questiondata);
$this->assertEquals($questiondata->name, $question->name);
$this->assertEquals($questiondata->questiontext, $question->questiontext);
$this->assertEquals($questiondata->questiontextformat, $question->questiontextformat);
$this->assertEquals($questiondata->generalfeedback, $question->generalfeedback);
$this->assertEquals($questiondata->generalfeedbackformat, $question->generalfeedbackformat);
$this->assertInstanceOf('qtype_match', $question->qtype);
$this->assertEquals($questiondata->options->shuffleanswers, $question->shufflestems);

$this->assertEquals(
[14 => 'System.out.println(0);', 15 => 'System.out.println(0.0);'],
$question->stems);

$this->assertEquals([14 => '0', 15 => '0.0', 16 => 'NULL'], $question->choices);

$this->assertEquals([14 => 14, 15 => 15], $question->right);
}

public function test_get_random_guess_score() {
$q = $this->get_test_question_data();
$this->assertEquals(0.3333333, $this->qtype->get_random_guess_score($q), '', 0.0000001);
Expand Down

0 comments on commit 9440b54

Please sign in to comment.