From 49cf79068504ef8791b0770b533131db4d545dca Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 15 Jan 2024 13:10:17 +1300 Subject: [PATCH 01/15] Added caching of grader results. Uses a Moolde file cache to store results of checked questions. The key is made up of the question id, template id, random seed, student answser and attachements (if any). --- .gitignore | 2 +- classes/jobrunner.php | 81 +++++++++++++++++++++++++++++++----- db/caches.php | 36 ++++++++++++++++ edit_coderunner_form.php | 6 ++- lang/en/qtype_coderunner.php | 1 + question.php | 54 ++++++++++++++++++++++-- 6 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 db/caches.php diff --git a/.gitignore b/.gitignore index 67f0e8831..51230817d 100644 --- a/.gitignore +++ b/.gitignore @@ -34,4 +34,4 @@ NonRepoFiles/* /amd/src/ui_blockly.json /amd/src/.eslintrc.js .grunt - +.vscode/ \ No newline at end of file diff --git a/classes/jobrunner.php b/classes/jobrunner.php index c323fbc6e..7a297f520 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -36,15 +36,32 @@ class qtype_coderunner_jobrunner { private $testcases = null; // The testcases (a subset of those in the question). private $allruns = null; // Array of the source code for all runs. - // Check the correctness of a student's code and possible extra attachments - // as an answer to the given - // question and and a given set of test cases (which may be empty or a - // subset of the question's set of testcases. $isprecheck is true if - // this is a run triggered by the student clicking the Precheck button. - // $answerlanguage will be the empty string except for multilanguage questions, - // when it is the language selected in the language drop-down menu. - // Returns a TestingOutcome object. - public function run_tests($question, $code, $attachments, $testcases, $isprecheck, $answerlanguage) { + /** + * Check the correctness of a student's code and possible extra attachments + * as an answer to the given question and and a given set of test cases (which may be empty or a + * subset of the question's set of testcases. + * @param qtype_coderunner_question $question object relevant to this step of the attempt + * @param string $code is the JSON repr of the code + * @param array $attachments is the array of attachments given by student, if any + * @param + * @param boolean $isprecheck is true if + * this is a run triggered by the student clicking the Precheck button. + * @param string $answerlanguage will be the empty string except for multilanguage questions, + * when it is the language selected in the language drop-down menu. + * @param boolean $validating Set to true if this is a validate on grading. Otherwise the + * validate on save uses the qid of the last version. + * @return qtype_coderunner_combinator_grader_outcome $testoutcome that contains the outcome + * of the grading. + */ + public function run_tests( + $question, + $code, + $attachments, + $testcases, + $isprecheck, + $answerlanguage, + $validating = false + ) { if (empty($question->prototype)) { // Missing prototype. We can't run this question. $outcome = new qtype_coderunner_testing_outcome(0, 0, false); @@ -57,10 +74,45 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec ['crtype' => $question->coderunnertype] ); } - $outcome->set_status(qtype_coderunner_testing_outcome::STATUS_MISSING_PROTOTYPE, $message); + $status = qtype_coderunner_testing_outcome::STATUS_MISSING_PROTOTYPE; + $outcome->set_status($status, $message); return $outcome; } + /* Validate on save will set validating to true. + This is needed otherwise the validate run uses the old question id + and the cache wouldn't think there was any change. + Note: Could add a coderunner config variable to allow for ignoring + cache - and maybe updating with the newly calculated outcomes. + This would guarantee no hash collisions but would only be used + when not under time pressue, eg, after a test/exam has finished. + Coul just purge the coderunner cache but this would nuke caches + for all quizzes... + */ + if (!$validating && false) { + // Use cached outcome if we have one. + $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + + // With same qid and prototype id the question text and test cases can't have changed. + // Maybe the answerlanguage can't have changed either? so we could remove that from the hash. + $qdetails = "qid=" . $question->id . ",prototype_id=" . $question->prototype->id; + $qdetails .= ",isprecheck=" . $isprecheck . ",answerlanguage=" . $answerlanguage; + + // Most student answers won't have attachments but if they do then ... + // ... include the hash of the attachments (which will be slow). + if ($attachments) { + $qdetails .= "attachmentshash=" . hash(" xxh64", serialize($attachments)); + } + + $rawkey = $qdetails . ",code=" . $code; + $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. + $outcome = $cache->get($key); + if ($outcome) { + // echo ("-from cache- " . $question->id . ", " . $question->prototype->id . "
"); + return unserialize($outcome); + } + } + // Extract the code from JSON if this is a Scratchpad UI or similar. // Note that this breaks the old python3_scratchpad question type // where the template expects the full JSON string to be presented as @@ -119,10 +171,17 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec if ($question->get_show_source()) { $outcome->sourcecodelist = $this->allruns; } + + // Only cache result if not doing a validate on save. + if (!$validating && false) { + // Cache the result for this qid+protid+code+isprecheck+language so we can use it again if needed. + $cache->set($key, serialize($outcome)); + } + // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); + return $outcome; } - // If the template is a combinator, try running all the tests in a single // go. // diff --git a/db/caches.php b/db/caches.php new file mode 100644 index 000000000..00dbc8c1c --- /dev/null +++ b/db/caches.php @@ -0,0 +1,36 @@ +. + +/** + * Defines cache used to store results of quiz attempt steps. + * If a jobe submission is cached we don't need to call jobe again + * as the result will be known :) + * + * @package qtype + * @subpackage coderunner + * @copyright 2023 Paul McKeown, University of Canterbury + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$definitions = [ + 'coderunner_grading_cache' => [ + 'mode' => cache_store::MODE_APPLICATION, + 'simpledata' => true, + 'canuselocalstore' => true, + ], +]; diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index 0d74cc224..0af48cb0b 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1616,7 +1616,11 @@ private function validate_sample_answer() { if ($error) { return $error; } - [$mark, , $cachedata] = $this->formquestion->grade_response($response); + [$mark, , $cachedata] = $this->formquestion->grade_response( + $response, + isprecheck:false, + validating:true + ); } catch (Exception $e) { return $e->getMessage(); } diff --git a/lang/en/qtype_coderunner.php b/lang/en/qtype_coderunner.php index d3fd272e3..a846d4a41 100644 --- a/lang/en/qtype_coderunner.php +++ b/lang/en/qtype_coderunner.php @@ -1333,3 +1333,4 @@ function should be applied, e.g. {{STUDENT_ANSWER | e(\'py\')}} is $string['wssubmissionrateexceeded'] = 'You have exceeded the maximum hourly \'Try it!\' submission rate. Request denied.'; $string['xmlcoderunnerformaterror'] = 'XML format error in coderunner question'; +$string['coderunner_grading_cache'] = 'Caches grading results so we can avoid going to Jobe so often'; diff --git a/question.php b/question.php index a2f20c3fd..3b959bcf8 100644 --- a/question.php +++ b/question.php @@ -69,6 +69,7 @@ public function start_attempt(question_attempt_step $step = null, $variant = nul $step->set_qt_var('_mtrandseed', $seed); } $this->evaluate_question_for_display($seed, $step); + $this->mtrandseed = $seed; // so we can see it when checking } // Retrieve the saved random number seed and reconstruct the template @@ -84,6 +85,7 @@ public function apply_attempt_state(question_attempt_step $step) { $seed = mt_rand(); } $this->evaluate_question_for_display($seed, $step); + $this->mtrandseed = $seed; // so we can see it when checking } @@ -643,19 +645,22 @@ public function display_feedback() { * state is used when a sandbox error occurs. * @throws coding_exception */ - public function grade_response(array $response, bool $isprecheck = false) { + public function grade_response(array $response, bool $isprecheck = false, $validating = false) { if ($isprecheck && empty($this->precheck)) { throw new coding_exception("Unexpected precheck"); } $language = empty($response['language']) ? '' : $response['language']; + + // QUESTION: Could we replace this with the new cached version? $gradingreqd = true; + $testoutcomeserial = null; if (!empty($response['_testoutcome'])) { $testoutcomeserial = $response['_testoutcome']; $testoutcome = unserialize($testoutcomeserial); if ( $testoutcome instanceof qtype_coderunner_testing_outcome // Ignore legacy-format outcomes. && $testoutcome->isprecheck == $isprecheck - ) { + ) { $gradingreqd = false; // Already graded and with same precheck state. } } @@ -674,8 +679,49 @@ public function grade_response(array $response, bool $isprecheck = false) { } else { $this->stepinfo->graderstate = ''; } - $testoutcome = $runner->run_tests($this, $code, $attachments, $testcases, $isprecheck, $language); - $testoutcomeserial = serialize($testoutcome); + + // Use cached outcome if we have one. + $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + + // QUESTION: Should we raise an error on missing prototype here?... + // ... Rather than in jobrunner?. + if (!empty($this->prototype)) { + $protoid = $this->prototype->id; + } else { + $protoid = ""; + } + $seed = $this->mtrandseed; + // With same qid and prototype id the question text and seed, test cases can't have changed. + $qdetails = "qid=" . $this->id . "ren,prototype_id=" . $protoid; + $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; + // Most student answers won't have attachments but if they do then ... + // ... include the hash of the attachments (which will be slow). + if ($attachments) { + $qdetails .= ",attachmentshash=" . hash("xxh64", serialize($attachments)); + } + $rawkey = $qdetails . ",code=" . $code; + $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. + if (!$validating) { + $testoutcomeserial = $cache->get($key); + if ($testoutcomeserial) { + $testoutcome = unserialize($testoutcomeserial); + } + } + if ($testoutcomeserial == null) { + $testoutcome = $runner->run_tests( + $this, + $code, + $attachments, + $testcases, + $isprecheck, + $language, + $validating + ); + $testoutcomeserial = serialize($testoutcome); + if (!$validating) { + $cache->set($key, $testoutcomeserial); + } + } } $datatocache = ['_testoutcome' => $testoutcomeserial]; From 2fdb0ea5fab7097a367876998ce7df42376dd84c Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 29 Jan 2024 13:45:07 +1300 Subject: [PATCH 02/15] Response caching code for testing on a real server. Code still under development. --- bulktestindex.php | 2 +- classes/jobrunner.php | 43 +------------- db/caches.php | 4 ++ edit_coderunner_form.php | 2 +- lang/en/qtype_coderunner.php | 3 + question.php | 103 +++++++++++++++++++++++---------- settings.php | 7 +++ tests/grader_test.php | 9 ++- tests/pythonquestions_test.php | 23 +++++--- tests/template_test.php | 6 +- 10 files changed, 117 insertions(+), 85 deletions(-) diff --git a/bulktestindex.php b/bulktestindex.php index e092c22f7..a4cb26dc1 100644 --- a/bulktestindex.php +++ b/bulktestindex.php @@ -150,7 +150,7 @@ expander.innerHTML = 'Collapse'; expander.nextSibling.style.display = 'inline'; } else { - expander.innerTHML = 'Expand'; + expander.innerHTML = 'Expand'; expander.nextSibling.style.display = 'none'; } }); diff --git a/classes/jobrunner.php b/classes/jobrunner.php index 7a297f520..8b5291b53 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -59,8 +59,7 @@ public function run_tests( $attachments, $testcases, $isprecheck, - $answerlanguage, - $validating = false + $answerlanguage ) { if (empty($question->prototype)) { // Missing prototype. We can't run this question. @@ -79,40 +78,6 @@ public function run_tests( return $outcome; } - /* Validate on save will set validating to true. - This is needed otherwise the validate run uses the old question id - and the cache wouldn't think there was any change. - Note: Could add a coderunner config variable to allow for ignoring - cache - and maybe updating with the newly calculated outcomes. - This would guarantee no hash collisions but would only be used - when not under time pressue, eg, after a test/exam has finished. - Coul just purge the coderunner cache but this would nuke caches - for all quizzes... - */ - if (!$validating && false) { - // Use cached outcome if we have one. - $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); - - // With same qid and prototype id the question text and test cases can't have changed. - // Maybe the answerlanguage can't have changed either? so we could remove that from the hash. - $qdetails = "qid=" . $question->id . ",prototype_id=" . $question->prototype->id; - $qdetails .= ",isprecheck=" . $isprecheck . ",answerlanguage=" . $answerlanguage; - - // Most student answers won't have attachments but if they do then ... - // ... include the hash of the attachments (which will be slow). - if ($attachments) { - $qdetails .= "attachmentshash=" . hash(" xxh64", serialize($attachments)); - } - - $rawkey = $qdetails . ",code=" . $code; - $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. - $outcome = $cache->get($key); - if ($outcome) { - // echo ("-from cache- " . $question->id . ", " . $question->prototype->id . "
"); - return unserialize($outcome); - } - } - // Extract the code from JSON if this is a Scratchpad UI or similar. // Note that this breaks the old python3_scratchpad question type // where the template expects the full JSON string to be presented as @@ -172,12 +137,6 @@ public function run_tests( $outcome->sourcecodelist = $this->allruns; } - // Only cache result if not doing a validate on save. - if (!$validating && false) { - // Cache the result for this qid+protid+code+isprecheck+language so we can use it again if needed. - $cache->set($key, serialize($outcome)); - } - // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); return $outcome; } diff --git a/db/caches.php b/db/caches.php index 00dbc8c1c..9df3b1502 100644 --- a/db/caches.php +++ b/db/caches.php @@ -30,7 +30,11 @@ $definitions = [ 'coderunner_grading_cache' => [ 'mode' => cache_store::MODE_APPLICATION, + 'maxsize' => 1000000, + 'simplekeys' => true, 'simpledata' => true, 'canuselocalstore' => true, + 'staticacceleration' => true, + 'staticaccelerationsize' => 500000, ], ]; diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index 0af48cb0b..e7bfd4db5 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1619,7 +1619,7 @@ private function validate_sample_answer() { [$mark, , $cachedata] = $this->formquestion->grade_response( $response, isprecheck:false, - validating:true + usecache:false ); } catch (Exception $e) { return $e->getMessage(); diff --git a/lang/en/qtype_coderunner.php b/lang/en/qtype_coderunner.php index a846d4a41..55d476e12 100644 --- a/lang/en/qtype_coderunner.php +++ b/lang/en/qtype_coderunner.php @@ -1334,3 +1334,6 @@ function should be applied, e.g. {{STUDENT_ANSWER | e(\'py\')}} is $string['xmlcoderunnerformaterror'] = 'XML format error in coderunner question'; $string['coderunner_grading_cache'] = 'Caches grading results so we can avoid going to Jobe so often'; +$string['cachedef_coderunner_grading_cache'] = 'Caches grading results so we can avoid going to Jobe so often'; +$string['cachegradingresultsenable'] = 'Cache results when grading answers.'; +$string['cachegradingresults_desc'] = 'Uses a local Moodle cache (currently file cache) to store results of grading questions. Mainly to speed up regrading by using cached results for steps where the same quesiton and answer have already been graded.'; diff --git a/question.php b/question.php index 3b959bcf8..1bfd6567f 100644 --- a/question.php +++ b/question.php @@ -38,6 +38,7 @@ class qtype_coderunner_question extends question_graded_automatically { public $testcases = null; // Array of testcases. + /** * Start a new attempt at this question, storing any information that will * be needed later in the step. It is retrieved and applied by @@ -150,6 +151,19 @@ public function evaluate_question_for_display($seed, $step) { * md5 hash of the template parameters within the question attempt step * record in the database, re-evaluating only if the hash changes. * + * + * QUESTION + * With question versioning the question's template paremters can't + * change between steps because the question id will be fixed. + * So, do we need the md5 for the question's template params? + * I'm not sure if the prototype id is fixed at the start of the question + * attempt. If so then we wouldn't be able to change the template + * during an attempt either + * + * Of course, doing a regrade will potentially change everything but + * this will work it's way through as the regrade will start from the + * first step and work through again... + * * If the prototype is missing, process just the template parameters from * this question; an error message will be given later. * @param int $seed The random number seed to set for Twig randomisation @@ -178,6 +192,9 @@ public function evaluate_merged_parameters($seed, $step = null) { /** * Evaluate the template parameter field for this question alone (i.e. * not including its prototype). + * Note: the prototype is also a question and will cache it's own jsontemplateparams + * eg, we call $prototype->template_params_json(..,.., '_prototype_template_params') in + * the evaulate_merged_parameters method. * * @param int $seed the random number seed for this instance of the question * @param question_attempt_step $step the current attempt step @@ -190,6 +207,7 @@ public function template_params_json($seed = 0, $step = null, $qtvar = '') { $params = $this->templateparams; $lang = $this->templateparamslang; if ($step === null) { + // Step is null when validating question so evaluate params. $jsontemplateparams = $this->evaluate_template_params($params, $lang, $seed); } else { $previousparamsmd5 = $step->get_qt_var($qtvar . '_md5'); @@ -242,7 +260,14 @@ public function evaluate_template_params($templateparams, $lang, $seed) { } else { $jsontemplateparams = $this->evaluate_template_params_on_jobe($templateparams, $lang, $seed); } + + // Cache in this to avoid multiple evaluations during question editing and validation. + /* + ************** + QUESTION: Seems to always store in cache even when it was read from cache in the first place. + ************** + */ $this->cachedfuncparams = ['lang' => $lang, 'seed' => $seed]; $this->cachedevaldtemplateparams = $jsontemplateparams; return $jsontemplateparams; @@ -645,22 +670,28 @@ public function display_feedback() { * state is used when a sandbox error occurs. * @throws coding_exception */ - public function grade_response(array $response, bool $isprecheck = false, $validating = false) { + public function grade_response(array $response, bool $isprecheck = false, $usecache = true) { if ($isprecheck && empty($this->precheck)) { throw new coding_exception("Unexpected precheck"); } - $language = empty($response['language']) ? '' : $response['language']; + + // Trying ?? $language = empty($response['language']) ? '' : $response['language'];. + $language = $response['language'] ?? ""; + // Was $language = empty($response['language']) ? '' : $response['language'];. // QUESTION: Could we replace this with the new cached version? $gradingreqd = true; $testoutcomeserial = null; + + // Use _testoutcome if it's already in $response. + // This should be even quicker than the file cache. if (!empty($response['_testoutcome'])) { $testoutcomeserial = $response['_testoutcome']; $testoutcome = unserialize($testoutcomeserial); if ( $testoutcome instanceof qtype_coderunner_testing_outcome // Ignore legacy-format outcomes. && $testoutcome->isprecheck == $isprecheck - ) { + ) { $gradingreqd = false; // Already graded and with same precheck state. } } @@ -671,17 +702,6 @@ public function grade_response(array $response, bool $isprecheck = false, $valid // filenames and values being file contents. $code = $response['answer']; $attachments = $this->get_attached_files($response); - $testcases = $this->filter_testcases($isprecheck, $this->precheck); - $runner = new qtype_coderunner_jobrunner(); - $this->stepinfo = self::step_info($response); - if (isset($response['graderstate'])) { - $this->stepinfo->graderstate = $response['graderstate']; - } else { - $this->stepinfo->graderstate = ''; - } - - // Use cached outcome if we have one. - $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); // QUESTION: Should we raise an error on missing prototype here?... // ... Rather than in jobrunner?. @@ -690,41 +710,66 @@ public function grade_response(array $response, bool $isprecheck = false, $valid } else { $protoid = ""; } - $seed = $this->mtrandseed; - // With same qid and prototype id the question text and seed, test cases can't have changed. - $qdetails = "qid=" . $this->id . "ren,prototype_id=" . $protoid; - $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; - // Most student answers won't have attachments but if they do then ... - // ... include the hash of the attachments (which will be slow). - if ($attachments) { - $qdetails .= ",attachmentshash=" . hash("xxh64", serialize($attachments)); - } - $rawkey = $qdetails . ",code=" . $code; - $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. - if (!$validating) { + + + if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { + // Use cached outcome if we have one. + $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + + $qid = $this->id; + $seed = $this->mtrandseed; + // With same qid and prototype id the question text and seed, test cases can't have changed. + $qdetails = "qid=" . $qid . ",prototype_id=" . $protoid; + $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; + $qdetails .= ",language=" . $language; + // Also note that prechecksetting can only change if qid or prototype id has changed. + + // Most student answers won't have attachments but if they do then ... + // ... include the hash of the attachments (which will be slow). + if ($attachments) { // was xxh64 + $qdetails .= ",attachmentshash=" . hash("md5", serialize($attachments)); + } + $rawkey = $qdetails . ",code=" . $code; + $key = hash("md5", $rawkey); // Fast with digest small enough to be used directly as key in DB. + // Can't use cache for validate on save as it uses the old question id ... + // ... rather than the new one. And caching would be pointless anyway! + $testoutcomeserial = $cache->get($key); if ($testoutcomeserial) { $testoutcome = unserialize($testoutcomeserial); } } + if ($testoutcomeserial == null) { + $this->stepinfo = self::step_info($response); + $this->stepinfo->graderstate = $response['graderstate'] ?? ""; + // if (isset($response['graderstate'])) { + // $this->stepinfo->graderstate = $response['graderstate']; + // } else { + // $this->stepinfo->graderstate = ''; + // } + $testcases = $this->filter_testcases($isprecheck, $this->precheck); + $runner = new qtype_coderunner_jobrunner(); $testoutcome = $runner->run_tests( $this, $code, $attachments, $testcases, $isprecheck, - $language, - $validating + $language ); $testoutcomeserial = serialize($testoutcome); - if (!$validating) { + // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); + if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { $cache->set($key, $testoutcomeserial); } } } + // To be saved in question step data. + // Note: This is used to render test results too so it's not just a cache. $datatocache = ['_testoutcome' => $testoutcomeserial]; + if ($testoutcome->run_failed()) { return [0, question_state::$invalid, $datatocache]; } else if ($testoutcome->all_correct()) { diff --git a/settings.php b/settings.php index ad3ee03dc..60a535cbe 100644 --- a/settings.php +++ b/settings.php @@ -134,3 +134,10 @@ get_string('wsmaxcputime_desc', 'qtype_coderunner'), '5' )); + +$settings->add(new admin_setting_configcheckbox( + "qtype_coderunner/cachegradingresults", + get_string('cachegradingresultsenable', 'qtype_coderunner'), + get_string('cachegradingresults_desc', 'qtype_coderunner'), + false +)); diff --git a/tests/grader_test.php b/tests/grader_test.php index 5f7a1eed8..5097f00d0 100644 --- a/tests/grader_test.php +++ b/tests/grader_test.php @@ -63,7 +63,8 @@ public function test_regex_grader() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -94,7 +95,8 @@ public function test_nearequality_grader_right_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -125,7 +127,8 @@ public function test_nearequality_grader_wrong_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(0, $mark); diff --git a/tests/pythonquestions_test.php b/tests/pythonquestions_test.php index fb8a17d62..eca0c1162 100644 --- a/tests/pythonquestions_test.php +++ b/tests/pythonquestions_test.php @@ -211,7 +211,8 @@ public function test_timeout() { $q = $this->make_question('timeout'); $code = "def timeout():\n while (1):\n pass"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); @@ -227,7 +228,8 @@ public function test_exceptions() { $q = $this->make_question('exceptions'); $code = "def checkOdd(n):\n if n & 1:\n raise ValueError()"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); @@ -253,21 +255,24 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return 0"; // Passes first test only. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); $code = "def sqr(n):\n return n * n if n <= 0 else -17.995"; // Passes first test and last two only. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 5.0 / 7.5) < 0.00001); $code = "def sqr(n):\n return n * n if n <= 0 else 1 / 0"; // Passes first test then aborts. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); @@ -281,11 +286,15 @@ public function test_customised_timeout() { print("Hello Python") EOT; $response = ['answer' => $slowsquare]; // Should time out. - [$mark, $grade, $cache] = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); + [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); $q->cputimelimitsecs = 20; // This should fix it. - [$mark, $grade, $cache] = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); + [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); } diff --git a/tests/template_test.php b/tests/template_test.php index d821311bb..31e1803b3 100644 --- a/tests/template_test.php +++ b/tests/template_test.php @@ -111,11 +111,13 @@ public function test_grading_template() { $q->allornothing = false; $code = "def sqr(n): return n * n\n"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertTrue(abs($mark - 24.0 / 31.0) < 0.000001); $q->allornothing = true; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertTrue($mark == 0.0); } From d526eb332dfb3e83360afef20299c636cec57c73 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 29 Jan 2024 13:45:07 +1300 Subject: [PATCH 03/15] Response caching code for testing on a real server. Code still under development. --- bulktestindex.php | 2 +- classes/jobrunner.php | 43 +------------- db/caches.php | 4 ++ edit_coderunner_form.php | 6 +- lang/en/qtype_coderunner.php | 3 + question.php | 105 ++++++++++++++++++++++++--------- settings.php | 7 +++ tests/grader_test.php | 9 ++- tests/pythonquestions_test.php | 23 +++++--- tests/template_test.php | 6 +- 10 files changed, 121 insertions(+), 87 deletions(-) diff --git a/bulktestindex.php b/bulktestindex.php index e092c22f7..a4cb26dc1 100644 --- a/bulktestindex.php +++ b/bulktestindex.php @@ -150,7 +150,7 @@ expander.innerHTML = 'Collapse'; expander.nextSibling.style.display = 'inline'; } else { - expander.innerTHML = 'Expand'; + expander.innerHTML = 'Expand'; expander.nextSibling.style.display = 'none'; } }); diff --git a/classes/jobrunner.php b/classes/jobrunner.php index 7a297f520..8b5291b53 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -59,8 +59,7 @@ public function run_tests( $attachments, $testcases, $isprecheck, - $answerlanguage, - $validating = false + $answerlanguage ) { if (empty($question->prototype)) { // Missing prototype. We can't run this question. @@ -79,40 +78,6 @@ public function run_tests( return $outcome; } - /* Validate on save will set validating to true. - This is needed otherwise the validate run uses the old question id - and the cache wouldn't think there was any change. - Note: Could add a coderunner config variable to allow for ignoring - cache - and maybe updating with the newly calculated outcomes. - This would guarantee no hash collisions but would only be used - when not under time pressue, eg, after a test/exam has finished. - Coul just purge the coderunner cache but this would nuke caches - for all quizzes... - */ - if (!$validating && false) { - // Use cached outcome if we have one. - $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); - - // With same qid and prototype id the question text and test cases can't have changed. - // Maybe the answerlanguage can't have changed either? so we could remove that from the hash. - $qdetails = "qid=" . $question->id . ",prototype_id=" . $question->prototype->id; - $qdetails .= ",isprecheck=" . $isprecheck . ",answerlanguage=" . $answerlanguage; - - // Most student answers won't have attachments but if they do then ... - // ... include the hash of the attachments (which will be slow). - if ($attachments) { - $qdetails .= "attachmentshash=" . hash(" xxh64", serialize($attachments)); - } - - $rawkey = $qdetails . ",code=" . $code; - $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. - $outcome = $cache->get($key); - if ($outcome) { - // echo ("-from cache- " . $question->id . ", " . $question->prototype->id . "
"); - return unserialize($outcome); - } - } - // Extract the code from JSON if this is a Scratchpad UI or similar. // Note that this breaks the old python3_scratchpad question type // where the template expects the full JSON string to be presented as @@ -172,12 +137,6 @@ public function run_tests( $outcome->sourcecodelist = $this->allruns; } - // Only cache result if not doing a validate on save. - if (!$validating && false) { - // Cache the result for this qid+protid+code+isprecheck+language so we can use it again if needed. - $cache->set($key, serialize($outcome)); - } - // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); return $outcome; } diff --git a/db/caches.php b/db/caches.php index 00dbc8c1c..9df3b1502 100644 --- a/db/caches.php +++ b/db/caches.php @@ -30,7 +30,11 @@ $definitions = [ 'coderunner_grading_cache' => [ 'mode' => cache_store::MODE_APPLICATION, + 'maxsize' => 1000000, + 'simplekeys' => true, 'simpledata' => true, 'canuselocalstore' => true, + 'staticacceleration' => true, + 'staticaccelerationsize' => 500000, ], ]; diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index 0af48cb0b..d4ab0a86b 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1616,10 +1616,10 @@ private function validate_sample_answer() { if ($error) { return $error; } - [$mark, , $cachedata] = $this->formquestion->grade_response( + [$mark, $state, $cachedata] = $this->formquestion->grade_response( $response, - isprecheck:false, - validating:true + false, // Not a precheck. + false // Don't use cache as it the old question id is used and would give old outcome. ); } catch (Exception $e) { return $e->getMessage(); diff --git a/lang/en/qtype_coderunner.php b/lang/en/qtype_coderunner.php index a846d4a41..55d476e12 100644 --- a/lang/en/qtype_coderunner.php +++ b/lang/en/qtype_coderunner.php @@ -1334,3 +1334,6 @@ function should be applied, e.g. {{STUDENT_ANSWER | e(\'py\')}} is $string['xmlcoderunnerformaterror'] = 'XML format error in coderunner question'; $string['coderunner_grading_cache'] = 'Caches grading results so we can avoid going to Jobe so often'; +$string['cachedef_coderunner_grading_cache'] = 'Caches grading results so we can avoid going to Jobe so often'; +$string['cachegradingresultsenable'] = 'Cache results when grading answers.'; +$string['cachegradingresults_desc'] = 'Uses a local Moodle cache (currently file cache) to store results of grading questions. Mainly to speed up regrading by using cached results for steps where the same quesiton and answer have already been graded.'; diff --git a/question.php b/question.php index 3b959bcf8..b1baab257 100644 --- a/question.php +++ b/question.php @@ -38,6 +38,7 @@ class qtype_coderunner_question extends question_graded_automatically { public $testcases = null; // Array of testcases. + /** * Start a new attempt at this question, storing any information that will * be needed later in the step. It is retrieved and applied by @@ -150,6 +151,19 @@ public function evaluate_question_for_display($seed, $step) { * md5 hash of the template parameters within the question attempt step * record in the database, re-evaluating only if the hash changes. * + * + * QUESTION + * With question versioning the question's template paremters can't + * change between steps because the question id will be fixed. + * So, do we need the md5 for the question's template params? + * I'm not sure if the prototype id is fixed at the start of the question + * attempt. If so then we wouldn't be able to change the template + * during an attempt either + * + * Of course, doing a regrade will potentially change everything but + * this will work it's way through as the regrade will start from the + * first step and work through again... + * * If the prototype is missing, process just the template parameters from * this question; an error message will be given later. * @param int $seed The random number seed to set for Twig randomisation @@ -178,6 +192,9 @@ public function evaluate_merged_parameters($seed, $step = null) { /** * Evaluate the template parameter field for this question alone (i.e. * not including its prototype). + * Note: the prototype is also a question and will cache it's own jsontemplateparams + * eg, we call $prototype->template_params_json(..,.., '_prototype_template_params') in + * the evaulate_merged_parameters method. * * @param int $seed the random number seed for this instance of the question * @param question_attempt_step $step the current attempt step @@ -190,6 +207,7 @@ public function template_params_json($seed = 0, $step = null, $qtvar = '') { $params = $this->templateparams; $lang = $this->templateparamslang; if ($step === null) { + // Step is null when validating question so evaluate params. $jsontemplateparams = $this->evaluate_template_params($params, $lang, $seed); } else { $previousparamsmd5 = $step->get_qt_var($qtvar . '_md5'); @@ -242,7 +260,14 @@ public function evaluate_template_params($templateparams, $lang, $seed) { } else { $jsontemplateparams = $this->evaluate_template_params_on_jobe($templateparams, $lang, $seed); } + + // Cache in this to avoid multiple evaluations during question editing and validation. + /* + ************** + QUESTION: Seems to always store in cache even when it was read from cache in the first place. + ************** + */ $this->cachedfuncparams = ['lang' => $lang, 'seed' => $seed]; $this->cachedevaldtemplateparams = $jsontemplateparams; return $jsontemplateparams; @@ -639,28 +664,36 @@ public function display_feedback() { * the history of prior submissions. * @param bool $isprecheck true iff this grading is occurring because the * student clicked the precheck button + * @param bool $usecache If true (and the coderunner cachegradingresults setting is also true) then + * cache results in coderunner cache and use results from the coderunner grading cache. * @return 3-element array of the mark (0 - 1), the question_state ( * gradedright, gradedwrong, gradedpartial, invalid) and the full * qtype_coderunner_testing_outcome object to be cached. The invalid * state is used when a sandbox error occurs. * @throws coding_exception */ - public function grade_response(array $response, bool $isprecheck = false, $validating = false) { + public function grade_response(array $response, bool $isprecheck = false, $usecache = true) { if ($isprecheck && empty($this->precheck)) { throw new coding_exception("Unexpected precheck"); } - $language = empty($response['language']) ? '' : $response['language']; + + // Trying ?? $language = empty($response['language']) ? '' : $response['language'];. + $language = $response['language'] ?? ""; + // Was $language = empty($response['language']) ? '' : $response['language'];. // QUESTION: Could we replace this with the new cached version? $gradingreqd = true; $testoutcomeserial = null; + + // Use _testoutcome if it's already in $response. + // This should be even quicker than the file cache. if (!empty($response['_testoutcome'])) { $testoutcomeserial = $response['_testoutcome']; $testoutcome = unserialize($testoutcomeserial); if ( $testoutcome instanceof qtype_coderunner_testing_outcome // Ignore legacy-format outcomes. && $testoutcome->isprecheck == $isprecheck - ) { + ) { $gradingreqd = false; // Already graded and with same precheck state. } } @@ -671,17 +704,6 @@ public function grade_response(array $response, bool $isprecheck = false, $valid // filenames and values being file contents. $code = $response['answer']; $attachments = $this->get_attached_files($response); - $testcases = $this->filter_testcases($isprecheck, $this->precheck); - $runner = new qtype_coderunner_jobrunner(); - $this->stepinfo = self::step_info($response); - if (isset($response['graderstate'])) { - $this->stepinfo->graderstate = $response['graderstate']; - } else { - $this->stepinfo->graderstate = ''; - } - - // Use cached outcome if we have one. - $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); // QUESTION: Should we raise an error on missing prototype here?... // ... Rather than in jobrunner?. @@ -690,41 +712,66 @@ public function grade_response(array $response, bool $isprecheck = false, $valid } else { $protoid = ""; } - $seed = $this->mtrandseed; - // With same qid and prototype id the question text and seed, test cases can't have changed. - $qdetails = "qid=" . $this->id . "ren,prototype_id=" . $protoid; - $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; - // Most student answers won't have attachments but if they do then ... - // ... include the hash of the attachments (which will be slow). - if ($attachments) { - $qdetails .= ",attachmentshash=" . hash("xxh64", serialize($attachments)); - } - $rawkey = $qdetails . ",code=" . $code; - $key = hash("xxh64", $rawkey); // Fast with digest small enough to be used directly as key in DB. - if (!$validating) { + + + if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { + // Use cached outcome if we have one. + $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + + $qid = $this->id; + $seed = $this->mtrandseed; + // With same qid and prototype id the question text and seed, test cases can't have changed. + $qdetails = "qid=" . $qid . ",prototype_id=" . $protoid; + $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; + $qdetails .= ",language=" . $language; + // Also note that prechecksetting can only change if qid or prototype id has changed. + + // Most student answers won't have attachments but if they do then ... + // ... include the hash of the attachments (which will be slow). + if ($attachments) { // was xxh64 + $qdetails .= ",attachmentshash=" . hash("md5", serialize($attachments)); + } + $rawkey = $qdetails . ",code=" . $code; + $key = hash("md5", $rawkey); // Fast with digest small enough to be used directly as key in DB. + // Can't use cache for validate on save as it uses the old question id ... + // ... rather than the new one. And caching would be pointless anyway! + $testoutcomeserial = $cache->get($key); if ($testoutcomeserial) { $testoutcome = unserialize($testoutcomeserial); } } + if ($testoutcomeserial == null) { + $this->stepinfo = self::step_info($response); + $this->stepinfo->graderstate = $response['graderstate'] ?? ""; + // if (isset($response['graderstate'])) { + // $this->stepinfo->graderstate = $response['graderstate']; + // } else { + // $this->stepinfo->graderstate = ''; + // } + $testcases = $this->filter_testcases($isprecheck, $this->precheck); + $runner = new qtype_coderunner_jobrunner(); $testoutcome = $runner->run_tests( $this, $code, $attachments, $testcases, $isprecheck, - $language, - $validating + $language ); $testoutcomeserial = serialize($testoutcome); - if (!$validating) { + // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); + if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { $cache->set($key, $testoutcomeserial); } } } + // To be saved in question step data. + // Note: This is used to render test results too so it's not just a cache. $datatocache = ['_testoutcome' => $testoutcomeserial]; + if ($testoutcome->run_failed()) { return [0, question_state::$invalid, $datatocache]; } else if ($testoutcome->all_correct()) { diff --git a/settings.php b/settings.php index ad3ee03dc..60a535cbe 100644 --- a/settings.php +++ b/settings.php @@ -134,3 +134,10 @@ get_string('wsmaxcputime_desc', 'qtype_coderunner'), '5' )); + +$settings->add(new admin_setting_configcheckbox( + "qtype_coderunner/cachegradingresults", + get_string('cachegradingresultsenable', 'qtype_coderunner'), + get_string('cachegradingresults_desc', 'qtype_coderunner'), + false +)); diff --git a/tests/grader_test.php b/tests/grader_test.php index 5f7a1eed8..5097f00d0 100644 --- a/tests/grader_test.php +++ b/tests/grader_test.php @@ -63,7 +63,8 @@ public function test_regex_grader() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -94,7 +95,8 @@ public function test_nearequality_grader_right_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -125,7 +127,8 @@ public function test_nearequality_grader_wrong_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Moodle's test helper question uses an id of zero so turn off cache! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(0, $mark); diff --git a/tests/pythonquestions_test.php b/tests/pythonquestions_test.php index fb8a17d62..eca0c1162 100644 --- a/tests/pythonquestions_test.php +++ b/tests/pythonquestions_test.php @@ -211,7 +211,8 @@ public function test_timeout() { $q = $this->make_question('timeout'); $code = "def timeout():\n while (1):\n pass"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); @@ -227,7 +228,8 @@ public function test_exceptions() { $q = $this->make_question('exceptions'); $code = "def checkOdd(n):\n if n & 1:\n raise ValueError()"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); @@ -253,21 +255,24 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return 0"; // Passes first test only. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); $code = "def sqr(n):\n return n * n if n <= 0 else -17.995"; // Passes first test and last two only. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 5.0 / 7.5) < 0.00001); $code = "def sqr(n):\n return n * n if n <= 0 else 1 / 0"; // Passes first test then aborts. $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); @@ -281,11 +286,15 @@ public function test_customised_timeout() { print("Hello Python") EOT; $response = ['answer' => $slowsquare]; // Should time out. - [$mark, $grade, $cache] = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); + [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); $q->cputimelimitsecs = 20; // This should fix it. - [$mark, $grade, $cache] = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); + [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); } diff --git a/tests/template_test.php b/tests/template_test.php index d821311bb..31e1803b3 100644 --- a/tests/template_test.php +++ b/tests/template_test.php @@ -111,11 +111,13 @@ public function test_grading_template() { $q->allornothing = false; $code = "def sqr(n): return n * n\n"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertTrue(abs($mark - 24.0 / 31.0) < 0.000001); $q->allornothing = true; - $result = $q->grade_response($response); + // Don't use cache as qid of zero is reused with different question! + $result = $q->grade_response($response, usecache:false); [$mark, $grade, $cache] = $result; $this->assertTrue($mark == 0.0); } From 1af42c794e8c4d859889daa8c7204723e0232123 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 29 Jan 2024 23:30:18 +1300 Subject: [PATCH 04/15] Minor update to allow running on php < v8... --- edit_coderunner_form.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index 4f70eab8d..d4ab0a86b 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1618,13 +1618,8 @@ private function validate_sample_answer() { } [$mark, $state, $cachedata] = $this->formquestion->grade_response( $response, -<<<<<<< HEAD false, // Not a precheck. false // Don't use cache as it the old question id is used and would give old outcome. -======= - isprecheck:false, - usecache:false ->>>>>>> 2fdb0ea5fab7097a367876998ce7df42376dd84c ); } catch (Exception $e) { return $e->getMessage(); From 6de3382d9a2ca86f001aa3c337de0773643dca2c Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Tue, 30 Jan 2024 00:19:09 +1300 Subject: [PATCH 05/15] Updated some tests to make them work in php < 8 Mainly replacing calls to grade_response to grade_response($response, false, false) instead of grade_response($response, usecache:false) --- tests/grader_test.php | 18 ++++++++++++--- tests/pythonquestions_test.php | 42 ++++++++++++++++++++++++++++------ tests/template_test.php | 12 ++++++++-- 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/tests/grader_test.php b/tests/grader_test.php index 5097f00d0..91633e569 100644 --- a/tests/grader_test.php +++ b/tests/grader_test.php @@ -64,7 +64,11 @@ public function test_regex_grader() { EOCODE; $response = ['answer' => $code]; // Moodle's test helper question uses an id of zero so turn off cache! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -96,7 +100,11 @@ public function test_nearequality_grader_right_answer() { EOCODE; $response = ['answer' => $code]; // Moodle's test helper question uses an id of zero so turn off cache! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -128,7 +136,11 @@ public function test_nearequality_grader_wrong_answer() { EOCODE; $response = ['answer' => $code]; // Moodle's test helper question uses an id of zero so turn off cache! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(0, $mark); diff --git a/tests/pythonquestions_test.php b/tests/pythonquestions_test.php index eca0c1162..d8d8ef1bf 100644 --- a/tests/pythonquestions_test.php +++ b/tests/pythonquestions_test.php @@ -212,7 +212,11 @@ public function test_timeout() { $code = "def timeout():\n while (1):\n pass"; $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); @@ -229,7 +233,11 @@ public function test_exceptions() { $code = "def checkOdd(n):\n if n & 1:\n raise ValueError()"; $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); @@ -256,7 +264,11 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return 0"; // Passes first test only. $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); @@ -264,7 +276,11 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return n * n if n <= 0 else -17.995"; // Passes first test and last two only. $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 5.0 / 7.5) < 0.00001); @@ -272,7 +288,11 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return n * n if n <= 0 else 1 / 0"; // Passes first test then aborts. $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); @@ -287,13 +307,21 @@ public function test_customised_timeout() { EOT; $response = ['answer' => $slowsquare]; // Should time out. // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); $q->cputimelimitsecs = 20; // This should fix it. // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); diff --git a/tests/template_test.php b/tests/template_test.php index 31e1803b3..32489a47f 100644 --- a/tests/template_test.php +++ b/tests/template_test.php @@ -112,12 +112,20 @@ public function test_grading_template() { $code = "def sqr(n): return n * n\n"; $response = ['answer' => $code]; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertTrue(abs($mark - 24.0 / 31.0) < 0.000001); $q->allornothing = true; // Don't use cache as qid of zero is reused with different question! - $result = $q->grade_response($response, usecache:false); + $result = $q->grade_response( + $response, + false, // Not a precheck. + false // Here usecache is set to false. + ); [$mark, $grade, $cache] = $result; $this->assertTrue($mark == 0.0); } From 46f66c51fa572bf70972d1ae2e0378cf70b5f449 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Wed, 31 Jan 2024 16:43:14 +1300 Subject: [PATCH 06/15] Little bugfix... --- question.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/question.php b/question.php index 9bd53783b..5de22e806 100644 --- a/question.php +++ b/question.php @@ -32,15 +32,14 @@ use qtype_coderunner\constants; use qtype_coderunner\coderunner_files; -/** + /** * Represents a 'CodeRunner' question. */ + #[AllowDynamicProperties] class qtype_coderunner_question extends question_graded_automatically { public $testcases = null; // Array of testcases. -<<<<<<< HEAD -======= /** @var string containing the language for coderunner type. */ public $coderunnertype; @@ -220,7 +219,6 @@ class qtype_coderunner_question extends question_graded_automatically { /** @var int questionid. */ public $questionid; ->>>>>>> origin/development /** * Start a new attempt at this question, storing any information that will @@ -897,7 +895,7 @@ public function grade_response(array $response, bool $isprecheck = false, $useca } - if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { + if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { // Use cached outcome if we have one. $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); @@ -945,7 +943,7 @@ public function grade_response(array $response, bool $isprecheck = false, $useca ); $testoutcomeserial = serialize($testoutcome); // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); - if ($usecache && get_config('qtype_coderunner')->cachegradingresults) { + if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { $cache->set($key, $testoutcomeserial); } } From 589891cabea30c459e1a4455312330355dc739ee Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Sat, 10 Feb 2024 13:16:31 +1300 Subject: [PATCH 07/15] Trial with grade caching at jobesandbox.php level. --- classes/jobesandbox.php | 132 ++++++++++++++++++++++----------- classes/jobrunner.php | 14 ++++ db/caches.php | 4 +- question.php | 101 ++++++++++++------------- tests/pythonquestions_test.php | 1 - 5 files changed, 155 insertions(+), 97 deletions(-) diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index 12c5ef687..e67a97cba 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -30,6 +30,9 @@ global $CFG; require_once($CFG->libdir . '/filelib.php'); // Needed when run as web service. +const READ_FROM_CACHE = true; +const WRITE_TO_CACHE = true; + class qtype_coderunner_jobesandbox extends qtype_coderunner_sandbox { const DEBUGGING = 0; const HTTP_GET = 1; @@ -201,57 +204,98 @@ public function execute($sourcecode, $language, $input, $files = null, $params = $this->apikey = $params['jobeapikey']; } } - - $postbody = ['run_spec' => $runspec]; + // QUESTION: Do we need this when using cached result? $this->currentjobid = sprintf('%08x', mt_rand()); - // Try submitting the job. If we get a 404, try again after - // putting all the files on the server. Anything else is an error. - $httpcode = $this->submit($postbody); - if ($httpcode == 404) { // If it's a file not found error ... - foreach ($files as $filename => $contents) { - if (($httpcode = $this->put_file($contents)) != 204) { - break; - } - } - if ($httpcode == 204) { - // Try again if put_files all worked. - $httpcode = $this->submit($postbody); + $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + $runresult = null; + if (READ_FROM_CACHE) { + // NOTE: Changing jobeserver setting will effectively flush the cache + // eg, adding another jobeserver to a list of servers will mean the + // jobeserver parameter has changed and therefore the key will change. + + // QUESTION: Do we want the cache to ignore the jobeserver setting? + // eg, adding a new, presumeably equal jobeserver to the mix shouldn't + // change the result (unless it isn't equal!) + // But, remember that the server is chosen at random from the pool! + + // NOTE: Changing model answer will mean key changes. + // This shouldn't matter if we are not using the answer in tests. + // QUESTION: Is there a way to see if useanswerfortests is set? + // Although this is really at the prototype level. + // Could add this as an option in the question type itself? + // This would make it consistent across languages, etc. + // If people set it and don't use the answer for tests then + // it will just mean that changing the answer doesn't affect + // the cache which is ok. But if they don't set it and do + // use the answer for tests anyway they might get into trouble + // when changing the answer doesn't prevent the cached result + // being used! + $key = hash("md5", serialize($runspec)); + // echo '
' . serialize($runspec) . '
'; + $runresult = $cache->get($key); // unersializes the returned value :) + if ($runresult) { + //echo $key . '-----------> FOUND' . '
'; } } - $runresult = []; - $runresult['sandboxinfo'] = [ - 'jobeserver' => $this->jobeserver, - 'jobeapikey' => $this->apikey, - ]; + if ($runresult == null) { + $postbody = ['run_spec' => $runspec]; + // Try submitting the job. If we get a 404, try again after + // putting all the files on the server. Anything else is an error. + $httpcode = $this->submit($postbody); + if ($httpcode == 404) { // If it's a file not found error ... + foreach ($files as $filename => $contents) { + if (($httpcode = $this->put_file($contents)) != 204) { + break; + } + } + if ($httpcode == 204) { + // Try again if put_files all worked. + $httpcode = $this->submit($postbody); + } + } - $okresponse = in_array($httpcode, [200, 203]); // Allow 203, which can result from an intevening proxy server. - if ( - !$okresponse // If it's not an OK response... - || !is_object($this->response) // ... or there's any sort of broken ... - || !isset($this->response->outcome) - ) { // ... communication with server. - // Return with errorcode set and as much extra info as possible in stderr. - $errorcode = $okresponse ? self::UNKNOWN_SERVER_ERROR : $this->get_error_code($httpcode); - $this->currentjobid = null; - $runresult['error'] = $errorcode; - $runresult['stderr'] = "HTTP response from Jobe was $httpcode: " . json_encode($this->response); - } else if ($this->response->outcome == self::RESULT_SERVER_OVERLOAD) { - $runresult['error'] = self::SERVER_OVERLOAD; - } else { - $stderr = $this->filter_file_path($this->response->stderr); - // Any stderr output is treated as a runtime error. - if (trim($stderr ?? '') !== '') { - $this->response->outcome = self::RESULT_RUNTIME_ERROR; + $runresult = []; + $runresult['sandboxinfo'] = [ + 'jobeserver' => $this->jobeserver, + 'jobeapikey' => $this->apikey, + ]; + + $okresponse = in_array($httpcode, [200, 203]); // Allow 203, which can result from an intevening proxy server. + if ( + !$okresponse // If it's not an OK response... + || !is_object($this->response) // ... or there's any sort of broken ... + || !isset($this->response->outcome) + ) { // ... communication with server. + // Return with errorcode set and as much extra info as possible in stderr. + $errorcode = $okresponse ? self::UNKNOWN_SERVER_ERROR : $this->get_error_code($httpcode); + $this->currentjobid = null; + $runresult['error'] = $errorcode; + $runresult['stderr'] = "HTTP response from Jobe was $httpcode: " . json_encode($this->response); + } else if ($this->response->outcome == self::RESULT_SERVER_OVERLOAD) { + $runresult['error'] = self::SERVER_OVERLOAD; + } else { + $stderr = $this->filter_file_path($this->response->stderr); + // Any stderr output is treated as a runtime error. + if (trim($stderr ?? '') !== '') { + $this->response->outcome = self::RESULT_RUNTIME_ERROR; + } + $this->currentjobid = null; + $runresult['error'] = self::OK; + $runresult['stderr'] = $stderr; + $runresult['result'] = $this->response->outcome; + $runresult['signal'] = 0; // Jobe doesn't return signals. + $runresult['cmpinfo'] = $this->response->cmpinfo; + $runresult['output'] = $this->filter_file_path($this->response->stdout); + + // Got a useable result from Jobe server so cache it if required + if (WRITE_TO_CACHE) { + $key = hash("md5", serialize($runspec)); + $cache->set($key, $runresult); // set serializes the result, get will unserialize + // echo 'CACHE WRITE for ---> ' . $key . '
'; + } } - $this->currentjobid = null; - $runresult['error'] = self::OK; - $runresult['stderr'] = $stderr; - $runresult['result'] = $this->response->outcome; - $runresult['signal'] = 0; // Jobe doesn't return signals. - $runresult['cmpinfo'] = $this->response->cmpinfo; - $runresult['output'] = $this->filter_file_path($this->response->stdout); } return (object) $runresult; } diff --git a/classes/jobrunner.php b/classes/jobrunner.php index d2f6dcb5f..eb84f5a0d 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -165,6 +165,13 @@ public function run_tests( // a list of all the test cases and QUESTION, the original question object. // Return the testing outcome object if successful else null. private function run_combinator($isprecheck) { + // Remove id and questionid keys+values from testcases so they don't + // affect caching. For example the questionid will change each time + // the question is saved thanks to question versioning - urgh! + foreach ($this->testcases as $tc) { + unset($tc->id); + unset($tc->questionid); + } $numtests = count($this->testcases); $this->templateparams['TESTCASES'] = $this->testcases; $maxmark = $this->maximum_possible_mark(); @@ -239,6 +246,13 @@ private function run_tests_singly($isprecheck) { if ($maxmark == 0) { $maxmark = 1; // Something silly is happening. Probably running a prototype with no tests. } + // Remove id and questionid keys+values from testcases so they don't + // affect caching. For example the questionid will change each time + // the question is saved thanks to question versioning - urgh! + foreach ($this->testcases as $tc) { + unset($tc->id); + unset($tc->questionid); + } $numtests = count($this->testcases); $outcome = new qtype_coderunner_testing_outcome($maxmark, $numtests, $isprecheck); $question = $this->question; diff --git a/db/caches.php b/db/caches.php index 9df3b1502..a80d78330 100644 --- a/db/caches.php +++ b/db/caches.php @@ -30,11 +30,11 @@ $definitions = [ 'coderunner_grading_cache' => [ 'mode' => cache_store::MODE_APPLICATION, - 'maxsize' => 1000000, + 'maxsize' => 10000000, 'simplekeys' => true, 'simpledata' => true, 'canuselocalstore' => true, 'staticacceleration' => true, - 'staticaccelerationsize' => 500000, + 'staticaccelerationsize' => 1000000, ], ]; diff --git a/question.php b/question.php index 5de22e806..9dc2a6cd8 100644 --- a/question.php +++ b/question.php @@ -888,49 +888,46 @@ public function grade_response(array $response, bool $isprecheck = false, $useca // QUESTION: Should we raise an error on missing prototype here?... // ... Rather than in jobrunner?. - if (!empty($this->prototype)) { - $protoid = $this->prototype->id; - } else { - $protoid = ""; - } - - - if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { - // Use cached outcome if we have one. - $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); - - $qid = $this->id; - $seed = $this->mtrandseed; - // With same qid and prototype id the question text and seed, test cases can't have changed. - $qdetails = "qid=" . $qid . ",prototype_id=" . $protoid; - $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; - $qdetails .= ",language=" . $language; - // Also note that prechecksetting can only change if qid or prototype id has changed. - - // Most student answers won't have attachments but if they do then ... - // ... include the hash of the attachments (which will be slow). - if ($attachments) { // was xxh64 - $qdetails .= ",attachmentshash=" . hash("md5", serialize($attachments)); - } - $rawkey = $qdetails . ",code=" . $code; - $key = hash("md5", $rawkey); // Fast with digest small enough to be used directly as key in DB. - // Can't use cache for validate on save as it uses the old question id ... - // ... rather than the new one. And caching would be pointless anyway! - - $testoutcomeserial = $cache->get($key); - if ($testoutcomeserial) { - $testoutcome = unserialize($testoutcomeserial); - } - } - - if ($testoutcomeserial == null) { + // if (!empty($this->prototype)) { + // $protoid = $this->prototype->id; + // } else { + // $protoid = ""; + // } + + + // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { + // // Use cached outcome if we have one. + // $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); + + // $qid = $this->id; + // $seed = $this->mtrandseed; + // // With same qid and prototype id the question text and seed, test cases can't have changed. + // $qdetails = "qid=" . $qid . ",prototype_id=" . $protoid; + // $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; + // $qdetails .= ",language=" . $language; + // // Also note that prechecksetting can only change if qid or prototype id has changed. + + // // Most student answers won't have attachments but if they do then ... + // // ... include the hash of the attachments (which will be slow). + // if ($attachments) { // was xxh64 + // $qdetails .= ",attachmentshash=" . hash("md5", serialize($attachments)); + // } + // $rawkey = $qdetails . ",code=" . $code; + // $key = hash("md5", $rawkey); // Fast with digest small enough to be used directly as key in DB. + // // Can't use cache for validate on save as it uses the old question id ... + // // ... rather than the new one. And caching would be pointless anyway! + + // $testoutcomeserial = $cache->get($key); + // if ($testoutcomeserial) { + // // QUESTION: Could be extra safe and not use the cached value + // // if is contains/implies an invalid state? + // $testoutcome = unserialize($testoutcomeserial); + // } + // } + + if ($testoutcomeserial == null) { // We didn't find it in the cache. $this->stepinfo = self::step_info($response); $this->stepinfo->graderstate = $response['graderstate'] ?? ""; - // if (isset($response['graderstate'])) { - // $this->stepinfo->graderstate = $response['graderstate']; - // } else { - // $this->stepinfo->graderstate = ''; - // } $testcases = $this->filter_testcases($isprecheck, $this->precheck); $runner = new qtype_coderunner_jobrunner(); $testoutcome = $runner->run_tests( @@ -943,9 +940,12 @@ public function grade_response(array $response, bool $isprecheck = false, $useca ); $testoutcomeserial = serialize($testoutcome); // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); - if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { - $cache->set($key, $testoutcomeserial); - } + + // QUESTION: Should we only cache if the question state isn't invalid? + // This would prevent us polluting the cache. + // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { + // $cache->set($key, $testoutcomeserial); + // } } } @@ -1117,23 +1117,24 @@ public function twig_expand($text, $context = []) { protected function filter_testcases($isprecheckrun, $prechecksetting) { if (!$isprecheckrun) { if ($prechecksetting != constants::PRECHECK_SELECTED) { - return $this->testcases; + $relevanttestcases = $this->testcases; } else { - return $this->selected_testcases(false); + $relevanttestcases = $this->selected_testcases(false); } } else { // This is a precheck run. if ($prechecksetting == constants::PRECHECK_EMPTY) { - return [$this->empty_testcase()]; + $relevanttestcases = [$this->empty_testcase()]; } else if ($prechecksetting == constants::PRECHECK_EXAMPLES) { - return $this->example_testcases(); + $relevanttestcases = $this->example_testcases(); } else if ($prechecksetting == constants::PRECHECK_SELECTED) { - return $this->selected_testcases(true); + $relevanttestcases = $this->selected_testcases(true); } else if ($prechecksetting == constants::PRECHECK_ALL) { - return $this->testcases; + $relevanttestcases = $this->testcases; } else { throw new coding_exception('Precheck clicked but no precheck button?!'); } } + return $relevanttestcases; } diff --git a/tests/pythonquestions_test.php b/tests/pythonquestions_test.php index 30511d89c..2a7651535 100644 --- a/tests/pythonquestions_test.php +++ b/tests/pythonquestions_test.php @@ -38,7 +38,6 @@ * @coversNothing */ class pythonquestions_test extends \qtype_coderunner_testcase { - /** @var string */ private $goodcode; From 9522f6389ca298ede02e4e0969143f18dd7b93cf Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Sat, 10 Feb 2024 17:37:18 +1300 Subject: [PATCH 08/15] Small tweaks while testing jobe caching. --- classes/jobesandbox.php | 8 +++---- question.php | 50 +++++++++++++++++++++-------------------- settings.php | 4 ++++ 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index e67a97cba..a4b6546d8 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -233,13 +233,13 @@ public function execute($sourcecode, $language, $input, $files = null, $params = // being used! $key = hash("md5", serialize($runspec)); // echo '
' . serialize($runspec) . '
'; - $runresult = $cache->get($key); // unersializes the returned value :) + $runresult = $cache->get($key); // unersializes the returned value :) false if not found. if ($runresult) { - //echo $key . '-----------> FOUND' . '
'; + echo $key . '-----------> FOUND' . '
'; } } - if ($runresult == null) { + if (!$runresult) { $postbody = ['run_spec' => $runspec]; // Try submitting the job. If we get a 404, try again after // putting all the files on the server. Anything else is an error. @@ -293,7 +293,7 @@ public function execute($sourcecode, $language, $input, $files = null, $params = if (WRITE_TO_CACHE) { $key = hash("md5", serialize($runspec)); $cache->set($key, $runresult); // set serializes the result, get will unserialize - // echo 'CACHE WRITE for ---> ' . $key . '
'; + echo 'CACHE WRITE for ---> ' . $key . '
'; } } } diff --git a/question.php b/question.php index 9dc2a6cd8..75443ebaa 100644 --- a/question.php +++ b/question.php @@ -924,33 +924,35 @@ public function grade_response(array $response, bool $isprecheck = false, $useca // $testoutcome = unserialize($testoutcomeserial); // } // } - - if ($testoutcomeserial == null) { // We didn't find it in the cache. - $this->stepinfo = self::step_info($response); - $this->stepinfo->graderstate = $response['graderstate'] ?? ""; - $testcases = $this->filter_testcases($isprecheck, $this->precheck); - $runner = new qtype_coderunner_jobrunner(); - $testoutcome = $runner->run_tests( - $this, - $code, - $attachments, - $testcases, - $isprecheck, - $language - ); - $testoutcomeserial = serialize($testoutcome); - // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); - - // QUESTION: Should we only cache if the question state isn't invalid? - // This would prevent us polluting the cache. - // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { - // $cache->set($key, $testoutcomeserial); - // } + // if ($testoutcomeserial == null) { // We didn't find it in the cache. + $this->stepinfo = self::step_info($response); + $this->stepinfo->graderstate = $response['graderstate'] ?? ""; + $testcases = $this->filter_testcases($isprecheck, $this->precheck); + $runner = new qtype_coderunner_jobrunner(); + // QUESTION why are we reading the graderstate?? + if (isset($response['graderstate'])) { + $this->stepinfo->graderstate = $response['graderstate']; + } else { + $this->stepinfo->graderstate = ''; } + $testoutcome = $runner->run_tests( + $this, + $code, + $attachments, + $testcases, + $isprecheck, + $language + ); + $testoutcomeserial = serialize($testoutcome); + // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); + // QUESTION: Should we only cache if the question state isn't invalid? + // This would prevent us polluting the cache. + // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { + // $cache->set($key, $testoutcomeserial); + // } } - // To be saved in question step data. - // Note: This is used to render test results too so it's not just a cache. + // Note: This is used to render test results too so it's not just a cache. $datatocache = ['_testoutcome' => $testoutcomeserial]; if ($testoutcome->run_failed()) { diff --git a/settings.php b/settings.php index 60a535cbe..39c40f44c 100644 --- a/settings.php +++ b/settings.php @@ -135,9 +135,13 @@ '5' )); +/* +Currently left out so we can test + $settings->add(new admin_setting_configcheckbox( "qtype_coderunner/cachegradingresults", get_string('cachegradingresultsenable', 'qtype_coderunner'), get_string('cachegradingresults_desc', 'qtype_coderunner'), false )); +*/ From 16b36e310f36ae6dd27b4a4d7d34302152438cf7 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 26 Feb 2024 12:18:49 +1300 Subject: [PATCH 09/15] Tidied up code ready for trial on a real server --- classes/jobesandbox.php | 18 +----- classes/jobrunner.php | 4 +- db/caches.php | 4 +- question.php | 112 +++++++++------------------------ tests/pythonquestions_test.php | 21 +++---- 5 files changed, 43 insertions(+), 116 deletions(-) diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index a4b6546d8..131f54919 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -219,27 +219,15 @@ public function execute($sourcecode, $language, $input, $files = null, $params = // change the result (unless it isn't equal!) // But, remember that the server is chosen at random from the pool! - // NOTE: Changing model answer will mean key changes. - // This shouldn't matter if we are not using the answer in tests. - // QUESTION: Is there a way to see if useanswerfortests is set? - // Although this is really at the prototype level. - // Could add this as an option in the question type itself? - // This would make it consistent across languages, etc. - // If people set it and don't use the answer for tests then - // it will just mean that changing the answer doesn't affect - // the cache which is ok. But if they don't set it and do - // use the answer for tests anyway they might get into trouble - // when changing the answer doesn't prevent the cached result - // being used! $key = hash("md5", serialize($runspec)); // echo '
' . serialize($runspec) . '
'; $runresult = $cache->get($key); // unersializes the returned value :) false if not found. if ($runresult) { - echo $key . '-----------> FOUND' . '
'; + // echo $key . '-----------> FOUND' . '
'; } } - if (!$runresult) { + if (!$runresult) { // if cache read failed regrade to be safe $postbody = ['run_spec' => $runspec]; // Try submitting the job. If we get a 404, try again after // putting all the files on the server. Anything else is an error. @@ -293,7 +281,7 @@ public function execute($sourcecode, $language, $input, $files = null, $params = if (WRITE_TO_CACHE) { $key = hash("md5", serialize($runspec)); $cache->set($key, $runresult); // set serializes the result, get will unserialize - echo 'CACHE WRITE for ---> ' . $key . '
'; + // echo 'CACHE WRITE for ---> ' . $key . '
'; } } } diff --git a/classes/jobrunner.php b/classes/jobrunner.php index eb84f5a0d..6d9ee0903 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -169,7 +169,7 @@ private function run_combinator($isprecheck) { // affect caching. For example the questionid will change each time // the question is saved thanks to question versioning - urgh! foreach ($this->testcases as $tc) { - unset($tc->id); + // unset($tc->id); May be ok to leave in there???? unset($tc->questionid); } $numtests = count($this->testcases); @@ -250,7 +250,7 @@ private function run_tests_singly($isprecheck) { // affect caching. For example the questionid will change each time // the question is saved thanks to question versioning - urgh! foreach ($this->testcases as $tc) { - unset($tc->id); + // unset($tc->id); May be Ok to leave there? unset($tc->questionid); } $numtests = count($this->testcases); diff --git a/db/caches.php b/db/caches.php index a80d78330..7d0f403d1 100644 --- a/db/caches.php +++ b/db/caches.php @@ -30,9 +30,9 @@ $definitions = [ 'coderunner_grading_cache' => [ 'mode' => cache_store::MODE_APPLICATION, - 'maxsize' => 10000000, + 'maxsize' => 50000000, 'simplekeys' => true, - 'simpledata' => true, + 'simpledata' => false, 'canuselocalstore' => true, 'staticacceleration' => true, 'staticaccelerationsize' => 1000000, diff --git a/question.php b/question.php index 75443ebaa..dd4b727a9 100644 --- a/question.php +++ b/question.php @@ -220,6 +220,9 @@ class qtype_coderunner_question extends question_graded_automatically { /** @var int questionid. */ public $questionid; + /** @var int randomseed in case we want to see the seed for a question */ + public $randomseed; + /** * Start a new attempt at this question, storing any information that will * be needed later in the step. It is retrieved and applied by @@ -251,7 +254,7 @@ public function start_attempt(question_attempt_step $step = null, $variant = nul $step->set_qt_var('_mtrandseed', $seed); } $this->evaluate_question_for_display($seed, $step); - $this->mtrandseed = $seed; // so we can see it when checking + $this->randomseed = $seed; // so we can see it when checking } // Retrieve the saved random number seed and reconstruct the template @@ -267,7 +270,7 @@ public function apply_attempt_state(question_attempt_step $step) { $seed = mt_rand(); } $this->evaluate_question_for_display($seed, $step); - $this->mtrandseed = $seed; // so we can see it when checking + $this->randomseed = $seed; // so we can see it when checking } @@ -419,38 +422,33 @@ public function template_params_json($seed = 0, $step = null, $qtvar = '') { * valid json). */ public function evaluate_template_params($templateparams, $lang, $seed) { - $lang = strtolower($lang); // Just in case some old legacy DB entries escaped. - if (empty($templateparams)) { - $jsontemplateparams = '{}'; - } else if ( + if ( isset($this->cachedfuncparams) && $this->cachedfuncparams === ['lang' => $lang, 'seed' => $seed] ) { // Use previously cached result if possible. $jsontemplateparams = $this->cachedevaldtemplateparams; - } else if ($lang == 'none') { - $jsontemplateparams = $templateparams; - } else if ($lang == 'twig') { - try { - $jsontemplateparams = $this->twig_render_with_seed($templateparams, $seed); - } catch (\Twig\Error\Error $e) { - throw new qtype_coderunner_bad_json_exception($e->getMessage()); - } - } else if (!$this->templateparamsevalpertry && !empty($this->templateparamsevald)) { - $jsontemplateparams = $this->templateparamsevald; } else { - $jsontemplateparams = $this->evaluate_template_params_on_jobe($templateparams, $lang, $seed); + $lang = strtolower($lang); // Just in case some old legacy DB entries escaped. + if (empty($templateparams)) { + $jsontemplateparams = '{}'; + } else if ($lang == 'none') { + $jsontemplateparams = $templateparams; + } else if ($lang == 'twig') { + try { + $jsontemplateparams = $this->twig_render_with_seed($templateparams, $seed); + } catch (\Twig\Error\Error $e) { + throw new qtype_coderunner_bad_json_exception($e->getMessage()); + } + } else if (!$this->templateparamsevalpertry && !empty($this->templateparamsevald)) { + $jsontemplateparams = $this->templateparamsevald; + } else { + $jsontemplateparams = $this->evaluate_template_params_on_jobe($templateparams, $lang, $seed); + } + // Cache in this to avoid multiple evaluations during question editing and validation. + $this->cachedfuncparams = ['lang' => $lang, 'seed' => $seed]; + $this->cachedevaldtemplateparams = $jsontemplateparams; } - - - // Cache in this to avoid multiple evaluations during question editing and validation. - /* - ************** - QUESTION: Seems to always store in cache even when it was read from cache in the first place. - ************** - */ - $this->cachedfuncparams = ['lang' => $lang, 'seed' => $seed]; - $this->cachedevaldtemplateparams = $jsontemplateparams; return $jsontemplateparams; } @@ -853,18 +851,14 @@ public function display_feedback() { * state is used when a sandbox error occurs. * @throws coding_exception */ - public function grade_response(array $response, bool $isprecheck = false, $usecache = true) { + public function grade_response(array $response, bool $isprecheck = false) { if ($isprecheck && empty($this->precheck)) { throw new coding_exception("Unexpected precheck"); } - // Trying ?? $language = empty($response['language']) ? '' : $response['language'];. - $language = $response['language'] ?? ""; - // Was $language = empty($response['language']) ? '' : $response['language'];. - - // QUESTION: Could we replace this with the new cached version? + $language = empty($response['language']) ? '' : $response['language']; $gradingreqd = true; - $testoutcomeserial = null; + $testoutcomeserial = false; // Use _testoutcome if it's already in $response. // This should be even quicker than the file cache. @@ -885,46 +879,6 @@ public function grade_response(array $response, bool $isprecheck = false, $useca // filenames and values being file contents. $code = $response['answer']; $attachments = $this->get_attached_files($response); - - // QUESTION: Should we raise an error on missing prototype here?... - // ... Rather than in jobrunner?. - // if (!empty($this->prototype)) { - // $protoid = $this->prototype->id; - // } else { - // $protoid = ""; - // } - - - // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { - // // Use cached outcome if we have one. - // $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache'); - - // $qid = $this->id; - // $seed = $this->mtrandseed; - // // With same qid and prototype id the question text and seed, test cases can't have changed. - // $qdetails = "qid=" . $qid . ",prototype_id=" . $protoid; - // $qdetails .= ",mtrandomseed=" . $seed . ",isprecheck=" . $isprecheck; - // $qdetails .= ",language=" . $language; - // // Also note that prechecksetting can only change if qid or prototype id has changed. - - // // Most student answers won't have attachments but if they do then ... - // // ... include the hash of the attachments (which will be slow). - // if ($attachments) { // was xxh64 - // $qdetails .= ",attachmentshash=" . hash("md5", serialize($attachments)); - // } - // $rawkey = $qdetails . ",code=" . $code; - // $key = hash("md5", $rawkey); // Fast with digest small enough to be used directly as key in DB. - // // Can't use cache for validate on save as it uses the old question id ... - // // ... rather than the new one. And caching would be pointless anyway! - - // $testoutcomeserial = $cache->get($key); - // if ($testoutcomeserial) { - // // QUESTION: Could be extra safe and not use the cached value - // // if is contains/implies an invalid state? - // $testoutcome = unserialize($testoutcomeserial); - // } - // } - // if ($testoutcomeserial == null) { // We didn't find it in the cache. $this->stepinfo = self::step_info($response); $this->stepinfo->graderstate = $response['graderstate'] ?? ""; $testcases = $this->filter_testcases($isprecheck, $this->precheck); @@ -944,15 +898,9 @@ public function grade_response(array $response, bool $isprecheck = false, $useca $language ); $testoutcomeserial = serialize($testoutcome); - // echo ("-not from cache- " . $question->id . ", " . $question->prototype->id . "
"); - // QUESTION: Should we only cache if the question state isn't invalid? - // This would prevent us polluting the cache. - // if ($usecache && get_config('qtype_coderunner', "cachegradingresults")) { - // $cache->set($key, $testoutcomeserial); - // } } // To be saved in question step data. - // Note: This is used to render test results too so it's not just a cache. + // Note: This is used to render test results too so it's not just a cache. $datatocache = ['_testoutcome' => $testoutcomeserial]; if ($testoutcome->run_failed()) { @@ -970,8 +918,6 @@ public function grade_response(array $response, bool $isprecheck = false, $useca question_state::$gradedpartial, $datatocache]; } } - - // Return a map from filename to file contents for all the attached files // in the given response. private function get_attached_files($response) { diff --git a/tests/pythonquestions_test.php b/tests/pythonquestions_test.php index 2a7651535..6667cc0cc 100644 --- a/tests/pythonquestions_test.php +++ b/tests/pythonquestions_test.php @@ -214,11 +214,10 @@ public function test_timeout() { $q = $this->make_question('timeout'); $code = "def timeout():\n while (1):\n pass"; $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); @@ -235,11 +234,10 @@ public function test_exceptions() { $q = $this->make_question('exceptions'); $code = "def checkOdd(n):\n if n & 1:\n raise ValueError()"; $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); @@ -266,11 +264,10 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return 0"; // Passes first test only. $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); @@ -278,11 +275,10 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return n * n if n <= 0 else -17.995"; // Passes first test and last two only. $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); @@ -290,11 +286,10 @@ public function test_partial_mark_question() { $code = "def sqr(n):\n return n * n if n <= 0 else 1 / 0"; // Passes first test then aborts. $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); @@ -309,21 +304,19 @@ public function test_customised_timeout() { print("Hello Python") EOT; $response = ['answer' => $slowsquare]; // Should time out. - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); $q->cputimelimitsecs = 20; // This should fix it. - // Don't use cache as qid of zero is reused with different question! + // NOTE: qid of zero is reused with different question! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); From 51fc87a9e8b3dd14106288d8762108f32a1b07f8 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Wed, 28 Feb 2024 14:46:49 +1300 Subject: [PATCH 10/15] Minor updates, removing useless bits and pieces. --- classes/jobrunner.php | 2 -- edit_coderunner_form.php | 3 +-- question.php | 2 +- tests/grader_test.php | 11 ++++------- tests/template_test.php | 10 ++++------ 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/classes/jobrunner.php b/classes/jobrunner.php index 6d9ee0903..d6ee09956 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -63,8 +63,6 @@ class qtype_coderunner_jobrunner { * this is a run triggered by the student clicking the Precheck button. * @param string $answerlanguage will be the empty string except for multilanguage questions, * when it is the language selected in the language drop-down menu. - * @param boolean $validating Set to true if this is a validate on grading. Otherwise the - * validate on save uses the qid of the last version. * @return qtype_coderunner_combinator_grader_outcome $testoutcome that contains the outcome * of the grading. */ diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index c18c55cca..ab195ddf4 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1630,8 +1630,7 @@ private function validate_sample_answer() { } [$mark, $state, $cachedata] = $this->formquestion->grade_response( $response, - false, // Not a precheck. - false // Don't use cache as it the old question id is used and would give old outcome. + false // Not a precheck. ); } catch (Exception $e) { return $e->getMessage(); diff --git a/question.php b/question.php index dd4b727a9..3e97469bb 100644 --- a/question.php +++ b/question.php @@ -32,7 +32,7 @@ use qtype_coderunner\constants; use qtype_coderunner\coderunner_files; - /** +/** * Represents a 'CodeRunner' question. */ diff --git a/tests/grader_test.php b/tests/grader_test.php index 91633e569..711859c04 100644 --- a/tests/grader_test.php +++ b/tests/grader_test.php @@ -63,11 +63,10 @@ public function test_regex_grader() { pass EOCODE; $response = ['answer' => $code]; - // Moodle's test helper question uses an id of zero so turn off cache! + // Note: Moodle's test helper question always uses an id of zero! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. @@ -99,12 +98,11 @@ public function test_nearequality_grader_right_answer() { pass EOCODE; $response = ['answer' => $code]; - // Moodle's test helper question uses an id of zero so turn off cache! + // Note: Moodle's test helper question always uses an id of zero! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. - ); + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -135,11 +133,10 @@ public function test_nearequality_grader_wrong_answer() { pass EOCODE; $response = ['answer' => $code]; - // Moodle's test helper question uses an id of zero so turn off cache! + // Note: Moodle's test helper question always uses an id of zero! $result = $q->grade_response( $response, false, // Not a precheck. - false // Here usecache is set to false. ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. diff --git a/tests/template_test.php b/tests/template_test.php index 32489a47f..8739b4ea3 100644 --- a/tests/template_test.php +++ b/tests/template_test.php @@ -111,20 +111,18 @@ public function test_grading_template() { $q->allornothing = false; $code = "def sqr(n): return n * n\n"; $response = ['answer' => $code]; - // Don't use cache as qid of zero is reused with different question! + // Note: qid of zero is reused with different question! $result = $q->grade_response( $response, - false, // Not a precheck. - false // Here usecache is set to false. + false // Not a precheck. ); [$mark, $grade, $cache] = $result; $this->assertTrue(abs($mark - 24.0 / 31.0) < 0.000001); $q->allornothing = true; - // Don't use cache as qid of zero is reused with different question! + // Note: qid of zero is reused with different question! $result = $q->grade_response( $response, - false, // Not a precheck. - false // Here usecache is set to false. + false // Not a precheck. ); [$mark, $grade, $cache] = $result; $this->assertTrue($mark == 0.0); From feb3b5d3e1a232c0a1f5b7f3deb01bd5325e9127 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Mon, 4 Mar 2024 00:04:12 +1300 Subject: [PATCH 11/15] Updated behat tests to initialise jobe sandbox properly each time May still need to change behat_coderunner.php to make it more general. That is make it possible to change the jobeserver address in different situations, eg, when running in docker locally or docker in CI tests on github. --- classes/jobesandbox.php | 102 ++++++++++-------- question.php | 2 - .../ace_scratchpad_compatibility.feature | 4 +- tests/behat/attachmentimportexport.feature | 4 +- tests/behat/attachments.feature | 3 +- tests/behat/backup_and_restore.feature | 1 + tests/behat/behat_coderunner.php | 26 ++++- .../behat/check_graph_question_types.feature | 3 +- .../check_python_template_params.feature | 3 +- tests/behat/check_stepinfo.feature | 3 +- .../behat/check_twig_student_variable.feature | 3 +- .../behat/create_python3_sqr_function.feature | 3 +- tests/behat/duplicate_prototype.feature | 3 +- tests/behat/edit.feature | 3 +- tests/behat/edit_question_precheck.feature | 3 +- tests/behat/edit_table.feature | 3 +- tests/behat/export.feature | 3 +- tests/behat/gapfiller_ui.feature | 3 +- tests/behat/grading_scenarios.feature | 3 +- tests/behat/html_ui.feature | 3 +- tests/behat/import.feature | 3 +- tests/behat/make_combinator_prototype.feature | 3 +- tests/behat/make_prototype.feature | 3 +- tests/behat/missing_prototype.feature | 3 +- tests/behat/reset_button.feature | 3 +- tests/behat/run_python3_sqr_function.feature | 3 +- ...run_python3_sqr_function_templated.feature | 3 +- tests/behat/sandbox_webservice.feature | 21 ++-- tests/behat/scratchpad_ui.feature | 4 +- tests/behat/scratchpad_ui_params.feature | 2 +- tests/behat/set_uiplugin.feature | 3 +- tests/behat/showdifferences_button.feature | 3 +- tests/behat/template_params_error.feature | 3 +- tests/behat/twigprefix.feature | 3 +- 34 files changed, 155 insertions(+), 86 deletions(-) diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index ca880fa5f..d410b41e4 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -75,12 +75,27 @@ public function __construct() { qtype_coderunner_sandbox::__construct(); // Hack to force use of a local jobe host when behat testing. - if ($CFG->prefix == "bht_") { - $this->jobeserver = "localhost"; - } else { - $this->jobeserver = get_config('qtype_coderunner', 'jobe_host'); + // if ($CFG->prefix == "bht_") { + // $this->jobeserver = "localhost"; // Should it be :4000 ? + // } else if ($CFG->prefix == "b_") { + // $this->jobeserver = "172.17.0.1:4000"; // For local moodle-docker usage. + // } else { + // $this->jobeserver = get_config('qtype_coderunner', 'jobe_host'); + // } + + // use Given the CodeRunner jobe sandbox is enabled + // or Given the CodeRunner jobe scratchpad is enabled + // in each feature to make sure the right jobe_host string is used + // adjust behat_coderunner.php to set the appropriate server + // eg, in a local docker we usually have 172.17.0.1:4000 + + $jobefromconfig = get_config('qtype_coderunner', 'jobe_host'); + $this->jobeserver = $jobefromconfig; + $keyenabled = get_config('qtype_coderunner', 'jobe_apikey_enabled'); + // $this->jobeserver = $jobefromconfig; + if ($jobefromconfig != "172.17.0.1:4000") { + $banana = 'what?'; } - $this->apikey = get_config('qtype_coderunner', 'jobe_apikey'); $this->languages = null; } @@ -220,10 +235,10 @@ public function execute($sourcecode, $language, $input, $files = null, $params = // But, remember that the server is chosen at random from the pool! $key = hash("md5", serialize($runspec)); - // echo '
' . serialize($runspec) . '
'; - $runresult = $cache->get($key); // unersializes the returned value :) false if not found. + // Debugger: echo '
' . serialize($runspec) . '
';. + $runresult = $cache->get($key); // Unserializes the returned value :) false if not found. if ($runresult) { - // echo $key . '-----------> FOUND' . '
'; + // echo $key . '-----------> FOUND' . '
'; . } } @@ -277,10 +292,10 @@ public function execute($sourcecode, $language, $input, $files = null, $params = $runresult['cmpinfo'] = $this->response->cmpinfo; $runresult['output'] = $this->filter_file_path($this->response->stdout); - // Got a useable result from Jobe server so cache it if required + // Got a useable result from Jobe server so cache it if required. if (WRITE_TO_CACHE) { $key = hash("md5", serialize($runspec)); - $cache->set($key, $runresult); // set serializes the result, get will unserialize + $cache->set($key, $runresult); // set serializes the result, get will unserialize. // echo 'CACHE WRITE for ---> ' . $key . '
'; } } @@ -292,58 +307,61 @@ public function execute($sourcecode, $language, $input, $files = null, $params = // such class found. Removes comments, strings and nested code and then // uses a regexp to find a public class. private function get_main_class($prog) { - // filter out comments and strings + // Filter out comments and strings. $prog = $prog . ' '; - $filteredProg = array(); - $skipTo = -1; + $filteredprog = []; + $skipto = -1; for ($i = 0; $i < strlen($prog) - 1; $i++) { - if ($skipTo == false) break; // an unclosed comment/string - bail out - if ($i < $skipTo) continue; - - // skip "//" comments - if ($prog[$i].$prog[$i+1] == '//') { - $skipTo = strpos($prog, "\n", $i + 2); + if ($skipto == false) { + break; // An unclosed comment/string - bail out. } - - // skip "/**/" comments - else if ($prog[$i].$prog[$i+1] == '/*') { - $skipTo = strpos($prog, '*/', $i + 2) + 2; - $filteredProg[] = ' '; // '/**/' is a token delimiter + if ($i < $skipto) { + continue; } - - // skip strings - else if ($prog[$i] == '"') { - // matches the whole string + // Skip "//" comments. + if ($prog[$i] . $prog[$i + 1] == '//') { + $skipto = strpos($prog, "\n", $i + 2); + // Skip "/**/" comments. + } else if ($prog[$i] . $prog[$i + 1] == '/*') { + $skipto = strpos($prog, '*/', $i + 2) + 2; + $filteredprog[] = ' '; // The string '/**/' is a token delimiter. + // Skip strings. + } else if ($prog[$i] == '"') { + // Matches the whole string. if (preg_match('/"((\\.)|[^\\"])*"/', $prog, $matches, 0, $i)) { - $skipTo = $i + strlen($matches[0]); + $skipto = $i + strlen($matches[0]); + } else { + $skipto = false; } - else $skipTo = false; + // Copy everything else. + } else { + $filteredprog[] = $prog[$i]; } - - // copy everything else - else $filteredProg[] = $prog[$i]; } - // remove nested code + // Remove nested code. $depth = 0; - for ($i = 0; $i < count($filteredProg); $i++) { - if ($filteredProg[$i] == '{') $depth++; - if ($filteredProg[$i] == '}') $depth--; - if ($filteredProg[$i] != "\n" && $depth > 0 && !($depth == 1 && $filteredProg[$i] == '{')) { - $filteredProg[$i] = ' '; + for ($i = 0; $i < count($filteredprog); $i++) { + if ($filteredprog[$i] == '{') { + $depth++; + } + if ($filteredprog[$i] == '}') { + $depth--; + } + if ($filteredprog[$i] != "\n" && $depth > 0 && !($depth == 1 && $filteredprog[$i] == '{')) { + $filteredprog[$i] = ' '; } } - // search for a public class - if (preg_match('/public\s(\w*\s)*class\s*(\w+)[^\w]/', implode('', $filteredProg), $matches) !== 1) { + // Search for a public class. + if (preg_match('/public\s(\w*\s)*class\s*(\w+)[^\w]/', implode('', $filteredprog), $matches) !== 1) { return false; } else { return $matches[2]; } } - // Return the sandbox error code corresponding to the given httpcode. private function get_error_code($httpcode) { diff --git a/question.php b/question.php index 3e97469bb..623c6d23a 100644 --- a/question.php +++ b/question.php @@ -843,8 +843,6 @@ public function display_feedback() { * the history of prior submissions. * @param bool $isprecheck true iff this grading is occurring because the * student clicked the precheck button - * @param bool $usecache If true (and the coderunner cachegradingresults setting is also true) then - * cache results in coderunner cache and use results from the coderunner grading cache. * @return 3-element array of the mark (0 - 1), the question_state ( * gradedright, gradedwrong, gradedpartial, invalid) and the full * qtype_coderunner_testing_outcome object to be cached. The invalid diff --git a/tests/behat/ace_scratchpad_compatibility.feature b/tests/behat/ace_scratchpad_compatibility.feature index d29805534..8e4075376 100644 --- a/tests/behat/ace_scratchpad_compatibility.feature +++ b/tests/behat/ace_scratchpad_compatibility.feature @@ -5,7 +5,8 @@ Feature: Ace UI convert to Scratchpad UI questions with one click I should be able to change a question from using Ace to Scratchpad in one click Background: - Given the following "users" exist: + Given the CodeRunner scratchpad is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: @@ -20,7 +21,6 @@ Feature: Ace UI convert to Scratchpad UI questions with one click And the following "questions" exist: | questioncategory | qtype | name | | Test questions | coderunner | Square function | - And the CodeRunner sandbox is enabled When I am on the "Square function" "core_question > edit" page logged in as teacher1 And I set the following fields to these values: diff --git a/tests/behat/attachmentimportexport.feature b/tests/behat/attachmentimportexport.feature index 10e88dcec..1b96a9f92 100644 --- a/tests/behat/attachmentimportexport.feature +++ b/tests/behat/attachmentimportexport.feature @@ -5,7 +5,8 @@ Feature: Test importing and exporting of question with attachments I need to be able to import and export them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | | teacher | And the following "courses" exist: @@ -20,7 +21,6 @@ Feature: Test importing and exporting of question with attachments And the following "questions" exist: | questioncategory | qtype | name | | Test questions | coderunner | Square function | - And the CodeRunner sandbox is enabled And I am on the "Square function" "core_question > edit" page logged in as teacher And I click on "a[aria-controls='id_attachmentoptionscontainer']" "css_element" And I set the field "Answer" to "from sqrmodule import sqr" diff --git a/tests/behat/attachments.feature b/tests/behat/attachments.feature index 2403b336d..3a25de6cf 100644 --- a/tests/behat/attachments.feature +++ b/tests/behat/attachments.feature @@ -5,7 +5,8 @@ Feature: Test editing and using attachments to a CodeRunner question I need to enable and configure them, then preview them. Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/backup_and_restore.feature b/tests/behat/backup_and_restore.feature index 870658339..6b183b60d 100644 --- a/tests/behat/backup_and_restore.feature +++ b/tests/behat/backup_and_restore.feature @@ -5,6 +5,7 @@ Feature: Duplicate a course containing a CodeRunner question I need to be able to back them up and restore them Background: + Given the CodeRunner jobe sandbox is enabled And the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | diff --git a/tests/behat/behat_coderunner.php b/tests/behat/behat_coderunner.php index 2ecaa8c20..4cb756964 100644 --- a/tests/behat/behat_coderunner.php +++ b/tests/behat/behat_coderunner.php @@ -30,14 +30,38 @@ class behat_coderunner extends behat_base { /** * Sets the webserver sandbox to enabled for testing purposes. * - * @Given /^the CodeRunner sandbox is enabled/ + * @Given /^the CodeRunner jobe sandbox is enabled/ */ public function the_coderunner_sandbox_is_enabled() { + set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); + set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); + } + + + /** + * Sets the webserver scratchpad to enabled for testing purposes. + * + * @Given /^the CodeRunner scratchpad is enabled/ + */ + public function the_coderunner_scratchpad_is_enabled() { set_config('wsenabled', 1, 'qtype_coderunner'); set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); } + + /** + * Sets the webserver scratchpad to disabled for testing purposes. + * + * @Given /^the CodeRunner scratchpad is disabled/ + */ + public function the_coderunner_scratchpad_is_disabled() { + set_config('wsenabled', 0, 'qtype_coderunner'); + set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); + set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); + } + + /** * Checks that a given string appears within answer textarea. * Intended for checking UI serialization diff --git a/tests/behat/check_graph_question_types.feature b/tests/behat/check_graph_question_types.feature index 8b65a60a4..e20065e5d 100644 --- a/tests/behat/check_graph_question_types.feature +++ b/tests/behat/check_graph_question_types.feature @@ -5,7 +5,8 @@ Feature: Check that the directed and undirected graph question types work. I should be able to write simple graph questions and have them work correctly Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/check_python_template_params.feature b/tests/behat/check_python_template_params.feature index 14955b0cd..e52ed874b 100644 --- a/tests/behat/check_python_template_params.feature +++ b/tests/behat/check_python_template_params.feature @@ -5,7 +5,8 @@ Feature: Check that Python and other languages can be used instead of Twig as a I should be able to write a function that prints the seed and my username it should be marked right Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | Last | teacher1@asd.com | | student1 | Student First | O'Connell | student@asd.com | diff --git a/tests/behat/check_stepinfo.feature b/tests/behat/check_stepinfo.feature index 69ebbc82d..125712c39 100644 --- a/tests/behat/check_stepinfo.feature +++ b/tests/behat/check_stepinfo.feature @@ -5,7 +5,8 @@ Feature: Check that the QUESTION.stepinfo record is working. I should be able to write a question that gives different feedback for different submissions. Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | | student1 | Student | 1 | student@asd.com | diff --git a/tests/behat/check_twig_student_variable.feature b/tests/behat/check_twig_student_variable.feature index 7d011155b..74c5b974d 100644 --- a/tests/behat/check_twig_student_variable.feature +++ b/tests/behat/check_twig_student_variable.feature @@ -5,7 +5,8 @@ Feature: Check the STUDENT Twig variable allows access to current username in Co I should be able to write a function that prints my username it should be marked right Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | | student1 | Student | 1 | student@asd.com | diff --git a/tests/behat/create_python3_sqr_function.feature b/tests/behat/create_python3_sqr_function.feature index e459fc9e9..4a938710a 100644 --- a/tests/behat/create_python3_sqr_function.feature +++ b/tests/behat/create_python3_sqr_function.feature @@ -5,7 +5,8 @@ Feature: Create a CodeRunner question (the sqr function example) I need to create a new CodeRunner question Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/duplicate_prototype.feature b/tests/behat/duplicate_prototype.feature index 7ff68ced9..fa32eedc6 100644 --- a/tests/behat/duplicate_prototype.feature +++ b/tests/behat/duplicate_prototype.feature @@ -5,7 +5,8 @@ Feature: duplicate_prototypes I should see an informative error message and be able to fix by editing the duplicates Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit.feature b/tests/behat/edit.feature index db79a1747..41a34a7df 100644 --- a/tests/behat/edit.feature +++ b/tests/behat/edit.feature @@ -5,7 +5,8 @@ Feature: Test editing a CodeRunner question I need to edit them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit_question_precheck.feature b/tests/behat/edit_question_precheck.feature index 29ee74e74..9ae276c55 100644 --- a/tests/behat/edit_question_precheck.feature +++ b/tests/behat/edit_question_precheck.feature @@ -5,7 +5,8 @@ Feature: edit_question_precheck I should get informative error messages if saving was unsuccessful Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit_table.feature b/tests/behat/edit_table.feature index 51371477e..153150e6a 100644 --- a/tests/behat/edit_table.feature +++ b/tests/behat/edit_table.feature @@ -5,7 +5,8 @@ Feature: Test editing a CodeRunner question using the Table UI I should be able to set the table headers and see the table in the edit form. Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/export.feature b/tests/behat/export.feature index 2180f4853..92c7a0faf 100644 --- a/tests/behat/export.feature +++ b/tests/behat/export.feature @@ -5,7 +5,8 @@ Feature: Export CodeRunner questions I need to export them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | | teacher | And the following "courses" exist: diff --git a/tests/behat/gapfiller_ui.feature b/tests/behat/gapfiller_ui.feature index 58e6c783b..537838eb5 100644 --- a/tests/behat/gapfiller_ui.feature +++ b/tests/behat/gapfiller_ui.feature @@ -5,7 +5,8 @@ Feature: Test the GapFiller_UI I should be able specify the required gaps in the global extra or test0 fields Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/grading_scenarios.feature b/tests/behat/grading_scenarios.feature index cb7602a3c..183f6209f 100644 --- a/tests/behat/grading_scenarios.feature +++ b/tests/behat/grading_scenarios.feature @@ -5,7 +5,8 @@ Feature: Check grading with the Python 3 sqr function CodeRunner question I must be able to preview them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/html_ui.feature b/tests/behat/html_ui.feature index b235bbcdf..3af90167b 100644 --- a/tests/behat/html_ui.feature +++ b/tests/behat/html_ui.feature @@ -5,7 +5,8 @@ Feature: Test the HTML_UI I should be able specify the required html in either globalextra or prototypeextra Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/import.feature b/tests/behat/import.feature index 01062935c..f760a2e09 100644 --- a/tests/behat/import.feature +++ b/tests/behat/import.feature @@ -5,7 +5,8 @@ Feature: Import CodeRunner questions I need to import them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | | teacher | And the following "courses" exist: diff --git a/tests/behat/make_combinator_prototype.feature b/tests/behat/make_combinator_prototype.feature index bfbc2d6be..da68fe775 100644 --- a/tests/behat/make_combinator_prototype.feature +++ b/tests/behat/make_combinator_prototype.feature @@ -5,7 +5,8 @@ Feature: make_combinator_prototype I must be able to create new combinator templates Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/make_prototype.feature b/tests/behat/make_prototype.feature index b1b1617f9..386bef982 100644 --- a/tests/behat/make_prototype.feature +++ b/tests/behat/make_prototype.feature @@ -5,7 +5,8 @@ Feature: make_prototype I must be able to create new question templates Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/missing_prototype.feature b/tests/behat/missing_prototype.feature index 9eb2687bf..d9d109c90 100644 --- a/tests/behat/missing_prototype.feature +++ b/tests/behat/missing_prototype.feature @@ -5,7 +5,8 @@ Feature: missing_prototype I should see an informative error message and be able to fix by editing Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/reset_button.feature b/tests/behat/reset_button.feature index fc215c1d4..c8da49a3b 100644 --- a/tests/behat/reset_button.feature +++ b/tests/behat/reset_button.feature @@ -5,7 +5,8 @@ Feature: Preview the Python 3 sqr function CodeRunner question with a preload I should see a Reset answer button that resets the preload, Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/run_python3_sqr_function.feature b/tests/behat/run_python3_sqr_function.feature index 47c424af4..e2c45fff7 100644 --- a/tests/behat/run_python3_sqr_function.feature +++ b/tests/behat/run_python3_sqr_function.feature @@ -5,7 +5,8 @@ Feature: Preview the Python 3 sqr function CodeRunner question I must be able to preview them Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/run_python3_sqr_function_templated.feature b/tests/behat/run_python3_sqr_function_templated.feature index b40922694..82025d9c7 100644 --- a/tests/behat/run_python3_sqr_function_templated.feature +++ b/tests/behat/run_python3_sqr_function_templated.feature @@ -5,7 +5,8 @@ Feature: Combinator template is called test-by-test if a runtime error occurs wh I need the combinator template to be used test-by-test if my answer gives a runtime error Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/sandbox_webservice.feature b/tests/behat/sandbox_webservice.feature index f013dd353..aa072a8c4 100644 --- a/tests/behat/sandbox_webservice.feature +++ b/tests/behat/sandbox_webservice.feature @@ -4,7 +4,7 @@ Feature: Test sandbox web service server (Jobe) via Ajax. Background: - Given the following "users" exist: + And the following "users" exist: | username | firstname | lastname | email | | teacher | Teacher | 1 | teacher@asd.com | | student | Student | 1 | student@asd.com | @@ -30,19 +30,22 @@ Feature: Test sandbox web service @javascript Scenario: As a student if I try to initiate a WS request I get an error if the service is disabled. - When I am on the "Quiz 1" "mod_quiz > View" page logged in as student + Given the CodeRunner scratchpad is disabled + And I am on the "Quiz 1" "mod_quiz > View" page logged in as student And I press "Attempt quiz" And I press "Click me" - Then I should see "ERROR: qtype_coderunner/Sandbox web service disabled." + #Then I should see "ERROR: qtype_coderunner/Sandbox web service disabled." + Then I should see "ERROR: qtype_coderunner/Sandbox web service disabled. Talk to a sysadmin" @javascript Scenario: As a student I can initiate a WS request and see the outcome if the service is enabled. - When I log in as "admin" - And I navigate to "Plugins > CodeRunner" in site administration - And I set the following fields to these values: - | Enable sandbox web service | Yes | - And I press "Save changes" - And I log out +# When I log in as "admin" +# And I navigate to "Plugins > CodeRunner" in site administration +# And I set the following fields to these values: +# | Enable sandbox web service | Yes | +# And I press "Save changes" +# And I log out + Given the CodeRunner scratchpad is enabled When I am on the "Quiz 1" "mod_quiz > View" page logged in as student And I press "Attempt quiz" And I press "Click me" diff --git a/tests/behat/scratchpad_ui.feature b/tests/behat/scratchpad_ui.feature index 9397521a5..54e55701c 100644 --- a/tests/behat/scratchpad_ui.feature +++ b/tests/behat/scratchpad_ui.feature @@ -5,7 +5,8 @@ Feature: Test the Scratchpad UI I should be able specify the required html in either globalextra or prototypeextra Background: - Given the following "users" exist: + Given the CodeRunner scratchpad is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: @@ -20,7 +21,6 @@ Feature: Test the Scratchpad UI And the following "questions" exist: | questioncategory | qtype | name | template | | Test questions | coderunner | Print answer | printans | - And the CodeRunner sandbox is enabled Scenario: Edit a CodeRunner question into a Scratchpad UI question When I am on the "Print answer" "core_question > edit" page logged in as teacher1 diff --git a/tests/behat/scratchpad_ui_params.feature b/tests/behat/scratchpad_ui_params.feature index 2dd3d8111..57332da91 100644 --- a/tests/behat/scratchpad_ui_params.feature +++ b/tests/behat/scratchpad_ui_params.feature @@ -20,7 +20,7 @@ Feature: Test the Scratchpad UI, UI Params And the following "questions" exist: | questioncategory | qtype | name | template | | Test questions | coderunner | Print answer | printans | - And the CodeRunner sandbox is enabled + And the CodeRunner scratchpad is enabled And I am on the "Print answer" "core_question > edit" page logged in as teacher1 And I set the field "id_validateonsave" to "" diff --git a/tests/behat/set_uiplugin.feature b/tests/behat/set_uiplugin.feature index 53f60b1d0..a39d11b3e 100644 --- a/tests/behat/set_uiplugin.feature +++ b/tests/behat/set_uiplugin.feature @@ -5,7 +5,8 @@ Feature: Check that a selected UI plugin is saved I should be able to select a UI plugin and save the form Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/showdifferences_button.feature b/tests/behat/showdifferences_button.feature index c244167eb..c63b496c7 100644 --- a/tests/behat/showdifferences_button.feature +++ b/tests/behat/showdifferences_button.feature @@ -5,7 +5,8 @@ Feature: Show differences in CodeRunner questions clicked. Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/template_params_error.feature b/tests/behat/template_params_error.feature index bce676548..543c6d942 100644 --- a/tests/behat/template_params_error.feature +++ b/tests/behat/template_params_error.feature @@ -5,7 +5,8 @@ Feature: template_params_error I should get informative template parameter error messages Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/twigprefix.feature b/tests/behat/twigprefix.feature index 86eb75637..4df509257 100644 --- a/tests/behat/twigprefix.feature +++ b/tests/behat/twigprefix.feature @@ -5,7 +5,8 @@ Feature: twigprefix I must be able to use the Twig prefix data in a question. Background: - Given the following "users" exist: + Given the CodeRunner jobe sandbox is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: From 633926569264326e014cc777fbea4f63f180b6eb Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Tue, 5 Mar 2024 16:25:20 +1300 Subject: [PATCH 12/15] Reverted behat fixtures to roughly what they were with minor improvements. Removed unneccesary Given jobe sandbox enabled lines. Removed the hack in jobesandbox.php in prep for merging with branch that has fixed the problem of reverting back to coderunner's default jobe2 server (jobe2.cosc.canterbury.ac.nz) during bhat tests. --- classes/jobesandbox.php | 24 +------------------ tests/behat/attachmentimportexport.feature | 3 +-- tests/behat/attachments.feature | 3 +-- tests/behat/backup_and_restore.feature | 3 +-- tests/behat/behat_coderunner.php | 5 ---- .../behat/check_graph_question_types.feature | 3 +-- .../check_python_template_params.feature | 3 +-- tests/behat/check_stepinfo.feature | 3 +-- .../behat/check_twig_student_variable.feature | 3 +-- .../behat/create_python3_sqr_function.feature | 3 +-- tests/behat/duplicate_prototype.feature | 3 +-- tests/behat/edit.feature | 3 +-- tests/behat/edit_question_precheck.feature | 3 +-- tests/behat/edit_table.feature | 3 +-- tests/behat/export.feature | 3 +-- tests/behat/gapfiller_ui.feature | 3 +-- tests/behat/grading_scenarios.feature | 3 +-- tests/behat/html_ui.feature | 3 +-- tests/behat/import.feature | 3 +-- tests/behat/make_combinator_prototype.feature | 3 +-- tests/behat/make_prototype.feature | 3 +-- tests/behat/missing_prototype.feature | 3 +-- tests/behat/reset_button.feature | 3 +-- tests/behat/run_python3_sqr_function.feature | 3 +-- ...run_python3_sqr_function_templated.feature | 3 +-- tests/behat/sandbox_webservice.feature | 2 +- tests/behat/scratchpad_ui_params.feature | 4 ++-- tests/behat/set_uiplugin.feature | 3 +-- tests/behat/showdifferences_button.feature | 3 +-- tests/behat/template_params_error.feature | 3 +-- tests/behat/twigprefix.feature | 3 +-- 31 files changed, 31 insertions(+), 85 deletions(-) diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index d410b41e4..1eba4ac09 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -73,29 +73,7 @@ class qtype_coderunner_jobesandbox extends qtype_coderunner_sandbox { public function __construct() { global $CFG; qtype_coderunner_sandbox::__construct(); - - // Hack to force use of a local jobe host when behat testing. - // if ($CFG->prefix == "bht_") { - // $this->jobeserver = "localhost"; // Should it be :4000 ? - // } else if ($CFG->prefix == "b_") { - // $this->jobeserver = "172.17.0.1:4000"; // For local moodle-docker usage. - // } else { - // $this->jobeserver = get_config('qtype_coderunner', 'jobe_host'); - // } - - // use Given the CodeRunner jobe sandbox is enabled - // or Given the CodeRunner jobe scratchpad is enabled - // in each feature to make sure the right jobe_host string is used - // adjust behat_coderunner.php to set the appropriate server - // eg, in a local docker we usually have 172.17.0.1:4000 - - $jobefromconfig = get_config('qtype_coderunner', 'jobe_host'); - $this->jobeserver = $jobefromconfig; - $keyenabled = get_config('qtype_coderunner', 'jobe_apikey_enabled'); - // $this->jobeserver = $jobefromconfig; - if ($jobefromconfig != "172.17.0.1:4000") { - $banana = 'what?'; - } + $this->jobeserver = get_config('qtype_coderunner', 'jobe_host'); $this->apikey = get_config('qtype_coderunner', 'jobe_apikey'); $this->languages = null; } diff --git a/tests/behat/attachmentimportexport.feature b/tests/behat/attachmentimportexport.feature index 1b96a9f92..1150d4c56 100644 --- a/tests/behat/attachmentimportexport.feature +++ b/tests/behat/attachmentimportexport.feature @@ -5,8 +5,7 @@ Feature: Test importing and exporting of question with attachments I need to be able to import and export them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | | teacher | And the following "courses" exist: diff --git a/tests/behat/attachments.feature b/tests/behat/attachments.feature index 3a25de6cf..2403b336d 100644 --- a/tests/behat/attachments.feature +++ b/tests/behat/attachments.feature @@ -5,8 +5,7 @@ Feature: Test editing and using attachments to a CodeRunner question I need to enable and configure them, then preview them. Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/backup_and_restore.feature b/tests/behat/backup_and_restore.feature index 6b183b60d..8d3d73552 100644 --- a/tests/behat/backup_and_restore.feature +++ b/tests/behat/backup_and_restore.feature @@ -5,8 +5,7 @@ Feature: Duplicate a course containing a CodeRunner question I need to be able to back them up and restore them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "courses" exist: + Given the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | And the following "question categories" exist: diff --git a/tests/behat/behat_coderunner.php b/tests/behat/behat_coderunner.php index 4cb756964..55d713d25 100644 --- a/tests/behat/behat_coderunner.php +++ b/tests/behat/behat_coderunner.php @@ -34,7 +34,6 @@ class behat_coderunner extends behat_base { */ public function the_coderunner_sandbox_is_enabled() { set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); - set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); } @@ -45,8 +44,6 @@ public function the_coderunner_sandbox_is_enabled() { */ public function the_coderunner_scratchpad_is_enabled() { set_config('wsenabled', 1, 'qtype_coderunner'); - set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); - set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); } @@ -57,8 +54,6 @@ public function the_coderunner_scratchpad_is_enabled() { */ public function the_coderunner_scratchpad_is_disabled() { set_config('wsenabled', 0, 'qtype_coderunner'); - set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); - set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); } diff --git a/tests/behat/check_graph_question_types.feature b/tests/behat/check_graph_question_types.feature index e20065e5d..8b65a60a4 100644 --- a/tests/behat/check_graph_question_types.feature +++ b/tests/behat/check_graph_question_types.feature @@ -5,8 +5,7 @@ Feature: Check that the directed and undirected graph question types work. I should be able to write simple graph questions and have them work correctly Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/check_python_template_params.feature b/tests/behat/check_python_template_params.feature index e52ed874b..14955b0cd 100644 --- a/tests/behat/check_python_template_params.feature +++ b/tests/behat/check_python_template_params.feature @@ -5,8 +5,7 @@ Feature: Check that Python and other languages can be used instead of Twig as a I should be able to write a function that prints the seed and my username it should be marked right Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | Last | teacher1@asd.com | | student1 | Student First | O'Connell | student@asd.com | diff --git a/tests/behat/check_stepinfo.feature b/tests/behat/check_stepinfo.feature index 125712c39..69ebbc82d 100644 --- a/tests/behat/check_stepinfo.feature +++ b/tests/behat/check_stepinfo.feature @@ -5,8 +5,7 @@ Feature: Check that the QUESTION.stepinfo record is working. I should be able to write a question that gives different feedback for different submissions. Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | | student1 | Student | 1 | student@asd.com | diff --git a/tests/behat/check_twig_student_variable.feature b/tests/behat/check_twig_student_variable.feature index 74c5b974d..7d011155b 100644 --- a/tests/behat/check_twig_student_variable.feature +++ b/tests/behat/check_twig_student_variable.feature @@ -5,8 +5,7 @@ Feature: Check the STUDENT Twig variable allows access to current username in Co I should be able to write a function that prints my username it should be marked right Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | | student1 | Student | 1 | student@asd.com | diff --git a/tests/behat/create_python3_sqr_function.feature b/tests/behat/create_python3_sqr_function.feature index 4a938710a..e459fc9e9 100644 --- a/tests/behat/create_python3_sqr_function.feature +++ b/tests/behat/create_python3_sqr_function.feature @@ -5,8 +5,7 @@ Feature: Create a CodeRunner question (the sqr function example) I need to create a new CodeRunner question Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/duplicate_prototype.feature b/tests/behat/duplicate_prototype.feature index fa32eedc6..7ff68ced9 100644 --- a/tests/behat/duplicate_prototype.feature +++ b/tests/behat/duplicate_prototype.feature @@ -5,8 +5,7 @@ Feature: duplicate_prototypes I should see an informative error message and be able to fix by editing the duplicates Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit.feature b/tests/behat/edit.feature index 41a34a7df..db79a1747 100644 --- a/tests/behat/edit.feature +++ b/tests/behat/edit.feature @@ -5,8 +5,7 @@ Feature: Test editing a CodeRunner question I need to edit them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit_question_precheck.feature b/tests/behat/edit_question_precheck.feature index 9ae276c55..29ee74e74 100644 --- a/tests/behat/edit_question_precheck.feature +++ b/tests/behat/edit_question_precheck.feature @@ -5,8 +5,7 @@ Feature: edit_question_precheck I should get informative error messages if saving was unsuccessful Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/edit_table.feature b/tests/behat/edit_table.feature index 153150e6a..51371477e 100644 --- a/tests/behat/edit_table.feature +++ b/tests/behat/edit_table.feature @@ -5,8 +5,7 @@ Feature: Test editing a CodeRunner question using the Table UI I should be able to set the table headers and see the table in the edit form. Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/export.feature b/tests/behat/export.feature index 92c7a0faf..2180f4853 100644 --- a/tests/behat/export.feature +++ b/tests/behat/export.feature @@ -5,8 +5,7 @@ Feature: Export CodeRunner questions I need to export them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | | teacher | And the following "courses" exist: diff --git a/tests/behat/gapfiller_ui.feature b/tests/behat/gapfiller_ui.feature index 537838eb5..58e6c783b 100644 --- a/tests/behat/gapfiller_ui.feature +++ b/tests/behat/gapfiller_ui.feature @@ -5,8 +5,7 @@ Feature: Test the GapFiller_UI I should be able specify the required gaps in the global extra or test0 fields Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/grading_scenarios.feature b/tests/behat/grading_scenarios.feature index 183f6209f..cb7602a3c 100644 --- a/tests/behat/grading_scenarios.feature +++ b/tests/behat/grading_scenarios.feature @@ -5,8 +5,7 @@ Feature: Check grading with the Python 3 sqr function CodeRunner question I must be able to preview them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/html_ui.feature b/tests/behat/html_ui.feature index 3af90167b..b235bbcdf 100644 --- a/tests/behat/html_ui.feature +++ b/tests/behat/html_ui.feature @@ -5,8 +5,7 @@ Feature: Test the HTML_UI I should be able specify the required html in either globalextra or prototypeextra Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/import.feature b/tests/behat/import.feature index f760a2e09..01062935c 100644 --- a/tests/behat/import.feature +++ b/tests/behat/import.feature @@ -5,8 +5,7 @@ Feature: Import CodeRunner questions I need to import them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | | teacher | And the following "courses" exist: diff --git a/tests/behat/make_combinator_prototype.feature b/tests/behat/make_combinator_prototype.feature index da68fe775..bfbc2d6be 100644 --- a/tests/behat/make_combinator_prototype.feature +++ b/tests/behat/make_combinator_prototype.feature @@ -5,8 +5,7 @@ Feature: make_combinator_prototype I must be able to create new combinator templates Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/make_prototype.feature b/tests/behat/make_prototype.feature index 386bef982..b1b1617f9 100644 --- a/tests/behat/make_prototype.feature +++ b/tests/behat/make_prototype.feature @@ -5,8 +5,7 @@ Feature: make_prototype I must be able to create new question templates Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/missing_prototype.feature b/tests/behat/missing_prototype.feature index d9d109c90..9eb2687bf 100644 --- a/tests/behat/missing_prototype.feature +++ b/tests/behat/missing_prototype.feature @@ -5,8 +5,7 @@ Feature: missing_prototype I should see an informative error message and be able to fix by editing Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/reset_button.feature b/tests/behat/reset_button.feature index c8da49a3b..fc215c1d4 100644 --- a/tests/behat/reset_button.feature +++ b/tests/behat/reset_button.feature @@ -5,8 +5,7 @@ Feature: Preview the Python 3 sqr function CodeRunner question with a preload I should see a Reset answer button that resets the preload, Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/run_python3_sqr_function.feature b/tests/behat/run_python3_sqr_function.feature index e2c45fff7..47c424af4 100644 --- a/tests/behat/run_python3_sqr_function.feature +++ b/tests/behat/run_python3_sqr_function.feature @@ -5,8 +5,7 @@ Feature: Preview the Python 3 sqr function CodeRunner question I must be able to preview them Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/run_python3_sqr_function_templated.feature b/tests/behat/run_python3_sqr_function_templated.feature index 82025d9c7..b40922694 100644 --- a/tests/behat/run_python3_sqr_function_templated.feature +++ b/tests/behat/run_python3_sqr_function_templated.feature @@ -5,8 +5,7 @@ Feature: Combinator template is called test-by-test if a runtime error occurs wh I need the combinator template to be used test-by-test if my answer gives a runtime error Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/sandbox_webservice.feature b/tests/behat/sandbox_webservice.feature index aa072a8c4..abc03e328 100644 --- a/tests/behat/sandbox_webservice.feature +++ b/tests/behat/sandbox_webservice.feature @@ -4,7 +4,7 @@ Feature: Test sandbox web service server (Jobe) via Ajax. Background: - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher | Teacher | 1 | teacher@asd.com | | student | Student | 1 | student@asd.com | diff --git a/tests/behat/scratchpad_ui_params.feature b/tests/behat/scratchpad_ui_params.feature index 57332da91..312f48894 100644 --- a/tests/behat/scratchpad_ui_params.feature +++ b/tests/behat/scratchpad_ui_params.feature @@ -5,7 +5,8 @@ Feature: Test the Scratchpad UI, UI Params I should be able specify the UI Parameters to change the Scratchpad UI Background: - Given the following "users" exist: + Given the CodeRunner scratchpad is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: @@ -20,7 +21,6 @@ Feature: Test the Scratchpad UI, UI Params And the following "questions" exist: | questioncategory | qtype | name | template | | Test questions | coderunner | Print answer | printans | - And the CodeRunner scratchpad is enabled And I am on the "Print answer" "core_question > edit" page logged in as teacher1 And I set the field "id_validateonsave" to "" diff --git a/tests/behat/set_uiplugin.feature b/tests/behat/set_uiplugin.feature index a39d11b3e..53f60b1d0 100644 --- a/tests/behat/set_uiplugin.feature +++ b/tests/behat/set_uiplugin.feature @@ -5,8 +5,7 @@ Feature: Check that a selected UI plugin is saved I should be able to select a UI plugin and save the form Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/showdifferences_button.feature b/tests/behat/showdifferences_button.feature index c63b496c7..c244167eb 100644 --- a/tests/behat/showdifferences_button.feature +++ b/tests/behat/showdifferences_button.feature @@ -5,8 +5,7 @@ Feature: Show differences in CodeRunner questions clicked. Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/template_params_error.feature b/tests/behat/template_params_error.feature index 543c6d942..bce676548 100644 --- a/tests/behat/template_params_error.feature +++ b/tests/behat/template_params_error.feature @@ -5,8 +5,7 @@ Feature: template_params_error I should get informative template parameter error messages Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: diff --git a/tests/behat/twigprefix.feature b/tests/behat/twigprefix.feature index 4df509257..86eb75637 100644 --- a/tests/behat/twigprefix.feature +++ b/tests/behat/twigprefix.feature @@ -5,8 +5,7 @@ Feature: twigprefix I must be able to use the Twig prefix data in a question. Background: - Given the CodeRunner jobe sandbox is enabled - And the following "users" exist: + Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: From d363a186c54cf5e931cb040e450b8b8d80d33cfe Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Tue, 5 Mar 2024 16:25:20 +1300 Subject: [PATCH 13/15] Reverted behat fixtures to roughly what they were with minor improvements. Removed unneccesary Given jobe sandbox enabled lines. Removed the hack in jobesandbox.php in prep for merging with branch that has fixed the problem of reverting back to coderunner's default jobe2 server (jobe2.cosc.canterbury.ac.nz) during bhat tests. --- .gitignore | 2 +- classes/jobesandbox.php | 2 +- tests/behat/backup_and_restore.feature | 2 +- tests/behat/behat_coderunner.php | 23 +++++++++++++++++++++-- tests/behat/scratchpad_ui_params.feature | 4 ++-- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 67f0e8831..a249b3c77 100644 --- a/.gitignore +++ b/.gitignore @@ -34,4 +34,4 @@ NonRepoFiles/* /amd/src/ui_blockly.json /amd/src/.eslintrc.js .grunt - +.vscode diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index 5a0017123..810d349a9 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -307,7 +307,7 @@ private function get_main_class($prog) { } } - + // Return the sandbox error code corresponding to the given httpcode. private function get_error_code($httpcode) { diff --git a/tests/behat/backup_and_restore.feature b/tests/behat/backup_and_restore.feature index 870658339..8d3d73552 100644 --- a/tests/behat/backup_and_restore.feature +++ b/tests/behat/backup_and_restore.feature @@ -5,7 +5,7 @@ Feature: Duplicate a course containing a CodeRunner question I need to be able to back them up and restore them Background: - And the following "courses" exist: + Given the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | And the following "question categories" exist: diff --git a/tests/behat/behat_coderunner.php b/tests/behat/behat_coderunner.php index 2ecaa8c20..a1066f2f7 100644 --- a/tests/behat/behat_coderunner.php +++ b/tests/behat/behat_coderunner.php @@ -33,11 +33,30 @@ class behat_coderunner extends behat_base { * @Given /^the CodeRunner sandbox is enabled/ */ public function the_coderunner_sandbox_is_enabled() { - set_config('wsenabled', 1, 'qtype_coderunner'); set_config('jobesandbox_enabled', 1, 'qtype_coderunner'); - set_config('jobe_host', '172.17.0.1:4000', 'qtype_coderunner'); } + + /** + * Sets the webserver scratchpad to enabled for testing purposes. + * + * @Given /^the CodeRunner scratchpad is enabled/ + */ + public function the_coderunner_scratchpad_is_enabled() { + set_config('wsenabled', 1, 'qtype_coderunner'); + } + + + /** + * Sets the webserver scratchpad to disabled for testing purposes. + * + * @Given /^the CodeRunner scratchpad is disabled/ + */ + public function the_coderunner_scratchpad_is_disabled() { + set_config('wsenabled', 0, 'qtype_coderunner'); + } + + /** * Checks that a given string appears within answer textarea. * Intended for checking UI serialization diff --git a/tests/behat/scratchpad_ui_params.feature b/tests/behat/scratchpad_ui_params.feature index 2dd3d8111..312f48894 100644 --- a/tests/behat/scratchpad_ui_params.feature +++ b/tests/behat/scratchpad_ui_params.feature @@ -5,7 +5,8 @@ Feature: Test the Scratchpad UI, UI Params I should be able specify the UI Parameters to change the Scratchpad UI Background: - Given the following "users" exist: + Given the CodeRunner scratchpad is enabled + And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@asd.com | And the following "courses" exist: @@ -20,7 +21,6 @@ Feature: Test the Scratchpad UI, UI Params And the following "questions" exist: | questioncategory | qtype | name | template | | Test questions | coderunner | Print answer | printans | - And the CodeRunner sandbox is enabled And I am on the "Print answer" "core_question > edit" page logged in as teacher1 And I set the field "id_validateonsave" to "" From c3335290eb5405dc6052b47f8f59ba4cbb68bcb8 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Tue, 5 Mar 2024 18:26:50 +1300 Subject: [PATCH 14/15] Added attribute definitions to qtype_coderunner_combinator_grader_outcome class to adhere to new PHP8.2 standard. Also fixed minor bug caused by missed merge conflict --- classes/combinator_grader_outcome.php | 9 +++++++++ classes/jobesandbox.php | 3 --- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/classes/combinator_grader_outcome.php b/classes/combinator_grader_outcome.php index db025b00f..c61cc1da4 100644 --- a/classes/combinator_grader_outcome.php +++ b/classes/combinator_grader_outcome.php @@ -47,6 +47,15 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin /** @var bool If true, is used when the question is to be used only to display the output and perhaps images from a run, with no mark. */ public $showoutputonly; + /** @var array Array where each item is a rows of test result table */ + public $testresults; + + /** @var ?string The feedback for a given question attempt */ + public $feedbackhtml; + + /** @var bool Whether or no show differences is selected */ + public $showdifferences; + // A list of the allowed attributes in the combinator template grader return value. public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'epiloguehtml', 'feedbackhtml', 'columnformats', 'showdifferences', diff --git a/classes/jobesandbox.php b/classes/jobesandbox.php index 6a0406bd4..78056e9ee 100644 --- a/classes/jobesandbox.php +++ b/classes/jobesandbox.php @@ -74,12 +74,9 @@ public function __construct() { global $CFG; qtype_coderunner_sandbox::__construct(); $this->jobeserver = get_config('qtype_coderunner', 'jobe_host'); -<<<<<<< HEAD if ($this->jobeserver === 'jobe2.cosc.canterbury.ac.nz') { throw new Exception("PLease don't use the Canterbury jobe server for test runs"); } -======= ->>>>>>> grade_cache_for_merging $this->apikey = get_config('qtype_coderunner', 'jobe_apikey'); $this->languages = null; } From 60f5321891515d908c5331bbb97725b0d3e3afa8 Mon Sep 17 00:00:00 2001 From: Paul McKeown Date: Fri, 8 Mar 2024 15:36:21 +1300 Subject: [PATCH 15/15] Updated grade caching so that questions with different question text still use the cached outcome. That is, something has to change in the actual job being run to force a real regrade. --- classes/jobrunner.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/jobrunner.php b/classes/jobrunner.php index d6ee09956..713e116ad 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -167,7 +167,7 @@ private function run_combinator($isprecheck) { // affect caching. For example the questionid will change each time // the question is saved thanks to question versioning - urgh! foreach ($this->testcases as $tc) { - // unset($tc->id); May be ok to leave in there???? + unset($tc->id); unset($tc->questionid); } $numtests = count($this->testcases); @@ -248,7 +248,7 @@ private function run_tests_singly($isprecheck) { // affect caching. For example the questionid will change each time // the question is saved thanks to question versioning - urgh! foreach ($this->testcases as $tc) { - // unset($tc->id); May be Ok to leave there? + unset($tc->id); unset($tc->questionid); } $numtests = count($this->testcases);