From eb3884e48f40e05fdf5a7caa01fdfda4934e94ee Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 24 Nov 2015 15:58:54 +0000 Subject: [PATCH] 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. --- .../behat/tests/behat/data_generators.feature | 18 +++++- blocks/online_users/tests/generator/lib.php | 25 --------- lib/testing/generator/block_generator.php | 55 +++++++++++++++---- lib/testing/generator/data_generator.php | 42 ++++++++++---- lib/tests/behat/behat_data_generators.php | 37 +++++++++++++ 5 files changed, 129 insertions(+), 48 deletions(-) diff --git a/admin/tool/behat/tests/behat/data_generators.feature b/admin/tool/behat/tests/behat/data_generators.feature index 9853ea51896..0f3e3afc8ee 100644 --- a/admin/tool/behat/tests/behat/data_generators.feature +++ b/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" diff --git a/blocks/online_users/tests/generator/lib.php b/blocks/online_users/tests/generator/lib.php index a560fd51caf..6b721a997d7 100644 --- a/blocks/online_users/tests/generator/lib.php +++ b/blocks/online_users/tests/generator/lib.php @@ -36,31 +36,6 @@ defined('MOODLE_INTERNAL') || die(); */ 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 */ diff --git a/lib/testing/generator/block_generator.php b/lib/testing/generator/block_generator.php index c5ed29a7c38..1305ee1ac6a 100644 --- a/lib/testing/generator/block_generator.php +++ b/lib/testing/generator/block_generator.php @@ -66,7 +66,8 @@ abstract class testing_block_generator extends component_generator_base { } /** - * Fill in record defaults + * Fill in record defaults. + * * @param stdClass $record * @return stdClass */ @@ -76,19 +77,19 @@ abstract class testing_block_generator extends component_generator_base { $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 @@ abstract class testing_block_generator extends component_generator_base { } /** - * 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) { + } } diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index c15bfa9d669..8b7678bd4e8 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -457,11 +457,21 @@ EOD; } /** - * 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); @@ -469,11 +479,23 @@ EOD; } /** - * 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); diff --git a/lib/tests/behat/behat_data_generators.php b/lib/tests/behat/behat_data_generators.php index d7f2ec03962..657b351ff4c 100644 --- a/lib/tests/behat/behat_data_generators.php +++ b/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 @@ class behat_data_generators extends behat_base { } } + /** + * 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