diff --git a/backup/moodle2/backup_qtype_formulas_plugin.class.php b/backup/moodle2/backup_qtype_formulas_plugin.class.php index 7ecc82d1..7aa719bf 100644 --- a/backup/moodle2/backup_qtype_formulas_plugin.class.php +++ b/backup/moodle2/backup_qtype_formulas_plugin.class.php @@ -50,10 +50,10 @@ protected function define_question_plugin_structure() { $formulasanswers = new backup_nested_element('formulas_answers'); $formulasanswer = new backup_nested_element('formulas_answer', ['id'], [ - 'partindex', 'placeholder', 'answermark', 'answertype', 'numbox', 'vars1', 'answer', 'answernotunique', 'vars2', - 'correctness', 'unitpenalty', 'postunit', 'ruleid', 'otherrule', 'subqtext', 'subqtextformat', 'feedback', + 'partindex', 'placeholder', 'answermark', 'answertype', 'numbox', 'vars1', 'answer', 'answernotunique', 'emptyallowed', + 'vars2', 'correctness', 'unitpenalty', 'postunit', 'ruleid', 'otherrule', 'subqtext', 'subqtextformat', 'feedback', 'feedbackformat', 'partcorrectfb', 'partcorrectfbformat', 'partpartiallycorrectfb', 'partpartiallycorrectfbformat', - 'partincorrectfb', 'partincorrectfbformat', + 'partincorrectfb', 'partincorrectfbformat' ]); // Don't need to annotate ids nor files. diff --git a/backup/moodle2/restore_qtype_formulas_plugin.class.php b/backup/moodle2/restore_qtype_formulas_plugin.class.php index 2ace8a39..5ffc5c6a 100644 --- a/backup/moodle2/restore_qtype_formulas_plugin.class.php +++ b/backup/moodle2/restore_qtype_formulas_plugin.class.php @@ -148,6 +148,11 @@ public function process_formulas_answer($data) { if (!isset($data->answernotunique)) { $data->answernotunique = '1'; } + // Backups prior to 6.2 do not yet have the emptyallowed field. In that case, we set it + // to false. It should default to true for *new* questions only. + if (!isset($data->emptyallowed)) { + $data->emptyallowed = '0'; + } // Insert record. $newitemid = $DB->insert_record('qtype_formulas_answers', $data); // Create mapping. @@ -194,6 +199,9 @@ public static function convert_backup_to_questiondata(array $backupdata): stdCla if (!key_exists('answernotunique', $answer)) { $answer['answernotunique'] = '1'; } + if (!key_exists('emptyallowed', $answer)) { + $answer['emptyallowed'] = '0'; + } if (!key_exists('partindex', $answer)) { $answer['partindex'] = $i; } diff --git a/classes/local/answer_parser.php b/classes/local/answer_parser.php index 0697ee90..fa22d1c3 100644 --- a/classes/local/answer_parser.php +++ b/classes/local/answer_parser.php @@ -78,6 +78,13 @@ public function __construct( } } + // If we only have one single token and it is an empty string, we set it to the $EMPTY token. + $firsttoken = reset($tokenlist); + if (count($tokenlist) === 1 && $firsttoken->value === '') { + // FIXME: temporarily disabling this + // $tokenlist[0] = new token(token::EMPTY, '$EMPTY', $firsttoken->row, $firsttoken->column); + } + // Once this is done, we can parse the expression normally. parent::__construct($tokenlist, $knownvariables); } @@ -88,7 +95,18 @@ public function __construct( * @param int $type the answer type, a constant from the qtype_formulas class * @return bool */ - public function is_acceptable_for_answertype(int $type): bool { + public function is_acceptable_for_answertype(int $type, bool $acceptempty = false): bool { + // An empty answer is never acceptable regardless of the answer type, unless empty fields + // are explicitly allowed for a question's part. + // FIXME: this can be removed later + if (empty($this->tokenlist)) { + return $acceptempty; + } + $firsttoken = reset($this->tokenlist); + if (count($this->tokenlist) === 1 && $firsttoken->type === token::EMPTY) { + return $acceptempty; + } + if ($type === qtype_formulas::ANSWER_TYPE_NUMBER) { return $this->is_acceptable_number(); } @@ -102,6 +120,9 @@ public function is_acceptable_for_answertype(int $type): bool { } if ($type === qtype_formulas::ANSWER_TYPE_ALGEBRAIC) { + if (count($this->tokenlist) === 1 && $this->tokenlist[0]->value === '') { + return $acceptempty; + } return $this->is_acceptable_algebraic_formula(); } diff --git a/classes/local/evaluator.php b/classes/local/evaluator.php index a2bdd6c8..59a248b2 100644 --- a/classes/local/evaluator.php +++ b/classes/local/evaluator.php @@ -276,6 +276,20 @@ public function export_single_variable(string $varname, bool $exportasvariable = return $result; } + /** + * FIXME + * + * @param string $name name of the variable + * @param variable $variable variable instance + */ + public function import_single_variable(string $name, variable $variable, bool $overwrite = false): void { + if (array_key_exists($name, $this->variables) && !$overwrite) { + return; + } + + $this->variables[$name] = $variable; + } + /** * Calculate the number of possible variants according to the defined random variables. * @@ -776,18 +790,31 @@ public function diff($first, $second, ?int $n = null) { // This is needed for the diff() function, because strings are evaluated as algebraic // formulas, i. e. in a completely different way. Also, both lists must have the same data // type. - $type = $first[0]->type; - if (!in_array($type, [token::NUMBER, token::STRING])) { - throw new Exception(get_string('error_diff_firstlist_content', 'qtype_formulas')); - } + $type = token::EMPTY; for ($i = 0; $i < $count; $i++) { - if ($first[$i]->type !== $type) { + // As long as we have not found a "real" (i. e. non-empty) element, we update the type. + if ($type === token::EMPTY) { + $type = $first[$i]->type; + } + // If the current element's type does not match, we throw an error, unless it is the + // $EMPTY token, because it may appear in a list of numbers or strings. + if ($first[$i]->type !== $type && $first[$i]->type !== token::EMPTY) { throw new Exception(get_string('error_diff_firstlist_mismatch', 'qtype_formulas', $i)); } - if ($second[$i]->type !== $type) { + if ($second[$i]->type !== $type && $second[$i]->type !== token::EMPTY) { throw new Exception(get_string('error_diff_secondlist_mismatch', 'qtype_formulas', $i)); } } + // If all elements of the first list are $EMPTY, we treat the list as a list of numbers, because + // that's the most straightforward way to calculate the difference. There's probably no real use + // case to have only empty answers in a question, but there's no reason to forbid it, either. + if ($type === token::EMPTY) { + $type = token::NUMBER; + } + // If the type is not valid, we throw an error. + if (!in_array($type, [token::NUMBER, token::STRING])) { + throw new Exception(get_string('error_diff_firstlist_content', 'qtype_formulas')); + } // If we are working with numbers, we can directly calculate the differences and return. if ($type === token::NUMBER) { @@ -798,8 +825,17 @@ public function diff($first, $second, ?int $n = null) { $result = []; for ($i = 0; $i < $count; $i++) { - $diff = abs($first[$i]->value - $second[$i]->value); - $result[$i] = token::wrap($diff); + // This function is also used to calculate the difference between the model answers + // and the student's response. In that case, the difference between an $EMPTY answer + // and any other value shall always be PHP_FLOAT_MAX. The difference between an + // $EMPTY answer and an empty response shall, of course, be 0. For "real" values, + // the difference is calculated normally. + if ($first[$i]->type === token::EMPTY || $second[$i]->type === token::EMPTY) { + $diff = ($second[$i]->type === $first[$i]->type ? 0 : PHP_FLOAT_MAX); + } else { + $diff = abs($first[$i]->value - $second[$i]->value); + } + $result[$i] = token::wrap($diff, token::NUMBER); } return $result; } @@ -812,8 +848,18 @@ public function diff($first, $second, ?int $n = null) { $result = []; // Iterate over all strings and calculate the root mean square difference between the two expressions. for ($i = 0; $i < $count; $i++) { + // If both list elements are the $EMPTY token, the difference is zero and we do not have to + // do any more calculations. Otherwise, we just carry on. The calculation will fail later + // and the difference will automatically be PHP_FLOAT_MAX. + if ($first[$i]->type === token::EMPTY && $second[$i]->type === token::EMPTY) { + $result[$i] = token::wrap(0, token::NUMBER); + continue; + } + $result[$i] = 0; $expression = "({$first[$i]}) - ({$second[$i]})"; + // FIXME: get rid of this again + $expression = str_replace('"', '', $expression); // Flag that we will set to TRUE if a difference cannot be evaluated. This // is to make sure that the difference will be PHP_FLOAT_MAX and not @@ -867,7 +913,7 @@ private function evaluate_the_right_thing($input, bool $godmode = false) { /** * Evaluate a single expression or an array of expressions. * - * @param expression|for_loop|array $input + * @param expression|for_loop|array|false $input * @param bool $godmode whether to run the evaluation in god mode * @return token|array */ @@ -875,6 +921,11 @@ public function evaluate($input, bool $godmode = false) { if (($input instanceof expression) || ($input instanceof for_loop)) { return $this->evaluate_the_right_thing($input, $godmode); } + // For convenience, the evaluator accepts FALSE as an input, This allows + // passing reset($array) with a possibly empty array. + if ($input === false) { + return new token(token::EMPTY, '$EMPTY'); + } if (!is_array($input)) { throw new Exception(get_string('error_evaluate_invocation', 'qtype_formulas', 'evaluate()')); } diff --git a/classes/local/formulas_part.php b/classes/local/formulas_part.php index 0a1ff4e7..352281c1 100644 --- a/classes/local/formulas_part.php +++ b/classes/local/formulas_part.php @@ -83,6 +83,9 @@ class formulas_part { /** @var int whether there are multiple possible answers */ public int $answernotunique; + /** @var int whether students can leave one or more fields empty */ + public int $emptyallowed; + /** @var string definition of the grading criterion */ public string $correctness; @@ -418,6 +421,11 @@ public function normalize_response(array $response): array { * @return bool */ public function is_gradable_response(array $response): bool { + // If the part allows empty fields, we do not have to check anything; the response would be + // gradable even if all fields were empty. + if ($this->emptyallowed) { + return true; + } return !$this->is_unanswered($response); } @@ -431,6 +439,11 @@ public function is_gradable_response(array $response): bool { * @return bool */ public function is_complete_response(array $response): bool { + // If the part allows empty fields, we do not have to check anything; the response can be + // considered complete even if all fields are empty. + if ($this->emptyallowed) { + return true; + } // First, we check if there is a combined unit field. In that case, there will // be only one field to verify. if ($this->has_combined_unit_field()) { @@ -463,6 +476,9 @@ public function is_complete_response(array $response): bool { * @return bool */ public function is_unanswered(array $response): bool { + if (array_key_exists('_seed', $response)) { + return true; + } if (!array_key_exists('normalized', $response)) { $response = $this->normalize_response($response); } @@ -528,6 +544,10 @@ public function get_evaluated_answers(): array { // their numerical value. if ($isalgebraic) { foreach ($this->evaluatedanswers as &$answer) { + // If the answer is $EMPTY, there is nothing to do. + if ($answer === '$EMPTY') { + continue; + } $answer = $this->evaluator->substitute_variables_in_algebraic_formula($answer); } // In case we later write to $answer, this would alter the last entry of the $modelanswers @@ -547,6 +567,11 @@ public function get_evaluated_answers(): array { */ private static function wrap_algebraic_formulas_in_quotes(array $formulas): array { foreach ($formulas as &$formula) { + // We do not have to wrap the $EMPTY token in quotes. + if ($formula === '$EMPTY') { + continue; + } + // If the formula is aready wrapped in quotes, we throw an Exception, because that // should not happen. It will happen, if the student puts quotes around their response, but // we want that to be graded wrong. The exception will be caught and dealt with upstream, @@ -631,7 +656,7 @@ public function add_special_variables(array $studentanswers, float $conversionfa foreach ($studentanswers as $i => &$studentanswer) { // We only do the calculation if the answer type is not algebraic. For algebraic // answers, we don't do anything, because quotes have already been added. - if (!$isalgebraic) { + if (!$isalgebraic && $studentanswer !== '$EMPTY') { $studentanswer = $conversionfactor * $studentanswer; $ssqstudentanswer += $studentanswer ** 2; } @@ -643,9 +668,9 @@ public function add_special_variables(array $studentanswers, float $conversionfa // The variable _d will contain the absolute differences between the model answer // and the student's response. Using the parser's diff() function will make sure // that algebraic answers are correctly evaluated. + // Note: We *must* send the model answer first, because the function has a special check for the + // EMPTY token. $command .= '_d = diff(_a, _r);'; - - // Prepare the variable _err which is the root of the sum of squared differences. $command .= "_err = sqrt(sum(map('*', _d, _d)));"; // Finally, calculate the relative error, unless the question uses an algebraic answer. @@ -653,6 +678,9 @@ public function add_special_variables(array $studentanswers, float $conversionfa // We calculate the sum of squares of all model answers. $ssqmodelanswer = 0; foreach ($this->get_evaluated_answers() as $answer) { + if ($answer === '$EMPTY') { + continue; + } $ssqmodelanswer += $answer ** 2; } // If the sum of squares is 0 (i.e. all answers are 0), then either the student @@ -731,7 +759,7 @@ public function grade(array $response, bool $finalsubmit = false): array { // Check whether the answer is valid for the given answer type. If it is not, // we just throw an exception to make use of the catch block. Note that if the // student's answer was empty, it will fail in this check. - if (!$parser->is_acceptable_for_answertype($this->answertype)) { + if (!$parser->is_acceptable_for_answertype($this->answertype, $this->emptyallowed)) { throw new Exception(); } @@ -739,8 +767,12 @@ public function grade(array $response, bool $finalsubmit = false): array { // failed evaluation, e.g. caused by an invalid answer. $this->evaluator->clear_stack(); - $evaluated = $this->evaluator->evaluate($parser->get_statements())[0]; - $evaluatedresponse[] = token::unpack($evaluated); + // Evaluate. If the answer was empty (an empty string or the '$EMPTY'), the parser + // will create an appropriate evaluable statement or return an empty array. The evaluator, + // on the other hand, will know how to deal with the "false" return value from reset() + // and return the $EMPTY token. + $statements = $parser->get_statements(); + $evaluatedresponse[] = token::unpack($this->evaluator->evaluate(reset($statements))); } catch (Throwable $t) { // TODO: convert to non-capturing catch // If parsing, validity check or evaluation fails, we consider the answer as wrong. @@ -828,8 +860,12 @@ public function get_correct_response(bool $forfeedback = false): array { $answers = $this->get_evaluated_answers(); // Numeric answers should be localized, if that functionality is enabled. + // Empty answers should be just the empty string; a more user-friendly + // output will be created in the renderer. foreach ($answers as &$answer) { - if (is_numeric($answer)) { + if ($answer === '$EMPTY') { + $answer = ''; + } else if (is_numeric($answer)) { $answer = qtype_formulas::format_float($answer); } } diff --git a/classes/local/lexer.php b/classes/local/lexer.php index 6dd2ab40..1e4b54af 100644 --- a/classes/local/lexer.php +++ b/classes/local/lexer.php @@ -95,6 +95,10 @@ private function read_next_token(): ?token { if ($currentchar === input_stream::EOF) { return self::EOF; } + // If we have a $ character, this could introduce the $EMPTY token. + if ($currentchar === '$') { + return $this->read_empty_token(); + } // If we have a " or ' character, this is the start of a string. if ($currentchar === '"' || $currentchar === "'") { return $this->read_string(); @@ -454,6 +458,27 @@ private function read_identifier(): token { return new token($type, $result, $startingposition['row'], $startingposition['column']); } + /** + * Read the special $EMPTY token from the input stream. + * + * @return token the $EMPTY token + */ + private function read_empty_token(): token { + // Start by reading the first char. If we are here, that means it was a $ symbol. + $currentchar = $this->inputstream->read(); + $result = $currentchar; + + // Record position of the $ symbol. + $startingposition = $this->inputstream->get_position(); + + $identifier = $this->read_identifier(); + if ($identifier->value === 'EMPTY') { + return new token(token::EMPTY, '$EMPTY', $startingposition['row'], $startingposition['column']); + } + + $this->inputstream->die(get_string('error_invalid_dollar', 'qtype_formulas')); + } + /** * Read an operator token from the input stream. * diff --git a/classes/local/shunting_yard.php b/classes/local/shunting_yard.php index fab93761..e6618792 100644 --- a/classes/local/shunting_yard.php +++ b/classes/local/shunting_yard.php @@ -307,11 +307,12 @@ public static function infix_to_rpn(array $tokens): array { } } switch ($type) { - // Literals (numbers or strings), constants and variable names go straight to the output queue. + // Literals (numbers or strings), constants, the $EMPTY token and variable names go straight to the output queue. case token::NUMBER: case token::STRING: case token::VARIABLE: case token::CONSTANT: + case token::EMPTY: $output[] = $token; break; // If we encounter an argument separator (,) *and* there is a pending function or array, diff --git a/classes/local/token.php b/classes/local/token.php index 054b25d5..277acffc 100644 --- a/classes/local/token.php +++ b/classes/local/token.php @@ -41,6 +41,9 @@ class token { /** @var int used to designate a token storing a string literal */ const STRING = 5; + /** @var int used to designate the special token used for empty answers */ + const EMPTY = 7; + /** * Parentheses are organised in groups, allowing for bitwise comparison. * examples: CLOSING_PAREN & ANY_PAREN = ANY_PAREN @@ -287,6 +290,10 @@ public static function unpack($token) { if (in_array($token->type, [self::NUMBER, self::STRING])) { return $token->value; } + // If the token is the $EMPTY token, return the string '$EMPTY'. + if ($token->type === self::EMPTY) { + return '$EMPTY'; + } // If the token is a list or set, we have to unpack all elements separately and recursively. if (in_array($token->type, [self::LIST, self::SET])) { diff --git a/db/install.xml b/db/install.xml index bc96d055..04ee9912 100644 --- a/db/install.xml +++ b/db/install.xml @@ -36,6 +36,7 @@ + diff --git a/db/upgrade.php b/db/upgrade.php index ac7b27c4..a03d07a3 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -546,5 +546,21 @@ function xmldb_qtype_formulas_upgrade($oldversion = 0) { upgrade_plugin_savepoint(true, 2023100800, 'qtype', 'formulas'); } + if ($oldversion < 2025080100) { + // Define field emptyallowed to be added to qtype_formulas_answers. + $table = new xmldb_table('qtype_formulas_answers'); + $field = new xmldb_field('emptyallowed', XMLDB_TYPE_INTEGER, '4', null, XMLDB_NOTNULL, null, '0', 'answernotunique'); + + // Conditionally add field. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + // Now fill it with '0' for compatibility with existing questions. + $DB->set_field('qtype_formulas_answers', 'emptyallowed', '0'); + } + + // Formulas savepoint reached. + upgrade_plugin_savepoint(true, 2025080100, 'qtype', 'formulas'); + } + return true; } diff --git a/edit_formulas_form.php b/edit_formulas_form.php index 366d7552..7a99beea 100644 --- a/edit_formulas_form.php +++ b/edit_formulas_form.php @@ -220,6 +220,15 @@ protected function get_per_answer_fields( ); $repeatedoptions['answer']['helpbutton'] = ['answer', 'qtype_formulas']; $repeatedoptions['answer']['type'] = PARAM_RAW_TRIMMED; + // Whether the part allows leaving one or more fields empty. + // FIXME: add some validation: if $EMPTY in model answers, this must be checked + // FIXME: add validation: at least one answer must be ≠ $EMPTY + $repeated[] = $mform->createElement( + 'advcheckbox', + 'emptyallowed', + get_string('emptyallowed', 'qtype_formulas') + ); + $repeatedoptions['emptyallowed']['helpbutton'] = ['emptyallowed', 'qtype_formulas']; // Whether the question has multiple answers. $repeated[] = $mform->createElement( 'advcheckbox', diff --git a/lang/en/qtype_formulas.php b/lang/en/qtype_formulas.php index be0bce67..93b32c8f 100644 --- a/lang/en/qtype_formulas.php +++ b/lang/en/qtype_formulas.php @@ -101,6 +101,9 @@ $string['defaultwidth_unit_desc'] = 'Default width of the separate unit field'; $string['defaultwidthunit'] = 'Unit of length'; $string['defaultwidthunit_desc'] = 'Unit of length used for the default width settings below. The units "em" or "rem" correspond approximately to the width of one digit.'; +$string['emptyallowed'] = 'Students may leave one or more fields empty.'; +$string['emptyallowed_help'] = 'Check this option for parts where not all fields have to be filled, e. g. when you have two fields for an equation that could have just one solution, and you do not want to give away the number of solutions.'; +$string['emptyanswer'] = '[empty]'; $string['error_algebraic_relerr'] = 'Relative error (_relerr) cannot be used with answer type algebraic formula.'; $string['error_algvar_numbers'] = 'Algebraic variables can only be initialized with a list of numbers.'; $string['error_answer_missing'] = 'No answer has been defined.'; @@ -138,6 +141,7 @@ $string['error_distribution_outcomes'] = '{$a} expects the number of successful outcomes to be a non-negative integer.'; $string['error_distribution_tries'] = '{$a} expects the number of tries to be a non-negative integer.'; $string['error_divzero'] = 'Division by zero is not defined.'; +$string['error_emptyparens'] = 'Parse error: closing {$a->close} immediately after opening {$a->open}.'; $string['error_emptyrange'] = 'Evaluation error: range from {$a->start} to {$a->end} with step {$a->step} will be empty.'; $string['error_emptystack'] = 'Evaluation error: empty stack - did you pass enough arguments for the function or operator?'; $string['error_evaluate_invocation'] = 'Bad invocation of {$a}.'; @@ -191,6 +195,7 @@ $string['error_inv_list'] = 'inv() expects a list.'; $string['error_inv_nodup'] = 'When using inv(), the list must not contain the same number multiple times.'; $string['error_inv_smallest'] = 'When using inv(), the smallest number in the list must be 0 or 1.'; +$string['error_invalid_dollar'] = 'Invalid use of the dollar ($) symbol. It can only be used for the special value $EMPTY.'; $string['error_invalidalgebraic'] = '\'{$a}\' is not a valid algebraic expression.'; $string['error_invalidargsep'] = 'Syntax error: invalid use of separator token \',\'.'; $string['error_invalidcodepoint'] = 'Invalid UTF-8 codepoint escape sequence.'; diff --git a/question.php b/question.php index 9a5f6112..dfa3a5f9 100644 --- a/question.php +++ b/question.php @@ -454,6 +454,9 @@ public function check_file_access($qa, $options, $component, $filearea, $args, $ * @return bool whether this response can be graded */ public function is_gradable_response(array $response): bool { + if (array_key_exists('_seed', $response)) { + return false; + } // Iterate over all parts. If at least one part is gradable, we can leave early. foreach ($this->parts as $part) { if ($part->is_gradable_response($response)) { @@ -517,7 +520,7 @@ public function summarise_response(array $response) { foreach ($this->parts as $part) { $summary[] = $part->summarise_response($response); } - return implode(', ', $summary); + return implode('; ', $summary); } /** @@ -534,6 +537,7 @@ public function classify_response(array $response) { // Now, we do the classification for every part. foreach ($this->parts as $part) { // Unanswered parts can immediately be classified. + // FIXME: change this if part allows empty fields if ($part->is_unanswered($response)) { $classification[$part->partindex] = question_classified_response::no_response(); continue; diff --git a/questiontype.php b/questiontype.php index d5302758..04c307c3 100644 --- a/questiontype.php +++ b/questiontype.php @@ -82,7 +82,7 @@ class qtype_formulas extends question_type { * - otherrule: additional rules for unit conversion */ const PART_BASIC_FIELDS = ['placeholder', 'answermark', 'answertype', 'numbox', 'vars1', 'answer', 'answernotunique', 'vars2', - 'correctness', 'unitpenalty', 'postunit', 'ruleid', 'otherrule']; + 'emptyallowed', 'correctness', 'unitpenalty', 'postunit', 'ruleid', 'otherrule']; /** * This function returns the "simple" additional fields defined in the qtype_formulas_options @@ -285,6 +285,7 @@ public function save_question_options($formdata) { 'numbox' => 1, 'answer' => '', 'answernotunique' => 1, + 'emptyallowed' => 1, 'correctness' => '', 'ruleid' => 1, 'subqtext' => '', @@ -623,12 +624,16 @@ public function import_from_xml($xml, $question, qformat_xml $format, $extra = n $question->partindex[$i] = $partindex; } foreach (self::PART_BASIC_FIELDS as $field) { - // Older questions do not have this field, so we do not want to issue an error message. - // Also, for maximum backwards compatibility, we set the default value to 1. With this, - // nothing changes for old questions. + // Older questions do not have the 'answernotunique' field, so we do not want to issue an + // error message. For maximum backwards compatibility, we set the default value to 1. With + // this, nothing changes for old questions. Also, questions exported prior to version 6.2 + // won't have the 'emptyallowed' field. We will set it to 0 (false) for backwards compatiblity. if ($field === 'answernotunique') { $ifnotexists = ''; $default = '1'; + } else if ($field === 'emptyallowed') { + $ifnotexists = ''; + $default = '0'; } else { $ifnotexists = get_string('error_import_missing_field', 'qtype_formulas', $field); $default = '0'; @@ -1151,17 +1156,18 @@ public function check_variables_and_expressions(object $data, array $parts, bool // "algebraic formula", they must all be strings. Otherwise, they must all be numbers or // at least numeric strings. foreach ($modelanswers as $answer) { - if ($isalgebraic && $answer->type !== token::STRING) { + if ($isalgebraic && ($answer->type !== token::STRING && $answer->type !== token::EMPTY)) { $errors["answer[$i]"] = get_string('error_string_for_algebraic_formula', 'qtype_formulas'); continue; } - if (!$isalgebraic && !($answer->type === token::NUMBER || is_numeric($answer->value))) { + if (!$isalgebraic && !($answer->type === token::EMPTY || $answer->type === token::NUMBER || is_numeric($answer->value))) { $errors["answer[$i]"] = get_string('error_number_for_numeric_answertypes', 'qtype_formulas'); continue; } } // Finally, we convert the array of tokens into an array of literals. - $modelanswers = token::unpack($modelanswers); + // FIXME: remove this + // $modelanswers = token::unpack($modelanswers); // Now that we know the model answers, we can set the $numbox property for the part, // i. e. the number of answer boxes that are to be shown. @@ -1170,10 +1176,14 @@ public function check_variables_and_expressions(object $data, array $parts, bool // If the answer type is algebraic, we must now try to do algebraic evaluation of each answer // to check for bad formulas. if ($isalgebraic) { - foreach ($modelanswers as $k => $answer) { + foreach ($modelanswers as $k => $answertoken) { + // If it is the $EMPTY token, we have nothing to do. + if ($answertoken->type === token::EMPTY) { + continue; + } // Evaluating the string should give us a numeric value. try { - $result = $partevaluator->calculate_algebraic_expression($answer); + $result = $partevaluator->calculate_algebraic_expression($answertoken->value); } catch (Exception $e) { $a = (object)[ // Answers are zero-indexed, but users normally count from 1. diff --git a/renderer.php b/renderer.php index cb72a75e..afd7b695 100644 --- a/renderer.php +++ b/renderer.php @@ -852,6 +852,11 @@ public function correct_response(question_attempt $qa) { */ public function part_correct_response($part) { $answers = $part->get_correct_response(true); + foreach ($answers as &$answer) { + if ($answer === '') { + $answer = get_string('emptyanswer', 'qtype_formulas'); + } + } $answertext = implode('; ', $answers); $string = ($part->answernotunique ? 'correctansweris' : 'uniquecorrectansweris'); @@ -873,7 +878,7 @@ protected function num_parts_correct(question_attempt $qa) { /** @var qtype_formulas_question $question */ $question = $qa->get_question(); $response = $qa->get_last_qt_data(); - if (!$question->is_gradable_response($response)) { + if (!$question->is_gradable_response($response) || array_key_exists('_seed', $response)) { return ''; } @@ -913,7 +918,11 @@ protected function combined_feedback(question_attempt $qa) { $state = $qa->get_state(); if (!$state->is_finished()) { $response = $qa->get_last_qt_data(); - if (!$question->is_gradable_response($response)) { + // When starting a question, there will be no response, but the _seed variable will be set. + // As we now accept questions with an empty response, the grading mechanism would try to grade + // this "startup data" like an entirely empty response and hence give the feedback for a wrong + // answer, before the student has even submitted anything. + if (!$question->is_gradable_response($response) || array_key_exists('_seed', $response)) { return ''; } $state = $question->grade_response($response)[1]; diff --git a/tests/backup_restore_test.php b/tests/backup_restore_test.php index 5c6a6cf3..fd255ed2 100644 --- a/tests/backup_restore_test.php +++ b/tests/backup_restore_test.php @@ -74,6 +74,7 @@ final class backup_restore_test extends \advanced_testcase { public static function provide_question_names(): array { return [ ['testsinglenum'], + ['testnumandempty'], ['testalgebraic'], ['testtwonums'], ['testsinglenumunit'], @@ -306,6 +307,7 @@ public function test_restore_quiz_with_same_stamp_questions(string $questionname */ public static function provide_xml_keys_to_remove(): array { return [ + ['emptyallowed'], ['answernotunique'], ['partindex'], ]; @@ -438,6 +440,7 @@ public function test_restore_of_legacy_backup_with_missing_fields(): void { // value that will be assigned in restore_qtype_formulas_plugin.class.php. $fieldstoremove = [ 'answernotunique' => '1', + 'emptyallowed' => '0', 'shownumcorrect' => 0, 'answernumbering' => 'none', 'feedbackformat' => FORMAT_HTML, diff --git a/tests/behat/export.feature b/tests/behat/export.feature index 1bdc0839..ff2fd32d 100644 --- a/tests/behat/export.feature +++ b/tests/behat/export.feature @@ -27,7 +27,7 @@ Feature: Test exporting Formulas questions When I am on the "Course 1" "core_question > course question export" page And I set the field "id_format_xml" to "1" And I press "Export questions to file" - Then following "click here" should download between "6250" and "6500" bytes + Then following "click here" should download between "6400" and "6600" bytes # If the download step is the last in the scenario then we can sometimes run # into the situation where the download page causes a http redirect but behat # has already conducted its reset (generating an error). By putting a logout diff --git a/tests/evaluator_test.php b/tests/evaluator_test.php index 98cd0cc5..284adcba 100644 --- a/tests/evaluator_test.php +++ b/tests/evaluator_test.php @@ -133,7 +133,8 @@ public static function provide_invalid_diff(): array { ['The first argument of diff() must be a list.', 'diff("", "");'], ['The second argument of diff() must be a list.', 'diff([1,2,3], 1);'], ['diff() expects two lists of the same size.', 'diff([1,2,3], [1,2]);'], - ['When using diff(), the first list must contain only numbers or only strings.', 'diff([[1,2]], [1]);'], + ['When using diff(), the first list must contain only numbers or only strings.', 'diff([[1,2]], [[1,2]]);'], + ['diff(): type mismatch for element #0 (zero-indexed) of the second list.', 'diff([[1,2]], [1]);'], ['diff(): type mismatch for element #1 (zero-indexed) of the first list.', 'diff([1,"a"], [1,2]);'], ['diff(): type mismatch for element #1 (zero-indexed) of the first list.', 'diff(["a",1], ["a","b"]);'], ['diff(): type mismatch for element #0 (zero-indexed) of the second list.', 'diff([1,2], ["a",2]);'], @@ -1937,6 +1938,29 @@ public function test_invalid_stuff($expected, $input): void { self::assertStringEndsWith($expected, $error); } + public function test_fixme_test(): void { + $error = ''; + $result = []; + $statements = [ + new expression([ + new token(token::NUMBER, 2), + new token(token::NUMBER, NAN), + new token(token::OPERATOR, '*'), + ]) + ]; + $parser = new parser('a = [1, 2]'); + $statements = $parser->get_statements(); + + try { + $evaluator = new evaluator(); + $result = $evaluator->evaluate($statements); + } catch (Exception $e) { + $error = $e->getMessage(); + echo $error . PHP_EOL; + } + //var_dump($result); + } + /** * Provide various (valid and invalid) algebraic expressions. * @@ -2057,6 +2081,40 @@ public function test_export_import_variable_context(): void { self::assertNotNull($e); } + public function test_import_single_variable(): void { + // Prepare an empty evaluator. + $evaluator = new evaluator(); + + // Add variables. + $evaluator->import_single_variable('_foo', new variable('_foo', 'value', token::STRING)); + $evaluator->import_single_variable('bar', new variable('bar', 1234, token::NUMBER)); + + // Verify the evaluator contains the correct variables with the correct values. + $variables = $evaluator->export_variable_list(); + self::assertCount(2, $variables); + self::assertContains('_foo', $variables); + self::assertContains('bar', $variables); + + $foo = $evaluator->export_single_variable('_foo'); + self::assertEquals(token::STRING, $foo->type); + self::assertEquals('value', $foo->value); + + $bar = $evaluator->export_single_variable('bar'); + self::assertEquals(token::NUMBER, $bar->type); + self::assertEquals(1234, $bar->value); + + // Try to set variable again without specifying the overwrite parameter. The value should not change. + $evaluator->import_single_variable('bar', new variable('bar', 5678, token::NUMBER)); + $bar = $evaluator->export_single_variable('bar'); + self::assertEquals(1234, $bar->value); + + // Try again, forcing overwrite this time. The value should thus change. + $evaluator->import_single_variable('bar', new variable('bar', 5678, token::NUMBER), true); + $bar = $evaluator->export_single_variable('bar'); + self::assertEquals(5678, $bar->value); + } + + /** * Provide answers of answer type number. * diff --git a/tests/helper.php b/tests/helper.php index c9581275..201e494e 100644 --- a/tests/helper.php +++ b/tests/helper.php @@ -40,8 +40,12 @@ class qtype_formulas_test_helper extends question_test_helper { */ public function get_test_questions(): array { return [ + // FIXME: one question algebraic 'x' and '$EMPTY' + // FIXME: one question multipart: 1 in first part, $EMPTY in second part // Minimal formulas question: one part, not randomised, answer = 5. 'testsinglenum', + // Minimal formulas question: one part, not randomised, one answer = 1, one answer = empty. + 'testnumandempty', // Formulas question with algebraic answer. 'testalgebraic', // Minimal formulas question: one part, two numbers, answer = 2 and 3. @@ -113,6 +117,7 @@ public static function make_a_formulas_part(): formulas_part { $p->vars2 = ''; $p->answer = '1'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->correctness = '_relerr < 0.01'; $p->unitpenalty = 1; $p->postunit = ''; @@ -159,6 +164,7 @@ public static function make_formulas_question_testalgebraic() { $p->answer = '"a*x^2"'; $p->answertype = strval(qtype_formulas::ANSWER_TYPE_ALGEBRAIC); $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->correctness = '_err < 0.01'; $p->subqtext = ''; $p->partcorrectfb = 'Your answer is correct.'; @@ -179,6 +185,7 @@ public function get_formulas_question_form_data_testalgebraic() { $form->noanswers = 1; $form->answer = ['"a*x^2"']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [1]; $form->answertype = [strval(qtype_formulas::ANSWER_TYPE_ALGEBRAIC)]; $form->correctness = ['_err < 0.01']; @@ -239,6 +246,7 @@ public static function make_formulas_question_testsinglenum() { $p->answermark = 2; $p->answer = '5'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->subqtext = ''; $q->parts[0] = $p; @@ -261,6 +269,7 @@ public function get_formulas_question_form_data_testsinglenum(): stdClass { $form->noanswers = 1; $form->answer = ['5']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -355,6 +364,201 @@ public static function get_formulas_question_data_testsinglenum(): stdClass { 'vars2' => '', 'answer' => '5', 'answernotunique' => '1', + 'emptyallowed' => '0', + 'correctness' => '_relerr < 0.01', + 'unitpenalty' => 1, + 'postunit' => '', + 'ruleid' => 1, + 'otherrule' => '', + 'subqtext' => '', + 'subqtextformat' => FORMAT_HTML, + 'feedback' => '', + 'feedbackformat' => FORMAT_HTML, + 'partcorrectfb' => self::DEFAULT_CORRECT_FEEDBACK, + 'partcorrectfbformat' => FORMAT_HTML, + 'partpartiallycorrectfb' => self::DEFAULT_PARTIALLYCORRECT_FEEDBACK, + 'partpartiallycorrectfbformat' => FORMAT_HTML, + 'partincorrectfb' => self::DEFAULT_INCORRECT_FEEDBACK, + 'partincorrectfbformat' => FORMAT_HTML, + 'partindex' => 0, + ], + ]; + + $qdata->options->numparts = 1; + + $qdata->hints = [ + (object) [ + 'id' => '101', + 'hint' => 'Hint 1.', + 'hintformat' => FORMAT_HTML, + 'shownumcorrect' => 1, + 'clearwrong' => 0, + 'options' => 0, + ], + (object) [ + 'id' => '102', + 'hint' => 'Hint 2.', + 'hintformat' => FORMAT_HTML, + 'shownumcorrect' => 1, + 'clearwrong' => 1, + 'options' => 1, + ], + ]; + + return $qdata; + } + + /** + * Create a single-part test question with one number and one empty field. + * + * @return qtype_formulas_question + */ + public static function make_formulas_question_testnumandempty() { + $q = self::make_a_formulas_question(); + + $q->name = 'test-numandempty'; + $q->questiontext = '

This is a minimal question. The first answer is 1, the second is empty.

'; + + $q->penalty = 0.3; // Non-zero and not the default. + $q->textfragments = [ + 0 => '

This is a minimal question. The first answer is 1, the second is empty.

', + 1 => '', + ]; + $q->numparts = 1; + $q->defaultmark = 2; + $q->generalfeedback = ''; + $p = self::make_a_formulas_part(); + $p->questionid = $q->id; + $p->id = 14; + $p->placeholder = ''; + $p->numbox = 2; + $p->answermark = 2; + $p->answer = '[1, $EMPTY]'; + $p->answernotunique = '1'; + $p->emptyallowed = '1'; + $p->subqtext = ''; + $q->parts[0] = $p; + + $q->hints = [ + new question_hint_with_parts(101, 'Hint 1.', FORMAT_HTML, 1, 0), + new question_hint_with_parts(102, 'Hint 2.', FORMAT_HTML, 1, 1), + ]; + return $q; + } + + /** + * Gets the question form data for the testnumandempty formulas question + * + * @return stdClass + */ + public function get_formulas_question_form_data_testnumandempty(): stdClass { + $form = new stdClass(); + + $form->name = 'test-numandempty'; + $form->noanswers = 1; + $form->answer = ['1', '$EMPTY']; + $form->answernotunique = ['1']; + $form->emptyallowed = ['1']; + $form->answermark = [2]; + $form->answertype = ['0']; + $form->correctness = ['_relerr < 0.01']; + $form->numbox = [2]; + $form->placeholder = ['']; + $form->vars1 = ['']; + $form->vars2 = ['']; + $form->postunit = ['']; + $form->otherrule = ['']; + $form->subqtext = [ + ['text' => '', 'format' => FORMAT_HTML], + ]; + $form->feedback = [ + ['text' => '', 'format' => FORMAT_HTML], + ]; + $form->partcorrectfb = [ + ['text' => self::DEFAULT_CORRECT_FEEDBACK, 'format' => FORMAT_HTML], + ]; + $form->partpartiallycorrectfb = [ + ['text' => self::DEFAULT_PARTIALLYCORRECT_FEEDBACK, 'format' => FORMAT_HTML], + ]; + $form->partincorrectfb = [ + ['text' => self::DEFAULT_INCORRECT_FEEDBACK, 'format' => FORMAT_HTML], + ]; + $form->questiontext = [ + 'text' => '

This is a minimal question. The first answer is 1, the second is empty.

', + 'format' => FORMAT_HTML, + ]; + $form->generalfeedback = ['text' => '', 'format' => FORMAT_HTML]; + $form->defaultmark = 2; + $form->penalty = 0.3; + $form->varsrandom = ''; + $form->varsglobal = ''; + $form->answernumbering = 'abc'; + $form->globalunitpenalty = 1; + $form->globalruleid = 1; + $form->correctfeedback = [ + 'text' => test_question_maker::STANDARD_OVERALL_CORRECT_FEEDBACK, + 'format' => FORMAT_HTML, + ]; + $form->partiallycorrectfeedback = [ + 'text' => test_question_maker::STANDARD_OVERALL_PARTIALLYCORRECT_FEEDBACK, + 'format' => FORMAT_HTML, + ]; + $form->incorrectfeedback = [ + 'text' => test_question_maker::STANDARD_OVERALL_INCORRECT_FEEDBACK, + 'format' => FORMAT_HTML, + ]; + $form->shownumcorrect = '1'; + $form->hint = [ + ['text' => 'Hint 1.', 'format' => FORMAT_HTML], + ['text' => 'Hint 2.', 'format' => FORMAT_HTML], + ]; + $form->hintclearwrong = [0, 1]; + $form->hintshownumcorrect = [1, 1]; + return $form; + } + + /** + * Return question data for a single-part question with one number and an empty field. + * + * @return stdClass + */ + public static function get_formulas_question_data_testnumandempty(): stdClass { + $qdata = new stdClass(); + test_question_maker::initialise_question_data($qdata); + + $qdata->qtype = 'formulas'; + $qdata->name = 'test-numandempty'; + $qdata->questiontext = '

This is a minimal question. The first answer is 1, the second is empty.

'; + $qdata->generalfeedback = ''; + $qdata->defaultmark = 2; + $qdata->penalty = 0.3; + + $qdata->options = new stdClass(); + $qdata->contextid = context_system::instance()->id; + $qdata->options->varsrandom = ''; + $qdata->options->varsglobal = ''; + $qdata->options->answernumbering = 'abc'; + $qdata->options->shownumcorrect = 1; + $qdata->options->correctfeedback = test_question_maker::STANDARD_OVERALL_CORRECT_FEEDBACK; + $qdata->options->correctfeedbackformat = FORMAT_HTML; + $qdata->options->partiallycorrectfeedback = test_question_maker::STANDARD_OVERALL_PARTIALLYCORRECT_FEEDBACK; + $qdata->options->partiallycorrectfeedbackformat = FORMAT_HTML; + $qdata->options->incorrectfeedback = test_question_maker::STANDARD_OVERALL_INCORRECT_FEEDBACK; + $qdata->options->incorrectfeedbackformat = FORMAT_HTML; + + $qdata->options->answers = [ + 14 => (object) [ + 'id' => 14, + 'questionid' => $qdata->id, + 'placeholder' => '', + 'answermark' => 2, + 'answertype' => '0', + 'numbox' => 2, + 'vars1' => '', + 'vars2' => '', + 'answer' => '[1, $EMPTY]', + 'answernotunique' => '1', + 'emptyallowed' => '1', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => '', @@ -421,6 +625,7 @@ public static function make_formulas_question_testsinglenumunit(): qtype_formula $p->answermark = 2; $p->answer = '5'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->postunit = 'm/s'; $p->subqtext = '{_0}{_u}'; @@ -439,6 +644,7 @@ public function get_formulas_question_form_data_testsinglenumunit() { $form->noanswers = 1; $form->answer = ['5']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -527,6 +733,7 @@ public static function get_formulas_question_data_testsinglenumunit(): stdClass 'vars2' => '', 'answer' => '5', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => 'm/s', @@ -577,6 +784,7 @@ public static function make_formulas_question_testsinglenumunitsep(): qtype_form $p->answermark = 2; $p->answer = '5'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->postunit = 'm/s'; $p->subqtext = '{_0} {_u}'; $p->partcorrectfb = self::DEFAULT_CORRECT_FEEDBACK; @@ -597,6 +805,7 @@ public function get_formulas_question_form_data_testsinglenumunitsep() { $form->noanswers = 1; $form->answer = ['5']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -684,6 +893,7 @@ public static function get_formulas_question_data_testsinglenumunitsep(): stdCla 'vars2' => '', 'answer' => '5', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => 'm/s', @@ -735,6 +945,7 @@ public static function make_formulas_question_testtwonums(): qtype_formulas_ques $p->answer = '[2, 3]'; $p->numbox = 2; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->subqtext = ''; $p->partcorrectfb = 'Your answer is correct.'; $p->partpartiallycorrectfb = 'Your answer is partially correct.'; @@ -754,6 +965,7 @@ public function get_formulas_question_form_data_testtwonums() { $form->noanswers = 1; $form->answer = ['[2, 3]']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -821,6 +1033,7 @@ public static function make_formulas_question_testthreeparts(): qtype_formulas_q $p0->answermark = 2; $p0->answer = '5'; $p0->answernotunique = '1'; + $p0->emptyallowed = '0'; $p0->subqtext = 'This is first part.'; $p0->partcorrectfb = 'Part 1 correct feedback.'; $p0->partpartiallycorrectfb = 'Part 1 partially correct feedback.'; @@ -833,6 +1046,7 @@ public static function make_formulas_question_testthreeparts(): qtype_formulas_q $p1->answermark = 2; $p1->answer = '6'; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->subqtext = 'This is second part.'; $p1->partcorrectfb = 'Part 2 correct feedback.'; $p1->partpartiallycorrectfb = 'Part 2 partially correct feedback.'; @@ -845,6 +1059,7 @@ public static function make_formulas_question_testthreeparts(): qtype_formulas_q $p2->answermark = 2; $p2->answer = '7'; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; $p2->subqtext = 'This is third part.'; $p2->partcorrectfb = 'Part 3 correct feedback.'; $p2->partpartiallycorrectfb = 'Part 3 partially correct feedback.'; @@ -873,6 +1088,7 @@ public function get_formulas_question_form_data_testthreeparts() { $form->noanswers = 3; $form->answer = ['5', '6', '7']; $form->answernotunique = ['1', '1', '1']; + $form->emptyallowed = ['0', '0', '0']; $form->answermark = ['2', '2', '2']; $form->numbox = [1, 1, 1]; $form->placeholder = ['#1', '#2', '#3']; @@ -957,6 +1173,7 @@ public static function make_formulas_question_testmethodsinparts(): qtype_formul $p0->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0}{_u}

'; // Combined unit. $p0->answer = 'v'; $p0->answernotunique = '1'; + $p0->emptyallowed = '0'; $p0->postunit = 'm/s'; $q->parts[0] = $p0; @@ -968,6 +1185,7 @@ public static function make_formulas_question_testmethodsinparts(): qtype_formul $p1->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0} {_u}

'; // Separated unit. $p1->answer = 'v'; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->postunit = 'm/s'; $q->parts[1] = $p1; @@ -980,6 +1198,7 @@ public static function make_formulas_question_testmethodsinparts(): qtype_formul $p2->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0} {_u}

'; $p2->answer = 'v'; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; $p2->postunit = ''; $q->parts[2] = $p2; @@ -992,6 +1211,7 @@ public static function make_formulas_question_testmethodsinparts(): qtype_formul $p3->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? speed = {_0}{_u}

'; $p3->answer = 'v'; $p3->answernotunique = '1'; + $p3->emptyallowed = '0'; $p3->postunit = ''; $q->parts[3] = $p3; @@ -1046,6 +1266,7 @@ public static function get_formulas_question_data_testmethodsinparts() { 'vars2' => '', 'answer' => 'v', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => 'm/s', @@ -1074,6 +1295,7 @@ public static function get_formulas_question_data_testmethodsinparts() { 'vars2' => '', 'answer' => 'v', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => 'm/s', @@ -1102,6 +1324,7 @@ public static function get_formulas_question_data_testmethodsinparts() { 'vars2' => '', 'answer' => 'v', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => '', @@ -1130,6 +1353,7 @@ public static function get_formulas_question_data_testmethodsinparts() { 'vars2' => '', 'answer' => 'v', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => '', @@ -1202,6 +1426,12 @@ public function get_formulas_question_form_data_testmethodsinparts() { 2 => '1', 3 => '1', ]; + $form->emptyallowed = [ + 0 => '0', + 1 => '0', + 2 => '0', + 3 => '0', + ]; $form->answermark = [ 0 => 2, 1 => 2, @@ -1338,6 +1568,7 @@ public static function make_formulas_question_testzero(): qtype_formulas_questio $p->answermark = 2; $p->answer = '0'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $q->parts[0] = $p; return $q; @@ -1354,6 +1585,7 @@ public function get_formulas_question_form_data_testzero() { $form->noanswers = 1; $form->answer = ['0']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -1427,6 +1659,7 @@ public static function make_formulas_question_test4(): qtype_formulas_question { $p0->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0}{_u}

'; // Combined unit. $p0->answer = 'v'; $p0->answernotunique = '1'; + $p0->emptyallowed = '0'; $p0->postunit = 'm/s'; $q->parts[0] = $p0; $p1 = self::make_a_formulas_part(); @@ -1436,6 +1669,7 @@ public static function make_formulas_question_test4(): qtype_formulas_question { $p1->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0} {_u}

'; // Separated unit. $p1->answer = 'v'; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->postunit = 'm/s'; $q->parts[1] = $p1; $p2 = self::make_a_formulas_part(); @@ -1443,6 +1677,7 @@ public static function make_formulas_question_test4(): qtype_formulas_question { $p2->partindex = 2; $p2->answermark = 2; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; // As postunit is empty {_u} should be ignored. $p2->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? {_0} {_u}

'; $p2->answer = 'v'; @@ -1456,6 +1691,7 @@ public static function make_formulas_question_test4(): qtype_formulas_question { $p3->subqtext = '

If a car travels {s} m in {dt} s, what is the speed of the car? speed = {_0}{_u}

'; $p3->answer = 'v'; $p3->answernotunique = '1'; + $p3->emptyallowed = '0'; $p3->postunit = ''; $q->parts[3] = $p3; @@ -1481,6 +1717,7 @@ public function get_formulas_question_form_data_test4() { $form->noanswers = 4; $form->answer = ['v', 'v', 'v', 'v']; $form->answernotunique = ['1', '1', '1', '1']; + $form->emptyallowed = ['0', '0', '0', '0']; $form->answermark = ['2', '2', '2', '2']; $form->numbox = [1, 1, 1, 1]; $form->placeholder = ['', '', '', '']; @@ -1572,6 +1809,7 @@ public static function make_formulas_question_testmc(): qtype_formulas_question $p->answermark = 2; $p->answer = '1'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->subqtext = '{_0:mychoices}'; $q->parts[0] = $p; @@ -1589,6 +1827,7 @@ public function get_formulas_question_form_data_testmc() { $form->noanswers = 1; $form->answer = ['1']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -1655,6 +1894,7 @@ public static function make_formulas_question_testmce(): qtype_formulas_question $p->answermark = 2; $p->answer = '1'; $p->answernotunique = '1'; + $p->emptyallowed = '0'; $p->subqtext = '{_0:mychoices:MCE}'; $q->parts[0] = $p; @@ -1672,6 +1912,7 @@ public function get_formulas_question_form_data_testmce() { $form->noanswers = 1; $form->answer = ['1']; $form->answernotunique = ['1']; + $form->emptyallowed = ['0']; $form->answermark = [2]; $form->answertype = ['0']; $form->correctness = ['_relerr < 0.01']; @@ -1741,6 +1982,7 @@ public static function make_formulas_question_testmcetwoparts(): qtype_formulas_ $p1->answermark = 1; $p1->answer = '1'; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->subqtext = '{_0:choices1:MCE}'; $p1->partcorrectfb = 'Your first answer is correct.'; $q->parts[0] = $p1; @@ -1750,6 +1992,7 @@ public static function make_formulas_question_testmcetwoparts(): qtype_formulas_ $p2->answermark = 1; $p2->answer = '1'; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; $p2->subqtext = '{_0:choices2:MCE}'; $p2->partcorrectfb = 'Your second answer is correct.'; $q->parts[1] = $p2; @@ -1768,6 +2011,7 @@ public function get_formulas_question_form_data_testmcetwoparts() { $form->noanswers = 2; $form->answer = ['1', '1']; $form->answernotunique = ['1', '1']; + $form->emptyallowed = ['0', '0']; $form->answermark = [1, 1]; $form->answertype = ['0', '0']; $form->correctness = ['_relerr < 0.01', '_relerr < 0.01']; @@ -1843,6 +2087,7 @@ public static function make_formulas_question_testmctwoparts(): qtype_formulas_q $p1->answermark = 1; $p1->answer = '1'; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->subqtext = 'Part 1 -- {_0:choices1}'; $p1->partcorrectfb = 'Your first answer is correct.'; $q->parts[0] = $p1; @@ -1852,6 +2097,7 @@ public static function make_formulas_question_testmctwoparts(): qtype_formulas_q $p2->answermark = 1; $p2->answer = '1'; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; $p2->subqtext = 'Part 2 -- {_0:choices2}'; $p2->partcorrectfb = 'Your second answer is correct.'; $q->parts[1] = $p2; @@ -1870,6 +2116,7 @@ public function get_formulas_question_form_data_testmctwoparts() { $form->noanswers = 2; $form->answer = ['1', '1']; $form->answernotunique = ['1', '1']; + $form->emptyallowed = ['0', '0']; $form->answermark = [1, 1]; $form->answertype = ['0', '0']; $form->correctness = ['_relerr < 0.01', '_relerr < 0.01']; @@ -1947,6 +2194,7 @@ public static function make_formulas_question_testtwoandtwo(): qtype_formulas_qu $p1->answer = '[1, 2]'; $p1->numbox = 2; $p1->answernotunique = '1'; + $p1->emptyallowed = '0'; $p1->subqtext = 'Part 1 -- {_0} -- {_1}'; $p1->partcorrectfb = 'Your answers in part 1 are correct.'; $q->parts[0] = $p1; @@ -1959,6 +2207,7 @@ public static function make_formulas_question_testtwoandtwo(): qtype_formulas_qu $p2->answer = '[3, 4]'; $p2->numbox = 2; $p2->answernotunique = '1'; + $p2->emptyallowed = '0'; $p2->subqtext = 'Part 2 -- {_0} -- {_1}'; $p2->partcorrectfb = 'Your answers in part 2 are correct.'; $q->parts[1] = $p2; @@ -1977,6 +2226,7 @@ public function get_formulas_question_form_data_testtwoandtwo() { $form->noanswers = 2; $form->answer = [0 => '[1, 2]', 1 => '[3, 4]']; $form->answernotunique = ['1', '1']; + $form->emptyallowed = ['0', '0']; $form->answermark = [0 => 1, 1 => 1]; $form->answertype = ['0', '0']; $form->correctness = ['_relerr < 0.01', '_relerr < 0.01']; @@ -2079,6 +2329,7 @@ public static function get_formulas_question_data_testtwoandtwo(): stdClass { 'vars2' => '', 'answer' => '[1, 2]', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => '', @@ -2107,6 +2358,7 @@ public static function get_formulas_question_data_testtwoandtwo(): stdClass { 'vars2' => '', 'answer' => '[3, 4]', 'answernotunique' => '1', + 'emptyallowed' => '0', 'correctness' => '_relerr < 0.01', 'unitpenalty' => 1, 'postunit' => '', diff --git a/tests/lexer_test.php b/tests/lexer_test.php index 1defa450..c1b9b317 100644 --- a/tests/lexer_test.php +++ b/tests/lexer_test.php @@ -426,6 +426,67 @@ public function test_get_token_list_7(): void { } } + /** + * Data provider. The test will assume the input starts with 'a =' and will automatically check for + * the 'a' (IDENTIFIER) and '=' (OPERATOR) token. For invalid input, we use the string of the expected + * error message. + * + * @return array + */ + public static function provide_inputs_with_empty(): array { + $string = new token(token::STRING, '$EMPTY'); + $empty = new token(token::EMPTY, '$EMPTY'); + $plus = new token(token::OPERATOR, '+'); + $errormessage = 'Invalid use of the dollar ($) symbol. It can only be used for the special value $EMPTY.'; + return [ + [[$empty], 'a = $EMPTY'], + [[$string], 'a = "$EMPTY"'], + [[$string], "a = '\$EMPTY'"], + [[$empty, $plus], 'a = $EMPTY+'], + ["Unexpected input: '.'", 'a = $EMPTY.'], + [$errormessage, 'a = $EMPT'], + [$errormessage, 'a = $EMPTYness'], + ]; + } + + /** + * Test whether lexing of the special $EMPTY token works as expected. + * + * @param array|string $expected array of tokens after the 'a =' part or expected error message + * @param string $input the input to be parsed + * @return void + * + * @dataProvider provide_inputs_with_empty + */ + public function test_get_token_list_with_empty_token($expected, $input): void { + $error = ''; + try { + $lexer = new lexer($input); + } catch (Exception $e) { + $error = $e->getMessage(); + } + + // If we expect an error, check the error message. + if (is_string($expected)) { + self::assertStringEndsWith($expected, $error); + return; + } + + // We did not expect an error. Check there was none and verify we got the right tokens. + self::assertEmpty($error); + $expectedlist = [ + new token(token::IDENTIFIER, 'a'), + new token(token::OPERATOR, '='), + ]; + $expectedlist = $expectedlist + $expected; + + $tokens = $lexer->get_tokens(); + foreach ($expectedlist as $i => $token) { + self::assertEquals($token->type, $tokens[$i]->type); + self::assertEquals($token->value, $tokens[$i]->value); + } + } + public function test_get_token_list_with_subsequent_comments(): void { $input = <<parts[2]->summarise_response($response); self::assertEquals('7', $summary2); $summary = $q->summarise_response($response); - self::assertEquals('5, 6, 7', $summary); + self::assertEquals('5; 6; 7', $summary); } public function test_summarise_response_test1(): void { @@ -978,7 +978,7 @@ public function test_summarise_response_test1(): void { $summary3 = $q->parts[3]->summarise_response($response); self::assertEquals('50', $summary3); $summary = $q->summarise_response($response); - self::assertEquals('30m/s, 20, m/s, 40, 50', $summary); + self::assertEquals('30m/s; 20, m/s; 40; 50', $summary); } public function test_is_complete_response_test3(): void { diff --git a/tests/questiontype_test.php b/tests/questiontype_test.php index 08aa5cf0..74be1c90 100644 --- a/tests/questiontype_test.php +++ b/tests/questiontype_test.php @@ -858,6 +858,7 @@ public function test_move_question_with_file_in_part($fieldname, $areaname): voi public static function provide_question_names(): array { return [ ['testsinglenum'], + ['testnumandempty'], ['testsinglenumunit'], ['testsinglenumunitsep'], ['testmethodsinparts'], diff --git a/tests/renderer_test.php b/tests/renderer_test.php index 029d00c7..2342729e 100644 --- a/tests/renderer_test.php +++ b/tests/renderer_test.php @@ -1074,6 +1074,7 @@ public function test_clear_wrong_with_combined_unit(): void { /** * Data provider. + * FIXME: add testnumandempty question * * @return Generator */ diff --git a/tests/walkthrough_adaptive_test.php b/tests/walkthrough_adaptive_test.php index 178a8b2e..8a9cd0b8 100644 --- a/tests/walkthrough_adaptive_test.php +++ b/tests/walkthrough_adaptive_test.php @@ -1097,4 +1097,98 @@ public function test_separate_unit_field_with_micro(): void { $this->process_submission(['0_0' => '1', '0_1' => 'µA', '-submit' => 1]); $this->check_current_mark(1); } + public function test_deferred_partially_answered_with_empty_allowed(): void { + // Create a question with one empty field. + $q = $this->get_test_formulas_question('testnumandempty'); + $this->start_attempt_at_question($q, 'deferredfeedback', 2); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->render(); + $this->check_output_contains_text_input('0_0'); + $this->check_output_contains_text_input('0_1'); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_does_not_contain_submit_button_expectation(), + $this->get_does_not_contain_feedback_expectation(), + ); + + // Submit the empty form. The question should be counted as "gave up" with no grade. + // The feedback should be "incorrect". + $this->finish(); + $this->check_current_state(question_state::$gaveup); + $this->check_current_mark(null); + $this->render(); + $this->check_output_contains_text_input('0_0', '', false); + $this->check_output_contains_text_input('0_1', '', false); + + // Submit a partial answer, filling only the first field, but wrong. The question should be + // graded wrong due to the grading criterion. The student's response should be shown in the + // text field. All text fields should be disabled. + $this->start_attempt_at_question($q, 'deferredfeedback', 2); + $this->check_current_state(question_state::$todo); + $this->process_submission(['0_0' => '5', '0_1' => '']); + $this->finish(); + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0); + $this->render(); + $this->check_output_contains_text_input('0_0', '5', false); + $this->check_output_contains_text_input('0_1', '', false); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_incorrect_expectation(), + ); + + // Submit an answer, filling only the first field, correct. The question should be + // graded correct. The student's response should be shown in the text field. + // All text fields should be disabled. + $this->start_attempt_at_question($q, 'deferredfeedback', 2); + $this->check_current_state(question_state::$todo); + $this->process_submission(['0_0' => '1', '0_1' => '']); + $this->finish(); + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(2); + $this->render(); + $this->check_output_contains_text_input('0_0', '1', false); + $this->check_output_contains_text_input('0_1', '', false); + $this->check_current_output( + $this->get_contains_mark_summary(2), + $this->get_contains_correct_expectation(), + ); + + // Submit an answer, filling only the second field, obviously wrong. The question should + // be graded wrong. The student's response should be shown in the text field. + // All text fields should be disabled. + $this->start_attempt_at_question($q, 'deferredfeedback', 2); + $this->check_current_state(question_state::$todo); + $this->process_submission(['0_0' => '', '0_1' => '1']); + $this->finish(); + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0); + $this->render(); + $this->check_output_contains_text_input('0_0', '', false); + $this->check_output_contains_text_input('0_1', '1', false); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_incorrect_expectation(), + ); + + // Submit an answer, filling both fields. The question should be graded wrong. + // The student's response should be shown in the text field. + // All text fields should be disabled. + $this->start_attempt_at_question($q, 'deferredfeedback', 2); + $this->check_current_state(question_state::$todo); + $this->process_submission(['0_0' => '1', '0_1' => '2']); + $this->finish(); + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0); + $this->render(); + $this->check_output_contains_text_input('0_0', '1', false); + $this->check_output_contains_text_input('0_1', '2', false); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_incorrect_expectation(), + ); + } }