MDL-52292 block testing generators: improve the API

* Refactor the block generator base class, to remove the amount
  of duplication required in base classes.

* Change the defaults that are filled in to be a little more natural.

* Make the Behat step 'Given the following "block" exist:' work.

admin/tool/behat/tests/behat/data_generators.feature

@@ -424,9 +424,9 @@ Feature: Set up contextual data for tests
     And the following "grade categories" exist:
       | fullname         | course |
       | Grade category 1 | C1     |
-     And the following "grade items" exist:
-       | itemname                  | course | outcome | gradecategory    |
-       | Test Outcome Grade Item 1 | C1     | OT1     | Grade category 1 |
+    And the following "grade items" exist:
+      | itemname                  | course | outcome | gradecategory    |
+      | Test Outcome Grade Item 1 | C1     | OT1     | Grade category 1 |
     And the following config values are set as admin:
       | enableoutcomes | 1 |
     When I log in as "admin"
@@ -441,3 +441,15 @@ Feature: Set up contextual data for tests
     And I expand all fieldsets
     And "//div[contains(@class, 'fitem')]/div[contains(@class, 'fitemtitle')]/div[contains(@class, fstaticlabel) and contains(., 'Grade category')]/../../div[contains(@class, 'felement') and contains(., 'Grade category 1')]" "xpath_element" should exist
     And I press "Cancel"
+
+  Scenario: Add a block
+    Given the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "blocks" exist:
+      | blockname    | contextlevel | reference | pagetypepattern | defaultregion |
+      | online_users | Course       | C1        | course-view-*   | site-pre      |
+    When I log in as "admin"
+    And I am on site homepage
+    And I follow "Course 1"
+    Then I should see "Online users"

blocks/online_users/tests/generator/lib.php

