Merge branch 'master' into development

classes/jobesandbox.php

@@ -53,6 +53,7 @@ class qtype_coderunner_jobesandbox extends qtype_coderunner_sandbox {
      *  - with haproxy try "balance hdr(X-CodeRunner-Job-Id)" (not tested)
      *  - with a Netscaler use rule-based persistence with expression
      *    HTTP.REQ.HEADER(“X-CodeRunner-Job-Id”)
+     *  - with cookies support
      */
     private $currentjobid = null;
 
@@ -74,7 +75,8 @@ public function __construct() {
         global $CFG;
         qtype_coderunner_sandbox::__construct();
         $this->jobeserver = get_config('qtype_coderunner', 'jobe_host');
-        if ($this->jobeserver === 'jobe2.cosc.canterbury.ac.nz' && $CFG->prefix === $CFG->behat_prefix) {
+        if (qtype_coderunner_sandbox::is_canterbury_server($this->jobeserver)
+                && qtype_coderunner_sandbox::is_using_test_sandbox()) {
             throw new Exception("Please don't use the Canterbury jobe server for test runs");
         }
         $this->apikey = get_config('qtype_coderunner', 'jobe_apikey');
@@ -139,6 +141,8 @@ public function get_languages() {
      */
 
     public function execute($sourcecode, $language, $input, $files = null, $params = null) {
+        global $CFG;
+
         $language = strtolower($language);
         if (is_null($input)) {
             $input = '';
@@ -203,6 +207,18 @@ public function execute($sourcecode, $language, $input, $files = null, $params =
         // QUESTION: Do we need this when using cached result?
         $this->currentjobid = sprintf('%08x', mt_rand());
 
+        // Create a single curl object here, with support for cookies, and use it for all requests.
+        // This supports Jobe back-ends that use cookies for sticky load-balancing.
+        // Make a place to store the cookies.
+        make_temp_directory('qtype_coderunner');
+        $cookiefile = $CFG->tempdir . '/qtype_coderunner/session_cookies_' . $this->currentjobid . '.txt';
+
+        $curl = new curl();
+        $curl->setopt([
+                'CURLOPT_COOKIEJAR' => $cookiefile,
+                'CURLOPT_COOKIEFILE' => $cookiefile,
+        ]);
+
         $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache');
         $runresult = null;
         if (READ_FROM_CACHE) {
@@ -227,19 +243,22 @@ public function execute($sourcecode, $language, $input, $files = null, $params =
             $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);
+            $httpcode = $this->submit($postbody, $curl);
             if ($httpcode == 404) { // If it's a file not found error ...
                 foreach ($files as $filename => $contents) {
-                    if (($httpcode = $this->put_file($contents)) != 204) {
+                    if (($httpcode = $this->put_file($contents, $curl)) != 204) {
                         break;
                     }
                 }
                 if ($httpcode == 204) {
                     // Try again if put_files all worked.
-                    $httpcode = $this->submit($postbody);
+                    $httpcode = $this->submit($postbody, $curl);
                 }
             }
 
+        // Delete the cookie file.
+        unlink($cookiefile);
+
             $runresult = [];
             $runresult['sandboxinfo'] = [
                 'jobeserver' => $this->jobeserver,
@@ -362,28 +381,20 @@ private function get_error_code($httpcode) {
     }
 
     // Put the given file to the server, using its MD5 checksum as the id.
+    // If you pass a curl object, this will be used to make the request.
     // Returns the HTTP response code, or -1 if the HTTP request fails
     // altogether.
-    // Moodle curl class doesn't support an appropriate form of PUT so
-    // we use raw PHP curl here.
-    private function put_file($contents) {
+    private function put_file($contents, $curl) {
         $id = md5($contents);
         $contentsb64 = base64_encode($contents);
         $resource = "files/$id";
 
         [$url, $headers] = $this->get_jobe_connection_info($resource);
 
         $body = ['file_contents' => $contentsb64];
-        $curl = curl_init();
-        curl_setopt($curl, CURLOPT_HTTPHEADER, $headers);
-        curl_setopt($curl, CURLOPT_CUSTOMREQUEST, "PUT");
-        curl_setopt($curl, CURLOPT_URL, $url);
-        curl_setopt($curl, CURLOPT_POSTFIELDS, json_encode($body));
-        curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1);
-        $result = curl_exec($curl);
-        $info = curl_getinfo($curl);
-        curl_close($curl);
-        return $result === false ? -1 : $info['http_code'];
+        $result = $curl->put($url, json_encode($body));
+        $returncode = $curl->info['http_code'];
+        return $result === false ? -1 : $returncode;
     }
 
     /**
@@ -437,8 +448,9 @@ private function get_jobe_connection_info($resource) {
     // response was 400 Bad Parameter.
     // We don't at this stage deal with Jobe servers that may defer requests
     // i.e. that return 202 Accepted rather than 200 OK.
-    private function submit($job) {
-        [$returncode, $response] = $this->http_request('runs', self::HTTP_POST, $job);
+    // If you pass a curl object, this will be used to make the request.
+    private function submit($job, $curl) {
+        [$returncode, $response] = $this->http_request('runs', self::HTTP_POST, $job, $curl);
         $this->response = $response;
         return $returncode;
     }
@@ -453,10 +465,13 @@ private function submit($job) {
     // Note that the Moodle curl class documentation lies when it says the
     // return value from get and post is a bool. It's either the value false
     // if the request failed or the actual string response, otherwise.
-    private function http_request($resource, $method, $body = null) {
+    // If you pass a curl object, this will be used to make the request.
+    private function http_request($resource, $method, $body = null, $curl = null) {
         [$url, $headers] = $this->get_jobe_connection_info($resource);
 
-        $curl = new curl();
+        if ($curl == null) {
+            $curl = new curl();
+        }
         $curl->setHeader($headers);
 
         if ($method === self::HTTP_GET) {

classes/sandbox.php

@@ -34,11 +34,9 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-global $CFG;
+use qtype_coderunner\constants;
 
-if ($CFG->prefix == $CFG->behat_prefix) {
-    require_once($CFG->dirroot .'/question/type/coderunner/tests/fixtures/test-sandbox-config.php');
-}
+global $CFG;
 
 abstract class qtype_coderunner_sandbox {
     protected $user;     // Username supplied when constructing.
@@ -209,6 +207,24 @@ public static function enabled_sandboxes() {
         return $enabled;
     }
 
+    /**
+     * Returns true if sandbox is being used for tests.
+     * @return bool
+     */
+    public static function is_using_test_sandbox(): bool {
+        global $CFG;
+        return !empty($CFG->behat_prefix) && $CFG->prefix === $CFG->behat_prefix;
+    }
+
+    /**
+     * Returns true if canterbury jobe server is being used.
+     * @param string jobeserver being used.
+     * @return bool
+     */
+    public static function is_canterbury_server(string $jobeserver): bool {
+        return $jobeserver === constants::JOBE_HOST_DEFAULT;
+    }
+
     /**
      * Get the filename containing the given external sandbox name.
      * @param string $externalsandboxname

edit_coderunner_form.php

@@ -250,7 +250,7 @@ protected function add_sample_answer_field($mform) {
         $mform->addHelpButton('sampleanswerattachments', 'sampleanswerattachments', 'qtype_coderunner');
         // Unless behat is running, hide the attachments file picker.
         // behat barfs if it's hidden.
-        if ($CFG->prefix !== $CFG->behat_prefix) {
+        if (!qtype_coderunner_sandbox::is_using_test_sandbox()) {
             $method = method_exists($mform, 'hideIf') ? 'hideIf' : 'disabledIf';
             $mform->$method('sampleanswerattachments', 'attachments', 'eq', 0);
         }
@@ -1635,7 +1635,8 @@ private function validate_sample_answer() {
             }
             [$mark, $state, $cachedata] = $this->formquestion->grade_response(
                 $response,
-                false // Not a precheck.
+                false, // Not a precheck.
+                true   // Is a validation run.
             );
         } catch (Exception $e) {
             return $e->getMessage();

question.php

@@ -843,13 +843,15 @@ 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 $isvalidationrun true iff this is a validation run when saving 
+     * a question.
      * @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) {
+    public function grade_response(array $response, bool $isprecheck = false, $isvalidationrun = false) {
         if ($isprecheck && empty($this->precheck)) {
             throw new coding_exception("Unexpected precheck");
         }
@@ -877,9 +879,13 @@ public function grade_response(array $response, bool $isprecheck = false) {
             // filenames and values being file contents.
             $code = $response['answer'];
             $attachments = $this->get_attached_files($response);
+            if ($isvalidationrun) {
+                $testcases = $this->testcases;
+            } else {
+                $testcases = $this->filter_testcases($isprecheck, $this->precheck);
+            }
             $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'])) {

renderer.php

@@ -411,7 +411,8 @@ protected function build_results_table($outcome, qtype_coderunner_question $ques
         if (isset($sandboxinfo['jobeserver'])) {
             $jobeserver = $sandboxinfo['jobeserver'];
             $apikey = $sandboxinfo['jobeapikey'];
-            if ($jobeserver == constants::JOBE_HOST_DEFAULT && $CFG->prefix !== $CFG->behat_prefix) {
+            if (qtype_coderunner_sandbox::is_canterbury_server($jobeserver)
+                    && (!qtype_coderunner_sandbox::is_using_test_sandbox())) {
                 if ($apikey == constants::JOBE_HOST_DEFAULT_API_KEY) {
                     $fb .= get_string('jobe_warning_html', 'qtype_coderunner');
                 } else {

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 CodeRunner webservice is enabled
+    Given the CodeRunner test configuration file is loaded
+    And the CodeRunner webservice is enabled
     And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |

tests/behat/attachmentimportexport.feature

@@ -5,7 +5,9 @@ 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 test configuration file is loaded
+    And the CodeRunner webservice is enabled
+    And the following "users" exist:
       | username |
       | teacher  |
     And the following "courses" exist:
@@ -20,7 +22,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 webservice 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"

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
     And the following "courses" exist:

tests/behat/backup_and_restore.feature

@@ -5,7 +5,8 @@ Feature: Duplicate a course containing a CodeRunner question
   I need to be able to back them up and restore them
 
   Background:
-    Given the following "courses" exist:
+    Given the CodeRunner test configuration file is loaded
+    And the following "courses" exist:
       | fullname | shortname | category |
       | Course 1 | C1        | 0        |
     And the following "question categories" exist:

tests/behat/behat_coderunner.php

@@ -27,11 +27,23 @@
 
 
 class behat_coderunner extends behat_base {
-     /**
-      * Sets the webserver sandbox to enabled for testing purposes.
-      *
-      * @Given /^the CodeRunner webservice is enabled/
-      */
+
+    /**
+     * Loads the default coderunner settings file for testing.
+     * It seems silly that I have to do that. Why is there not
+     * a global behat configuration option apply to all features?
+     * @Given /^the CodeRunner test configuration file is loaded/
+     */
+    public function the_coderunner_test_configuration_file_is_loaded() {
+        global $CFG;
+        require($CFG->dirroot .'/question/type/coderunner/tests/fixtures/test-sandbox-config.php');
+    }
+
+    /**
+     * Enables the CodeRunner webservice for testing purposes.
+     *
+     * @Given /^the CodeRunner webservice is enabled/
+     */
     public function the_coderunner_webservice_is_enabled() {
         set_config('wsenabled', 1, 'qtype_coderunner');
     }
@@ -47,6 +59,15 @@ public function the_coderunner_webservice_is_disabled() {
     }
 
 
+    /**
+     * Disables the Jobe sandbox. Currently unused/untested.
+     *
+     * @Given /^the Jobe sandbox is disabled/
+     */
+    public function the_jobe_sandbox_is_disabled() {
+        set_config('jobesandbox_enabled', 0, 'qtype_coderunner');
+    }
+
     /**
      * Checks that a given string appears within answer textarea.
      * Intended for checking UI serialization

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
     And the following "courses" exist:

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname       | lastname  | email            |
       | teacher1 | Teacher         | Last      | teacher1@asd.com |
       | student1 | Student First   | O'Connell | student@asd.com  |

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
       | student1 | Student   | 1        | student@asd.com  |

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
       | student1 | Student   | 1        | student@asd.com  |

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
     And the following "courses" exist:
@@ -27,8 +28,10 @@ Feature: Create a CodeRunner question (the sqr function example)
       | id_questiontext   | Write a sqr function    |
       | id_testcode_0     | print(sqr(-7))          |
       | id_expected_0     | 49                      |
+      | id_ordering_0     | 20                      |
       | id_testcode_1     | print(sqr(11))          |
       | id_expected_1     | 120                     |
+      | id_ordering_1     | 10                      |
     Then I should see "Failed 1 test(s)"
     And I should see "Click on the << button to replace the expected output of this testcase with actual output."
     And the field "Customise" matches value "0"

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 test configuration file is loaded
+    And the following "users" exist:
       | username | firstname | lastname | email            |
       | teacher1 | Teacher   | 1        | teacher1@asd.com |
     And the following "courses" exist: