Skip to content

Commit

Permalink
Grade caching code merged into development branch (#205)
Browse files Browse the repository at this point in the history 
* 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).

* Response caching code for testing on a real server.
Code still under development.

* Response caching code for testing on a real server.
Code still under development.

* Minor update to allow running on php < v8...

* 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)

* Little bugfix...

* Trial with grade caching at jobesandbox.php level.

* Small tweaks while testing jobe caching.

* Tidied up code ready for trial on a real server

* Minor updates, removing useless bits and pieces.

* 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.

* 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.

* 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.

* 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

* 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.
  • Loading branch information
@mckeownp
mckeownp authored Mar 9, 2024
1 parent 7629ae0 commit 0bb30b6
Show file tree
Hide file tree
Showing 20 changed files with 397 additions and 151 deletions.
2 changes: 1 addition & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ NonRepoFiles/*
/amd/src/ui_blockly.json
/amd/src/.eslintrc.js
.grunt

.vscode/
2 changes: 1 addition & 1 deletion bulktestindex.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@
expander.innerHTML = 'Collapse';
expander.nextSibling.style.display = 'inline';
} else {
expander.innerTHML = 'Expand';
expander.innerHTML = 'Expand';
expander.nextSibling.style.display = 'none';
}
});
Expand Down
9 changes: 9 additions & 0 deletions classes/combinator_grader_outcome.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
187 changes: 111 additions & 76 deletions classes/jobesandbox.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 '<pre>' . serialize($runspec) . '</pre>';.
$runresult = $cache->get($key); // Unserializes the returned value :) false if not found.
if ($runresult) {
// echo $key . '-----------> FOUND' . '<br>'; .
}
}

$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 . '<br>';
}
}
$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;
}
Expand All @@ -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) {
Expand Down
56 changes: 45 additions & 11 deletions classes/jobrunner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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.
//
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 0bb30b6

Please sign in to comment.