diff --git a/.gitignore b/.gitignore index 67f0e8831..11b445d92 100644 --- a/.gitignore +++ b/.gitignore @@ -34,4 +34,4 @@ NonRepoFiles/* /amd/src/ui_blockly.json /amd/src/.eslintrc.js .grunt - +.vscode/ 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/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 5a0017123..78056e9ee 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; @@ -197,57 +200,86 @@ 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! + + $key = hash("md5", serialize($runspec)); + // Debugger: echo '
' . serialize($runspec) . '
';. + $runresult = $cache->get($key); // Unserializes the returned value :) false if not found. + if ($runresult) { + // echo $key . '-----------> FOUND' . '
'; . } } - $runresult = []; - $runresult['sandboxinfo'] = [ - 'jobeserver' => $this->jobeserver, - 'jobeapikey' => $this->apikey, - ]; + 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. + $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 ({$this->jobeserver}) 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; } @@ -256,58 +288,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/classes/jobrunner.php b/classes/jobrunner.php index 40d7efda7..713e116ad 100644 --- a/classes/jobrunner.php +++ b/classes/jobrunner.php @@ -36,6 +36,7 @@ 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. + /** @var ?array Array of sandbox params. */ private $sandboxparams = null; @@ -48,15 +49,32 @@ class qtype_coderunner_jobrunner { /** @var bool True if this grading is occurring because the student clicked the precheck button. */ private $isprecheck = false; - // 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. + * @return qtype_coderunner_combinator_grader_outcome $testoutcome that contains the outcome + * of the grading. + */ + public function run_tests( + $question, + $code, + $attachments, + $testcases, + $isprecheck, + $answerlanguage + ) { + if (empty($question->prototype)) { // Missing prototype. We can't run this question. $outcome = new qtype_coderunner_testing_outcome(0, 0, false); @@ -69,7 +87,8 @@ 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; } @@ -131,10 +150,11 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec if ($question->get_show_source()) { $outcome->sourcecodelist = $this->allruns; } + + return $outcome; } - // If the template is a combinator, try running all the tests in a single // go. // @@ -143,6 +163,13 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec // 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(); @@ -217,6 +244,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 new file mode 100644 index 000000000..7d0f403d1 --- /dev/null +++ b/db/caches.php @@ -0,0 +1,40 @@ +. + +/** + * 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, + 'maxsize' => 50000000, + 'simplekeys' => true, + 'simpledata' => false, + 'canuselocalstore' => true, + 'staticacceleration' => true, + 'staticaccelerationsize' => 1000000, + ], +]; diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index e9aba7d0f..5a930b413 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -1633,7 +1633,10 @@ private function validate_sample_answer() { if ($error) { return $error; } - [$mark, , $cachedata] = $this->formquestion->grade_response($response); + [$mark, $state, $cachedata] = $this->formquestion->grade_response( + $response, + false // Not a precheck. + ); } catch (Exception $e) { return $e->getMessage(); } diff --git a/lang/en/qtype_coderunner.php b/lang/en/qtype_coderunner.php index 2da4b4af5..14611f986 100644 --- a/lang/en/qtype_coderunner.php +++ b/lang/en/qtype_coderunner.php @@ -1343,3 +1343,7 @@ 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'; +$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 b6d6e1413..623c6d23a 100644 --- a/question.php +++ b/question.php @@ -35,6 +35,7 @@ /** * Represents a 'CodeRunner' question. */ + #[AllowDynamicProperties] class qtype_coderunner_question extends question_graded_automatically { public $testcases = null; // Array of testcases. @@ -219,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 @@ -250,6 +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->randomseed = $seed; // so we can see it when checking } // Retrieve the saved random number seed and reconstruct the template @@ -265,6 +270,7 @@ public function apply_attempt_state(question_attempt_step $step) { $seed = mt_rand(); } $this->evaluate_question_for_display($seed, $step); + $this->randomseed = $seed; // so we can see it when checking } @@ -329,6 +335,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 @@ -357,6 +376,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 @@ -369,6 +391,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'); @@ -399,31 +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. - $this->cachedfuncparams = ['lang' => $lang, 'seed' => $seed]; - $this->cachedevaldtemplateparams = $jsontemplateparams; return $jsontemplateparams; } @@ -828,8 +853,13 @@ public function grade_response(array $response, bool $isprecheck = false) { if ($isprecheck && empty($this->precheck)) { throw new coding_exception("Unexpected precheck"); } + $language = empty($response['language']) ? '' : $response['language']; $gradingreqd = true; + $testoutcomeserial = false; + + // 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); @@ -847,19 +877,30 @@ public function grade_response(array $response, bool $isprecheck = false) { // filenames and values being file contents. $code = $response['answer']; $attachments = $this->get_attached_files($response); + $this->stepinfo = self::step_info($response); + $this->stepinfo->graderstate = $response['graderstate'] ?? ""; $testcases = $this->filter_testcases($isprecheck, $this->precheck); $runner = new qtype_coderunner_jobrunner(); - $this->stepinfo = self::step_info($response); + // 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); + $testoutcome = $runner->run_tests( + $this, + $code, + $attachments, + $testcases, + $isprecheck, + $language + ); $testoutcomeserial = serialize($testoutcome); } - + // 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()) { @@ -875,8 +916,6 @@ public function grade_response(array $response, bool $isprecheck = false) { 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) { @@ -1024,23 +1063,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/settings.php b/settings.php index ad3ee03dc..39c40f44c 100644 --- a/settings.php +++ b/settings.php @@ -134,3 +134,14 @@ get_string('wsmaxcputime_desc', 'qtype_coderunner'), '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 +)); +*/ 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..1150d4c56 100644 --- a/tests/behat/attachmentimportexport.feature +++ b/tests/behat/attachmentimportexport.feature @@ -20,7 +20,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/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..55d713d25 100644 --- a/tests/behat/behat_coderunner.php +++ b/tests/behat/behat_coderunner.php @@ -30,14 +30,33 @@ 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('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/sandbox_webservice.feature b/tests/behat/sandbox_webservice.feature index f013dd353..abc03e328 100644 --- a/tests/behat/sandbox_webservice.feature +++ b/tests/behat/sandbox_webservice.feature @@ -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..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 "" diff --git a/tests/grader_test.php b/tests/grader_test.php index 5f7a1eed8..711859c04 100644 --- a/tests/grader_test.php +++ b/tests/grader_test.php @@ -63,7 +63,11 @@ public function test_regex_grader() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Note: Moodle's test helper question always uses an id of zero! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -94,7 +98,11 @@ public function test_nearequality_grader_right_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Note: Moodle's test helper question always uses an id of zero! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$mark, $grade, $cache] = $result; $testoutcome = unserialize($cache['_testoutcome']); // For debugging test. $this->assertEquals(1, $mark); @@ -125,7 +133,11 @@ public function test_nearequality_grader_wrong_answer() { pass EOCODE; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Note: Moodle's test helper question always uses an id of zero! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$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 ed633bded..6667cc0cc 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; @@ -215,7 +214,11 @@ 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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(0, $mark); $this->assertEquals(\question_state::$gradedwrong, $grade); @@ -231,7 +234,11 @@ 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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(1, $mark); $this->assertEquals(\question_state::$gradedright, $grade); @@ -257,21 +264,33 @@ 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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); [$mark, $grade, $cache] = $result; $this->assertEquals(\question_state::$gradedpartial, $grade); $this->assertTrue(abs($mark - 0.5 / 7.5) < 0.00001); @@ -285,11 +304,21 @@ public function test_customised_timeout() { print("Hello Python") EOT; $response = ['answer' => $slowsquare]; // Should time out. - [$mark, $grade, $cache] = $q->grade_response($response); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); + [$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); + // NOTE: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false, // Not a precheck. + ); + [$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..8739b4ea3 100644 --- a/tests/template_test.php +++ b/tests/template_test.php @@ -111,11 +111,19 @@ public function test_grading_template() { $q->allornothing = false; $code = "def sqr(n): return n * n\n"; $response = ['answer' => $code]; - $result = $q->grade_response($response); + // Note: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false // Not a precheck. + ); [$mark, $grade, $cache] = $result; $this->assertTrue(abs($mark - 24.0 / 31.0) < 0.000001); $q->allornothing = true; - $result = $q->grade_response($response); + // Note: qid of zero is reused with different question! + $result = $q->grade_response( + $response, + false // Not a precheck. + ); [$mark, $grade, $cache] = $result; $this->assertTrue($mark == 0.0); }