From cc3e7418018aadf5a2e66389f78a4949c4af0f39 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 8 Jul 2020 15:34:10 +0800 Subject: [PATCH 1/4] MDL-69232 behat: Move suite tests to BeforeSuite hook --- lib/tests/behat/behat_hooks.php | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/tests/behat/behat_hooks.php b/lib/tests/behat/behat_hooks.php index 16a1c036424..26252661e05 100644 --- a/lib/tests/behat/behat_hooks.php +++ b/lib/tests/behat/behat_hooks.php @@ -204,6 +204,25 @@ class behat_hooks extends behat_base { } } + /** + * Run final tests before running the suite. + * + * @BeforeSuite + * @param BeforeSuiteScope $scope scope passed by event fired before suite. + */ + public static function before_suite_final_checks(BeforeSuiteScope $scope) { + $happy = defined('BEHAT_TEST'); + $happy = $happy && defined('BEHAT_SITE_RUNNING'); + $happy = $happy && php_sapi_name() == 'cli'; + $happy = $happy && behat_util::is_test_mode_enabled(); + $happy = $happy && behat_util::is_test_site(); + + if (!$happy) { + error_log('Behat only can modify the test database and the test dataroot!'); + exit(1); + } + } + /** * Gives access to moodle codebase, to keep track of feature start time. * @@ -283,15 +302,6 @@ class behat_hooks extends behat_base { public function before_scenario(BeforeScenarioScope $scope) { global $DB, $CFG; - // As many checks as we can. - if (!defined('BEHAT_TEST') || - !defined('BEHAT_SITE_RUNNING') || - php_sapi_name() != 'cli' || - !behat_util::is_test_mode_enabled() || - !behat_util::is_test_site()) { - throw new behat_stop_exception('Behat only can modify the test database and the test dataroot!'); - } - $moreinfo = 'More info in ' . behat_command::DOCS_URL; $driverexceptionmsg = 'Selenium server is not running, you need to start it to run tests that involve Javascript. ' . $moreinfo; try { From f216c901e161105ddf84028af9ada7350aad1c59 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 8 Jul 2020 15:35:11 +0800 Subject: [PATCH 2/4] MDL-69232 behat: Add debugging information to Selenium fails --- lib/tests/behat/behat_hooks.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/tests/behat/behat_hooks.php b/lib/tests/behat/behat_hooks.php index 26252661e05..e2c7e45a5be 100644 --- a/lib/tests/behat/behat_hooks.php +++ b/lib/tests/behat/behat_hooks.php @@ -309,12 +309,23 @@ class behat_hooks extends behat_base { } catch (CurlExec $e) { // Exception thrown by WebDriver, so only @javascript tests will be caugth; in // behat_util::check_server_status() we already checked that the server is running. - throw new behat_stop_exception($driverexceptionmsg); + throw new behat_stop_exception( + $driverexceptionmsg . '. ' . + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); } catch (DriverException $e) { - throw new behat_stop_exception($driverexceptionmsg); + throw new behat_stop_exception( + $driverexceptionmsg . '. ' . + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); } catch (UnknownError $e) { // Generic 'I have no idea' Selenium error. Custom exception to provide more feedback about possible solutions. - throw new behat_stop_exception($e->getMessage()); + throw new behat_stop_exception( + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); } $suitename = $scope->getSuite()->getName(); From 775b9a363e21a324ce526eedc9840d8e786f5bd5 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 8 Jul 2020 15:35:34 +0800 Subject: [PATCH 3/4] MDL-69232 behat: Make selenium start more fault tolerant The session was previously only started once, but has now been moved to restart between each test. Because of this the `$session->start()` is now called more frequently which exposes a pre-existing issue whereby Selenium will periodically fail to start. When the session was only started at the beginning of the test, and at occasional other times, it made sense to always stop the test runner because it signified an issue with obtaining the session at all. Since the session is restarted more frequently, and now between every test, it no longer makes sense to do this. The test should only be _stopped_ if the first attempt to obtain the Session fails. Once a Session has been retrieved at least once it no longer makes sense to assume that the failure was a configuration failure, but a connection failure. Therefore subsequent tests should be attempted. --- lib/tests/behat/behat_hooks.php | 71 ++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/lib/tests/behat/behat_hooks.php b/lib/tests/behat/behat_hooks.php index e2c7e45a5be..b5b12a13af2 100644 --- a/lib/tests/behat/behat_hooks.php +++ b/lib/tests/behat/behat_hooks.php @@ -293,6 +293,21 @@ class behat_hooks extends behat_base { } } + /** + * Helper function to restart the Mink session. + */ + protected function restart_session(): void { + $session = $this->getSession(); + if ($session->isStarted()) { + $session->restart(); + } else { + $session->start(); + } + if ($this->running_javascript() && $this->getSession()->getDriver()->getWebDriverSessionId() === 'session') { + throw new DriverException('Unable to create valid session'); + } + } + /** * Resets the test environment. * @@ -302,30 +317,35 @@ class behat_hooks extends behat_base { public function before_scenario(BeforeScenarioScope $scope) { global $DB, $CFG; - $moreinfo = 'More info in ' . behat_command::DOCS_URL; - $driverexceptionmsg = 'Selenium server is not running, you need to start it to run tests that involve Javascript. ' . $moreinfo; - try { - $session = $this->getSession(); - } catch (CurlExec $e) { - // Exception thrown by WebDriver, so only @javascript tests will be caugth; in - // behat_util::check_server_status() we already checked that the server is running. - throw new behat_stop_exception( - $driverexceptionmsg . '. ' . - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); - } catch (DriverException $e) { - throw new behat_stop_exception( - $driverexceptionmsg . '. ' . - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); - } catch (UnknownError $e) { - // Generic 'I have no idea' Selenium error. Custom exception to provide more feedback about possible solutions. - throw new behat_stop_exception( - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); + if (self::$initprocessesfinished) { + $this->restart_session(); + } else { + $moreinfo = 'More info in ' . behat_command::DOCS_URL; + $driverexceptionmsg = 'Selenium server is not running, you need to start it to run tests that involve Javascript. ' . $moreinfo; + + try { + $this->restart_session(); + } catch (CurlExec $e) { + // Exception thrown by WebDriver, so only @javascript tests will be caugth; in + // behat_util::check_server_status() we already checked that the server is running. + throw new behat_stop_exception( + $driverexceptionmsg . '. ' . + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); + } catch (DriverException $e) { + throw new behat_stop_exception( + $driverexceptionmsg . '. ' . + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); + } catch (UnknownError $e) { + // Generic 'I have no idea' Selenium error. Custom exception to provide more feedback about possible solutions. + throw new behat_stop_exception( + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); + } } $suitename = $scope->getSuite()->getName(); @@ -361,9 +381,6 @@ class behat_hooks extends behat_base { } - // Reset mink session between the scenarios. - $session->reset(); - // Reset $SESSION. \core\session\manager::init_empty_session(); From eec91baff9c13a9df6da894af659fe7fea556523 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 9 Jul 2020 09:10:09 +0800 Subject: [PATCH 4/4] MDL-69232 behat: Simplify hooks The behat hooks were needlessly complicated which made them much harder to read, and understand, leading to bugs during development. These have been significantly simplified to favour clarity over overloading. --- lib/tests/behat/behat_hooks.php | 224 ++++++++++++++++++-------------- 1 file changed, 129 insertions(+), 95 deletions(-) diff --git a/lib/tests/behat/behat_hooks.php b/lib/tests/behat/behat_hooks.php index b5b12a13af2..1ffcb14bcbe 100644 --- a/lib/tests/behat/behat_hooks.php +++ b/lib/tests/behat/behat_hooks.php @@ -68,6 +68,9 @@ class behat_hooks extends behat_base { */ protected static $initprocessesfinished = false; + /** @var bool Whether the first javascript scenario has been seen yet */ + protected static $firstjavascriptscenarioseen = false; + /** * @var bool Scenario running */ @@ -109,40 +112,23 @@ class behat_hooks extends behat_base { */ protected static $scenariotags; - /** - * Hook to capture BeforeSuite event so as to give access to moodle codebase. - * This will try and catch any exception and exists if anything fails. - * - * @param BeforeSuiteScope $scope scope passed by event fired before suite. - * @BeforeSuite - */ - public static function before_suite_hook(BeforeSuiteScope $scope) { - // If behat has been initialised then no need to do this again. - if (self::$initprocessesfinished) { - return; - } - - try { - self::before_suite($scope); - } catch (behat_stop_exception $e) { - echo $e->getMessage() . PHP_EOL; - exit(1); - } - } - /** * Gives access to moodle codebase, ensures all is ready and sets up the test lock. * - * Includes config.php to use moodle codebase with $CFG->behat_* - * instead of $CFG->prefix and $CFG->dataroot, called once per suite. + * Includes config.php to use moodle codebase with $CFG->behat_* instead of $CFG->prefix and $CFG->dataroot, called + * once per suite. * + * @BeforeSuite * @param BeforeSuiteScope $scope scope passed by event fired before suite. - * @static - * @throws behat_stop_exception */ - public static function before_suite(BeforeSuiteScope $scope) { + public static function before_suite_hook(BeforeSuiteScope $scope) { global $CFG; + // If behat has been initialised then no need to do this again. + if (!self::is_first_scenario()) { + return; + } + // Defined only when the behat CLI command is running, the moodle init setup process will // read this value and switch to $CFG->behat_dataroot and $CFG->behat_prefix instead of // the normal site. @@ -170,8 +156,7 @@ class behat_hooks extends behat_base { // before each scenario (accidental user deletes) in the BeforeScenario hook. if (!behat_util::is_test_mode_enabled()) { - throw new behat_stop_exception('Behat only can run if test mode is enabled. More info in ' . - behat_command::DOCS_URL); + self::log_and_stop('Behat only can run if test mode is enabled. More info in ' . behat_command::DOCS_URL); } // Reset all data, before checking for check_server_status. @@ -179,20 +164,28 @@ class behat_hooks extends behat_base { behat_util::clean_tables_updated_by_scenario_list(); behat_util::reset_all_data(); - // Check if server is running and using same version for cli and apache. + // Check if the web server is running and using same version for cli and apache. behat_util::check_server_status(); // Prevents using outdated data, upgrade script would start and tests would fail. if (!behat_util::is_test_data_updated()) { $commandpath = 'php admin/tool/behat/cli/init.php'; - throw new behat_stop_exception("Your behat test site is outdated, please run\n\n " . - $commandpath . "\n\nfrom your moodle dirroot to drop and install the behat test site again."); + $message = <<behat_faildump_path) && !is_writable($CFG->behat_faildump_path)) { - throw new behat_stop_exception('You set $CFG->behat_faildump_path to a non-writable directory'); + self::log_and_stop( + "The \$CFG->behat_faildump_path value is set to a non-writable directory ({$CFG->behat_faildump_path})." + ); } // Handle interrupts on PHP7. @@ -278,21 +271,6 @@ class behat_hooks extends behat_base { @file_put_contents(BEHAT_FEATURE_TIMING_FILE, json_encode(self::$timings, JSON_PRETTY_PRINT)); } - /** - * Hook to capture before scenario event to get scope. - * - * @param BeforeScenarioScope $scope scope passed by event fired before scenario. - * @BeforeScenario - */ - public function before_scenario_hook(BeforeScenarioScope $scope) { - try { - $this->before_scenario($scope); - } catch (behat_stop_exception $e) { - echo $e->getMessage() . PHP_EOL; - exit(1); - } - } - /** * Helper function to restart the Mink session. */ @@ -304,50 +282,98 @@ class behat_hooks extends behat_base { $session->start(); } if ($this->running_javascript() && $this->getSession()->getDriver()->getWebDriverSessionId() === 'session') { - throw new DriverException('Unable to create valid session'); + throw new DriverException('Unable to create a valid session'); } } + /** + * Restart the session before each non-javascript scenario. + * + * @BeforeScenario @~javascript + * @param BeforeScenarioScope $scope scope passed by event fired before scenario. + */ + public function before_goutte_scenarios(BeforeScenarioScope $scope) { + if ($this->running_javascript()) { + // A bug in the BeforeScenario filtering prevents the @~javascript filter on this hook from working + // properly. + // See https://github.com/Behat/Behat/issues/1235 for further information. + return; + } + + $this->restart_session(); + } + + /** + * Start the session before the first javascript scenario. + * + * This is treated slightly differently to try to capture when Selenium is not running at all. + * + * @BeforeScenario @javascript + * @param BeforeScenarioScope $scope scope passed by event fired before scenario. + */ + public function before_first_scenario_start_session(BeforeScenarioScope $scope) { + if (!self::is_first_javascript_scenario()) { + // The first Scenario has started. + // The `before_subsequent_scenario_start_session` function will restart the session instead. + return; + } + self::$firstjavascriptscenarioseen = true; + + $docsurl = behat_command::DOCS_URL; + $driverexceptionmsg = <<restart_session(); + } catch (CurlExec | DriverException $e) { + // The CurlExec Exception is thrown by WebDriver. + self::log_and_stop( + $driverexceptionmsg . '. ' . + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); + } catch (UnknownError $e) { + // Generic 'I have no idea' Selenium error. Custom exception to provide more feedback about possible solutions. + self::log_and_stop( + $e->getMessage() . "\n\n" . + format_backtrace($e->getTrace(), true) + ); + } + } + + /** + * Start the session before each javascript scenario. + * + * Note: Before the first scenario the @see before_first_scenario_start_session() function is used instead. + * + * @BeforeScenario @javascript + * @param BeforeScenarioScope $scope scope passed by event fired before scenario. + */ + public function before_subsequent_scenario_start_session(BeforeScenarioScope $scope) { + if (self::is_first_javascript_scenario()) { + // The initial init has not yet finished. + // The `before_first_scenario_start_session` function will have started the session instead. + return; + } + + $this->restart_session(); + } + /** * Resets the test environment. * + * @BeforeScenario * @param BeforeScenarioScope $scope scope passed by event fired before scenario. - * @throws behat_stop_exception If here we are not using the test database it should be because of a coding error */ - public function before_scenario(BeforeScenarioScope $scope) { - global $DB, $CFG; - - if (self::$initprocessesfinished) { - $this->restart_session(); - } else { - $moreinfo = 'More info in ' . behat_command::DOCS_URL; - $driverexceptionmsg = 'Selenium server is not running, you need to start it to run tests that involve Javascript. ' . $moreinfo; - - try { - $this->restart_session(); - } catch (CurlExec $e) { - // Exception thrown by WebDriver, so only @javascript tests will be caugth; in - // behat_util::check_server_status() we already checked that the server is running. - throw new behat_stop_exception( - $driverexceptionmsg . '. ' . - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); - } catch (DriverException $e) { - throw new behat_stop_exception( - $driverexceptionmsg . '. ' . - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); - } catch (UnknownError $e) { - // Generic 'I have no idea' Selenium error. Custom exception to provide more feedback about possible solutions. - throw new behat_stop_exception( - $e->getMessage() . "\n\n" . - format_backtrace($e->getTrace(), true) - ); - } - } - + public function before_scenario_hook(BeforeScenarioScope $scope) { + global $DB; $suitename = $scope->getSuite()->getName(); // Register behat selectors for theme, if suite is changed. We do it for every suite change. @@ -733,6 +759,15 @@ class behat_hooks extends behat_base { return !(self::$initprocessesfinished); } + /** + * Returns whether the first scenario of the suite is running + * + * @return bool + */ + protected static function is_first_javascript_scenario(): bool { + return !self::$firstjavascriptscenarioseen; + } + /** * Register a set of component selectors. * @@ -772,20 +807,19 @@ class behat_hooks extends behat_base { * @param BeforeStepScope $scope * @BeforeStep */ - public function first_step_setup_complete(BeforeStepScope $scope) { + public function first_step_setup_complete(BeforeStepScope $scope): void { self::$initprocessesfinished = true; } -} + /** + * Log a notification, and then exit. + * + * @param string $message The content to dispaly + */ + protected static function log_and_stop(string $message): void { + error_log($message); + + exit(1); + } -/** - * Behat stop exception - * - * This exception is thrown from before suite or scenario if any setup problem found. - * - * @package core_test - * @copyright 2016 Rajesh Taneja - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class behat_stop_exception extends \Exception { }