From 8daabe65bdc3a08a5357be6a0e7a11bc0436afbf Mon Sep 17 00:00:00 2001
From: Chris Sangwin <C.J.Sangwin@ed.ac.uk>
Date: Tue, 28 Nov 2023 14:23:57 +0000
Subject: [PATCH] Fix to issue #1059: test case construction and decimal
 separators.

---
 doc/en/Authoring/Testing.md | 5 ++++-
 questiontestedit.php        | 4 ++--
 questiontestrun.php         | 2 ++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/doc/en/Authoring/Testing.md b/doc/en/Authoring/Testing.md
index 8f9459687..bde887104 100644
--- a/doc/en/Authoring/Testing.md
+++ b/doc/en/Authoring/Testing.md
@@ -94,10 +94,13 @@ For the checkbox type you will need the whole list.
 
     tc1:mcq_correct(ta);
 
-### Test case construction and numerical precision.
+### Test case construction and numerical precision
 
 You can construct test cases using the functions such as `dispdp` to create a test-case input with trailing zeros.  This is neeeded if the input, or answer test, is testing for a minimum number of decimal places or significant figures.
 
+### Test case construction and decimal separators
+
+The decimal separator option (e.g. `.` or `,`) is a very thin layer based on the student input.  The teacher must always use a `.` (full stop) as the decimal separator in question variables.  Consistent with this, all test-case construction must use a `.` (full stop) as the decimal separator.  This means it's hard to test the functionality of the decimal separator option (sorry), but otherwise there is genuine confusion in the internal logic about _when_ to assume a `,` is a decimal separator or a list separator.  Also, if you change this option in the question you do not need to change all your test cases.
 
 ## Testing values of variables
 
diff --git a/questiontestedit.php b/questiontestedit.php
index 45333c8a5..c47b9a97c 100644
--- a/questiontestedit.php
+++ b/questiontestedit.php
@@ -39,6 +39,8 @@ $confirmthistestcase = optional_param('confirmthistestcase', null, PARAM_INT);
 // Load the necessary data.
 $questiondata = $DB->get_record('question', array('id' => $questionid), '*', MUST_EXIST);
 $question = question_bank::load_question($questionid);
+// We hard-wire decimals to be a full stop when testing questions.
+$question->options->set_option('decimals', '.');
 if ($testcase || $confirmthistestcase) {
     $qtest = question_bank::get_qtype('stack')->load_question_test($questionid, $testcase);
 }
@@ -71,7 +73,6 @@ if (!is_null($seed)) {
 $slot = $quba->add_question($question, $question->defaultmark);
 $quba->start_question($slot);
 
-
 // Initialise $PAGE.
 $backurl = new moodle_url('/question/type/stack/questiontestrun.php', $urlparams);
 if (!is_null($testcase)) {
@@ -125,7 +126,6 @@ if ($mform->is_cancelled()) {
         $inputs[$name] = $value;
     }
     $qtest = new stack_question_test($qtest->description, $inputs);
-
     $response = stack_question_test::compute_response($question, $inputs);
 
     foreach ($question->prts as $prtname => $prt) {
diff --git a/questiontestrun.php b/questiontestrun.php
index 9f194608d..29927cb18 100644
--- a/questiontestrun.php
+++ b/questiontestrun.php
@@ -68,6 +68,8 @@ if (!$questiondata) {
     throw new stack_exception('questiondoesnotexist');
 }
 $question = question_bank::load_question($questionid);
+// We hard-wire decimals to be a full stop when testing questions.
+$question->options->set_option('decimals', '.');
 
 // Process any other URL parameters, and do require_login.
 list($context, $seed, $urlparams) = qtype_stack_setup_question_test_page($question);
-- 
GitLab