From 0656c0a4033a4cd3d231e330d58ddccb568d0541 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 16 Jul 2012 14:30:12 +0100 Subject: [PATCH 1/2] MDL-34226 multichoice qtype: correct is_complete_response for multianswer The multianswer qtype uses the multi-choice one, and the way it does its responses array breaks an assumption that multi-choice was making. --- question/type/multichoice/question.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php index f31ceb6e443..afca6a47740 100644 --- a/question/type/multichoice/question.php +++ b/question/type/multichoice/question.php @@ -199,7 +199,7 @@ class qtype_multichoice_single_question extends qtype_multichoice_base { } public function is_complete_response(array $response) { - return array_key_exists('answer', $response); + return array_key_exists('answer', $response) && $response['answer'] !== ''; } public function is_gradable_response(array $response) { From 0288eb2151b017674a8978440e2e67c544130a35 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 16 Jul 2012 14:32:25 +0100 Subject: [PATCH 2/2] MDL-34226 multianswer qtype: basic unit test. This verifies the previous fix, and gets rid of a rather embarassing TODO. --- question/type/multianswer/tests/helper.php | 75 ++++++++++++++++++- .../multianswer/tests/walkthrough_test.php | 57 +++++--------- 2 files changed, 94 insertions(+), 38 deletions(-) diff --git a/question/type/multianswer/tests/helper.php b/question/type/multianswer/tests/helper.php index 90f78efb3f7..75a3d7dcf47 100644 --- a/question/type/multianswer/tests/helper.php +++ b/question/type/multianswer/tests/helper.php @@ -38,7 +38,7 @@ require_once($CFG->dirroot . '/question/type/multianswer/question.php'); */ class qtype_multianswer_test_helper extends question_test_helper { public function get_test_questions() { - return array('twosubq'); + return array('twosubq', 'fourmc'); } /** @@ -54,6 +54,14 @@ class qtype_multianswer_test_helper extends question_test_helper { 'Complete this opening line of verse: "The {#1} and the {#2} went to sea".'; $q->generalfeedback = 'General feedback: It\'s from "The Owl and the Pussy-cat" by Lear: ' . '"The owl and the pussycat went to sea'; + $q->qtype = question_bank::get_qtype('multianswer'); + + $q->textfragments = array( + 'Complete this opening line of verse: "The ', + ' and the ', + ' went to sea".', + ); + $q->places = array('1' => '1', '2' => '2'); // Shortanswer subquestion. question_bank::load_question_definition_classes('shortanswer'); @@ -214,4 +222,69 @@ class qtype_multianswer_test_helper extends question_test_helper { return $formdata; } + + /** + * Makes a multianswer question about completing two blanks in some text. + * @return qtype_multianswer_question + */ + public function make_multianswer_question_fourmc() { + question_bank::load_question_definition_classes('multianswer'); + $q = new qtype_multianswer_question(); + test_question_maker::initialise_a_question($q); + $q->name = 'Multianswer four multi-choice'; + $q->questiontext = '

Match the following cities with the correct state:

+ '; + $q->questiontextformat = FORMAT_HTML; + $q->generalfeedback = ''; + $q->qtype = question_bank::get_qtype('multianswer'); + + $q->textfragments = array('

Match the following cities with the correct state:

+ '); + $q->places = array('1' => '1', '2' => '2', '3' => '3', '4' => '4'); + + $subqdata = array( + 1 => array('qt' => '{1:MULTICHOICE:=California#OK~Arizona#Wrong}', 'California' => 'OK', 'Arizona' => 'Wrong'), + 2 => array('qt' => '{1:MULTICHOICE:%0%California#Wrong~=Arizona#OK}', 'California' => 'Wrong', 'Arizona' => 'OK'), + 3 => array('qt' => '{1:MULTICHOICE:=California#OK~Arizona#Wrong}', 'California' => 'OK', 'Arizona' => 'Wrong'), + 4 => array('qt' => '{1:MULTICHOICE:%0%California#Wrong~=Arizona#OK}', 'California' => 'Wrong', 'Arizona' => 'OK'), + ); + + foreach ($subqdata as $i => $data) { + // Multiple-choice subquestion. + question_bank::load_question_definition_classes('multichoice'); + $mc = new qtype_multichoice_single_question(); + test_question_maker::initialise_a_question($mc); + $mc->name = 'Multianswer four multi-choice'; + $mc->questiontext = $data['qt']; + $mc->questiontextformat = FORMAT_HTML; + $mc->generalfeedback = ''; + $mc->generalfeedbackformat = FORMAT_HTML; + + $mc->shuffleanswers = 0; // TODO this is a cheat to make the unit tests easier to write. + // In reality, multianswer questions always shuffle. + $mc->answernumbering = 'none'; + $mc->layout = qtype_multichoice_base::LAYOUT_DROPDOWN; + + $mc->answers = array( + 10 * $i => new question_answer(13, 'California', $data['California'] == 'OK', $data['California'], FORMAT_HTML), + 10 * $i + 1 => new question_answer(14, 'Arizona', $data['Arizona'] == 'OK', $data['Arizona'], FORMAT_HTML), + ); + $mc->qtype = question_bank::get_qtype('multichoice'); + $mc->maxmark = 1; + + $q->subquestions[$i] = $mc; + } + + return $q; + } } diff --git a/question/type/multianswer/tests/walkthrough_test.php b/question/type/multianswer/tests/walkthrough_test.php index 65dff591520..19af3c7564b 100644 --- a/question/type/multianswer/tests/walkthrough_test.php +++ b/question/type/multianswer/tests/walkthrough_test.php @@ -37,70 +37,53 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qtype_multianswer_walkthrough_test extends qbehaviour_walkthrough_test_base { - public function test_interactive() { - return; // TODO + public function test_deferred_feedback() { // Create a gapselect question. - $q = test_question_maker::make_question('calculated'); - $q->hints = array( - new question_hint(1, 'This is the first hint.', FORMAT_HTML), - new question_hint(2, 'This is the second hint.', FORMAT_HTML), - ); - $this->start_attempt_at_question($q, 'interactive', 3); - $values = $q->vs->get_values(); + $q = test_question_maker::make_question('multianswer', 'fourmc'); + $this->start_attempt_at_question($q, 'deferredfeedback', 4); // Check the initial state. $this->check_current_state(question_state::$todo); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_marked_out_of_summary(), - $this->get_contains_submit_button_expectation(true), $this->get_does_not_contain_feedback_expectation(), - $this->get_does_not_contain_validation_error_expectation(), - $this->get_does_not_contain_try_again_button_expectation(), - $this->get_no_hint_visible_expectation()); + $this->get_does_not_contain_validation_error_expectation()); - // Submit blank. - $this->process_submission(array('-submit' => 1, 'answer' => '')); + // Save in incomplete answer. + $this->process_submission(array('sub1_answer' => '1', 'sub2_answer' => '', + 'sub3_answer' => '', 'sub4_answer' => '')); // Verify. $this->check_current_state(question_state::$invalid); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_marked_out_of_summary(), - $this->get_contains_submit_button_expectation(true), $this->get_does_not_contain_feedback_expectation(), - $this->get_contains_validation_error_expectation(), - $this->get_does_not_contain_try_again_button_expectation(), - $this->get_no_hint_visible_expectation()); + $this->get_does_not_contain_validation_error_expectation()); // TODO, really, it should. See MDL-32049. - // Sumit something that does not look like a number. - $this->process_submission(array('-submit' => 1, 'answer' => 'newt')); + // Save a partially correct answer. + $this->process_submission(array('sub1_answer' => '1', 'sub2_answer' => '1', + 'sub3_answer' => '1', 'sub4_answer' => '1')); // Verify. - $this->check_current_state(question_state::$invalid); + $this->check_current_state(question_state::$complete); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_marked_out_of_summary(), - $this->get_contains_submit_button_expectation(true), $this->get_does_not_contain_feedback_expectation(), - $this->get_contains_validation_error_expectation(), - new question_pattern_expectation('/' . - preg_quote(get_string('invalidnumber', 'qtype_numerical') . '/')), - $this->get_does_not_contain_try_again_button_expectation(), - $this->get_no_hint_visible_expectation()); + $this->get_does_not_contain_validation_error_expectation()); - // Now get it right. - $this->process_submission(array('-submit' => 1, 'answer' => $values['a'] + $values['b'])); + // Now submit all and finish. + $this->process_submission(array('-finish' => 1)); // Verify. - $this->check_current_state(question_state::$gradedright); - $this->check_current_mark(3); + $this->check_current_state(question_state::$gradedpartial); + $this->check_current_mark(2); $this->check_current_output( - $this->get_contains_mark_summary(3), - $this->get_contains_submit_button_expectation(false), - $this->get_contains_correct_expectation(), - $this->get_does_not_contain_validation_error_expectation(), - $this->get_no_hint_visible_expectation()); + $this->get_contains_mark_summary(2), + $this->get_contains_partcorrect_expectation(), + $this->get_does_not_contain_validation_error_expectation()); } }