@@ -36,31 +36,6 @@
  */
 class block_online_users_generator extends testing_block_generator {
 
-    /**
-     * Create new block instance
-     * @param array|stdClass $record
-     * @param array $options
-     * @return stdClass activity record with extra cmid field
-     */
-    public function create_instance($record = null, array $options = null) {
-        global $DB, $CFG;
-        require_once("$CFG->dirroot/mod/page/locallib.php");
-
-        $this->instancecount++;
-
-        $record = (object)(array)$record;
-        $options = (array)$options;
-
-        $record = $this->prepare_record($record);
-
-        $id = $DB->insert_record('block_instances', $record);
-        context_block::instance($id);
-
-        $instance = $DB->get_record('block_instances', array('id'=>$id), '*', MUST_EXIST);
-
-        return $instance;
-    }
-
     /**
      * Create (simulated) logged in users and add some of them to groups in a course
      */

lib/testing/generator/block_generator.php

@@ -66,7 +66,8 @@ public function get_blockname() {
     }
 
     /**
-     * Fill in record defaults
+     * Fill in record defaults.
+     *
      * @param stdClass $record
      * @return stdClass
      */
@@ -76,19 +77,19 @@ protected function prepare_record(stdClass $record) {
             $record->parentcontextid = context_system::instance()->id;
         }
         if (!isset($record->showinsubcontexts)) {
-            $record->showinsubcontexts = 1;
+            $record->showinsubcontexts = 0;
         }
         if (!isset($record->pagetypepattern)) {
-            $record->pagetypepattern = '';
+            $record->pagetypepattern = '*';
         }
         if (!isset($record->subpagepattern)) {
             $record->subpagepattern = null;
         }
         if (!isset($record->defaultregion)) {
-            $record->defaultregion = '';
+            $record->defaultregion = 'side-pre';
         }
         if (!isset($record->defaultweight)) {
-            $record->defaultweight = '';
+            $record->defaultweight = 5;
         }
         if (!isset($record->configdata)) {
             $record->configdata = null;
@@ -97,10 +98,44 @@ protected function prepare_record(stdClass $record) {
     }
 
     /**
-     * Create a test block
-     * @param array|stdClass $record
-     * @param array $options
-     * @return stdClass activity record
+     * Create a test block instance.
+     *
+     * The $record passed in becomes the basis for the new row added to the
+     * block_instances table. You only need to supply the values of interest.
+     * Any missing values have sensible defaults filled in.
+     *
+     * The $options array provides additional data, not directly related to what
+     * will be inserted in the block_instance table, which may affect the block
+     * that is created. The meanings of any data passed here depends on the particular
+     * type of block being created.
+     *
+     * @param array|stdClass $record forms the basis for the entry to be inserted in the block_instances table.
+     * @param array $options further, block-specific options to control how the block is created.
+     * @return stdClass the block_instance record that has just been created.
      */
-    abstract public function create_instance($record = null, array $options = null);
+    public function create_instance($record = null, $options = array()) {
+        global $DB;
+
+        $this->instancecount++;
+
+        $record = (object)(array)$record;
+        $this->preprocess_record($record, $options);
+        $record = $this->prepare_record($record);
+
+        $id = $DB->insert_record('block_instances', $record);
+        context_block::instance($id);
+
+        $instance = $DB->get_record('block_instances', array('id' => $id), '*', MUST_EXIST);
+        return $instance;
+    }
+
+    /**
+     * Can be overridden to do block-specific processing. $record can be modified
+     * in-place.
+     *
+     * @param stdClass $record the data, before defaults are filled in.
+     * @param array $options further, block-specific options, as passed to {@link create_instance()}.
+     */
+    protected function preprocess_record(stdClass $record, array $options) {
+    }
 }

lib/testing/generator/data_generator.php

@@ -457,23 +457,45 @@ public function create_course_section($record = null, array $options = null) {
     }
 
     /**
-     * Create a test block
-     * @param string $blockname
-     * @param array|stdClass $record
-     * @param array $options
-     * @return stdClass block instance record
+     * Create a test block.
+     *
+     * The $record passed in becomes the basis for the new row added to the
+     * block_instances table. You only need to supply the values of interest.
+     * Any missing values have sensible defaults filled in, and ->blockname will be set based on $blockname.
+     *
+     * The $options array provides additional data, not directly related to what
+     * will be inserted in the block_instance table, which may affect the block
+     * that is created. The meanings of any data passed here depends on the particular
+     * type of block being created.
+     *
+     * @param string $blockname the type of block to create. E.g. 'html'.
+     * @param array|stdClass $record forms the basis for the entry to be inserted in the block_instances table.
+     * @param array $options further, block-specific options to control how the block is created.
+     * @return stdClass new block_instance record.
      */
     public function create_block($blockname, $record=null, array $options=null) {
         $generator = $this->get_plugin_generator('block_'.$blockname);
         return $generator->create_instance($record, $options);
     }
 
     /**
-     * Create a test module
-     * @param string $modulename
-     * @param array|stdClass $record
-     * @param array $options
-     * @return stdClass activity record
+     * Create a test activity module.
+     *
+     * The $record should contain the same data that you would call from
+     * ->get_data() when the mod_[type]_mod_form is submitted, except that you
+     * only need to supply values of interest. The only required value is
+     * 'course'. Any missing values will have a sensible default supplied.
+     *
+     * The $options array provides additional data, not directly related to what
+     * would come back from the module edit settings form, which may affect the activity
+     * that is created. The meanings of any data passed here depends on the particular
+     * type of activity being created.
+     *
+     * @param string $modulename the type of activity to create. E.g. 'forum' or 'quiz'.
+     * @param array|stdClass $record data, as if from the module edit settings form.
+     * @param array $options additional data that may affect how the module is created.
+     * @return stdClass activity record new new record that was just inserted in the table
+     *      like 'forum' or 'quiz', with a ->cmid field added.
      */
     public function create_module($modulename, $record=null, array $options=null) {
         $generator = $this->get_plugin_generator('mod_'.$modulename);

lib/tests/behat/behat_data_generators.php

@@ -112,6 +112,10 @@ class behat_data_generators extends behat_base {
             'required' => array('activity', 'idnumber', 'course'),
             'switchids' => array('course' => 'course', 'gradecategory' => 'gradecat')
         ),
+        'blocks' => array(
+            'datagenerator' => 'block_instance',
+            'required' => array('blockname', 'contextlevel', 'reference'),
+        ),
         'group members' => array(
             'datagenerator' => 'group_member',
             'required' => array('user', 'group'),
@@ -341,6 +345,39 @@ protected function process_activity($data) {
         }
     }
 
+    /**
+     * Add a block to a page.
+     *
+     * @param array $data should mostly match the fields of the block_instances table.
+     *     The block type is specified by blockname.
+     *     The parentcontextid is set from contextlevel and reference.
+     *     Missing values are filled in by testing_block_generator::prepare_record.
+     *     $data is passed to create_block as both $record and $options. Normally
+     *     the keys are different, so this is a way to let people set values in either place.
+     */
+    protected function process_block_instance($data) {
+
+        if (empty($data['blockname'])) {
+            throw new Exception('\'blocks\' requires the field \'block\' type to be specified');
+        }
+
+        if (empty($data['contextlevel'])) {
+            throw new Exception('\'blocks\' requires the field \'contextlevel\' to be specified');
+        }
+
+        if (!isset($data['reference'])) {
+            throw new Exception('\'blocks\' requires the field \'reference\' to be specified');
+        }
+
+        $context = $this->get_context($data['contextlevel'], $data['reference']);
+        $data['parentcontextid'] = $context->id;
+
+        // Pass $data as both $record and $options. I think that is unlikely to
+        // cause problems since the relevant key names are different.
+        // $options is not used in most blocks I have seen, but where it is, it is necessary.
+        $this->datagenerator->create_block($data['blockname'], $data, $data);
+    }
+
     /**
      * Adapter to enrol_user() data generator.
      * @throws Exception