From 5c2d01abe9b86cc73048fcd96c7192b1d8505bb6 Mon Sep 17 00:00:00 2001 From: pdohogne-magento Date: Tue, 3 Jul 2018 11:09:08 -0500 Subject: [PATCH 1/3] MAGETWO-46837: Implementing extension to wait for readiness metrics to pass before executing test steps --- etc/config/codeception.dist.yml | 11 + .../Extension/PageReadinessExtension.php | 227 ++++++++++++ .../ReadinessMetrics/AbstractMetricCheck.php | 330 ++++++++++++++++++ .../ReadinessMetrics/DocumentReadyState.php | 45 +++ .../ReadinessMetrics/JQueryAjaxRequests.php | 57 +++ .../ReadinessMetrics/MagentoLoadingMasks.php | 56 +++ .../PrototypeAjaxRequests.php | 57 +++ .../ReadinessMetrics/RequireJsDefinitions.php | 62 ++++ 8 files changed, 845 insertions(+) create mode 100644 src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php create mode 100644 src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php diff --git a/etc/config/codeception.dist.yml b/etc/config/codeception.dist.yml index 273ede0b0..57582f1bd 100755 --- a/etc/config/codeception.dist.yml +++ b/etc/config/codeception.dist.yml @@ -15,6 +15,7 @@ extensions: - Codeception\Extension\RunFailed - Magento\FunctionalTestingFramework\Extension\TestContextExtension - Magento\FunctionalTestingFramework\Allure\Adapter\MagentoAllureAdapter + - Magento\FunctionalTestingFramework\Extension\PageReadinessExtension config: Yandex\Allure\Adapter\AllureAdapter: deletePreviousResults: true @@ -23,5 +24,15 @@ extensions: - env - zephyrId - useCaseId + Magento\FunctionalTestingFramework\Extension\PageReadinessExtension: + driver: \Magento\FunctionalTestingFramework\Module\MagentoWebDriver + timeout: 30 + resetFailureThreshold: 3 + readinessMetrics: + - \Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\DocumentReadyState + - \Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\JQueryAjaxRequests + - \Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\PrototypeAjaxRequests + - \Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\RequireJsDefinitions + - \Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\MagentoLoadingMasks params: - .env \ No newline at end of file diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php new file mode 100644 index 000000000..f288c6f4e --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php @@ -0,0 +1,227 @@ + 'beforeTest', + Events::STEP_BEFORE => 'beforeStep', + Events::STEP_AFTER => 'afterStep' + ]; + + /** + * @var Logger + */ + private $logger; + + /** + * Logger verbosity + * + * @var bool + */ + private $verbose; + + /** + * Array of readiness metrics, initialized during beforeTest event + * + * @var AbstractMetricCheck[] + */ + private $readinessMetrics; + + /** + * Active test object + * + * @var TestInterface + */ + private $test; + + /** + * Initialize local vars + * + * @return void + * @throws \Exception + */ + public function _initialize() + { + $this->logger = LoggingUtil::getInstance()->getLogger(get_class($this)); + $this->verbose = MftfApplicationConfig::getConfig()->verboseEnabled(); + } + + /** + * WebDriver instance to use to execute readiness metric checks + * + * @return WebDriver + * @throws ModuleRequireException + */ + public function getDriver() + { + return $this->getModule($this->config['driver']); + } + + /** + * Initialize the readiness metrics for the test + * + * @param \Codeception\Event\TestEvent $e + * @return void + */ + public function beforeTest(TestEvent $e) { + $this->test = $e->getTest(); + + if (isset($this->config['resetFailureThreshold'])) { + $failThreshold = intval($this->config['resetFailureThreshold']); + } + else { + $failThreshold = 3; + } + + $metrics = []; + foreach ($this->config['readinessMetrics'] as $metricClass) { + $metrics[] = new $metricClass($this, $this->test, $failThreshold); + } + + $this->readinessMetrics = $metrics; + } + + /** + * Waits for busy page flags to disappear before executing a step + * + * @param StepEvent $e + * @return void + * @throws \Exception + */ + public function beforeStep(StepEvent $e) { + $step = $e->getStep(); + if ($step->getAction() == 'saveScreenshot') { + return; + } + // $step->getArguments()['skipReadiness'] + + try { + $this->test->getMetadata()->setCurrent(['uri', $this->getDriver()->_getCurrentUri()]); + } + catch (\Exception $exception) { + // $this->debugLog('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]); + } + + + if (isset($this->config['timeout'])) { + $timeout = intval($this->config['timeout']); + } + else { + $timeout = $this->getDriver()->_getConfig()['pageload_timeout']; + } + + $metrics = $this->readinessMetrics; + + try { + $this->getDriver()->webDriver->wait($timeout)->until( + function () use ($metrics) { + $passing = true; + + /** @var AbstractMetricCheck $metric */ + foreach ($metrics as $metric) { + try { + if (!$metric->runCheck()) { + $passing = false; + } + } + catch (UnexpectedAlertOpenException $exception) {} + } + return $passing; + } + ); + } + catch (TimeoutException $exception) {} + + /** @var AbstractMetricCheck $metric */ + foreach ($metrics as $metric) { + $metric->finalize($step); + } + } + + /** + * Checks to see if the step changed the uri and resets failure tracking if so + * + * @param StepEvent $e + * @return void + */ + public function afterStep(StepEvent $e) { + $step = $e->getStep(); + if ($step->getAction() == 'saveScreenshot') { + return; + } + + try { + $currentUri = $this->getDriver()->_getCurrentUri(); + } + catch (\Exception $e) { + // $this->debugLog('Could not retrieve current URI', ['action' => $step()->getAction()]); + return; + } + + $previousUri = $this->test->getMetadata()->getCurrent('uri'); + + if ($previousUri !== $currentUri) { + $this->logDebug('Page URI changed; resetting readiness metric failure tracking', + [ + 'action' => $step->getAction(), + 'newUri' => $currentUri + ] + ); + + /** @var AbstractMetricCheck $metric */ + foreach ($this->readinessMetrics as $metric) { + $metric->setTracker(); + } + } + } + + /** + * If verbose, log the given message to logger->debug including test context information + * + * @param string $message + * @param array $context + */ + private function logDebug($message, $context = []) { + if ($this->verbose) { + $testMeta = $this->test->getMetadata(); + $logContext = [ + 'test' => $testMeta->getName(), + 'uri' => $testMeta->getCurrent('uri') + ]; + foreach ($this->readinessMetrics as $metric) { + $logContext[$metric->getName()] = $metric->getStoredValue(); + $logContext[$metric->getName() . '.failCount'] = $metric->getFailureCount(); + } + $context = array_merge($logContext, $context); + $this->logger->info($message, $context); + } + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php new file mode 100644 index 000000000..78ba40c4d --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php @@ -0,0 +1,330 @@ +extension = $extension; + $this->test = $test; + $this->logger = LoggingUtil::getInstance()->getLogger(get_class($this)); + $this->verbose = MftfApplicationConfig::getConfig()->verboseEnabled(); + + // If the clearFailureOnPage() method is overridden, use the configured failure threshold + // If not, the default clearFailureOnPage() method does nothing so don't worry about resetting failures + $reflector = new \ReflectionMethod($this, 'clearFailureOnPage'); + if ($reflector->getDeclaringClass()->getName() === get_class($this)) { + $this->resetFailureThreshold = $resetFailureThreshold; + } + else { + $this->resetFailureThreshold = -1; + } + + $this->setTracker(); + } + + /** + * Does the given value pass the readiness metric + * + * @param mixed $value + * @return bool + */ + protected abstract function doesMetricPass($value); + + /** + * Retrieve the active value for the metric to check from the page + * + * @return mixed + * @throws UnexpectedAlertOpenException + */ + protected abstract function fetchValueFromPage(); + + /** + * Override this method to reset the actual state of the page to make the metric pass + * This method is called when too many identical failures were encountered in a row + * + * @return void + */ + protected function clearFailureOnPage() { + return; + } + + /** + * Get the base class name of the metric implementation + * + * @return string + */ + public function getName() { + $clazz = get_class($this); + $namespaceBreak = strrpos($clazz, '\\'); + if ($namespaceBreak !== false) { + $clazz = substr($clazz, $namespaceBreak + 1); + } + return $clazz; + } + + /** + * Fetches a new value for the metric and checks if it passes, clearing the failure tracking if so + * + * Even on a success, the readiness check will continue to be run until all metrics pass at the same time in order + * to catch cases where a slow request of one metric can trigger calls for other metrics that were previously + * thought ready + * + * @return bool + * @throws UnexpectedAlertOpenException + */ + public function runCheck() { + if ($this->doesMetricPass($this->getCurrentValue(true))) { + $this->setTracker($this->getCurrentValue()); + return true; + } + + return false; + } + + /** + * Update the state of the metric including tracked failure state and checking if a failing value is stuck and + * needs to be reset so future checks can be accurate + * + * Called when the readiness check is finished (either all metrics pass or the check has timed out) + * + * @param Step $step + * @return void + */ + public function finalize($step) { + try { + $currentValue = $this->getCurrentValue(); + } + catch (UnexpectedAlertOpenException $exception) { + $this->debugLog( + 'An alert is open, bypassing javascript-based metric check', + ['action' => $step->getAction()] + ); + return; + } + + if ($this->doesMetricPass($currentValue)) { + $this->setTracker($currentValue); + } + else { + // If failure happened on the same value as before, increment the fail count, otherwise set at 1 + if ($currentValue !== $this->getStoredValue()) { + $failCount = 1; + } + else { + $failCount = $this->getFailureCount() + 1; + } + $this->setTracker($currentValue, $failCount); + + $this->errorLog('Failed readiness check', ['action' => $step->getAction()]); + + if ($this->resetFailureThreshold >= 0 && $failCount >= $this->resetFailureThreshold) { + $this->debugLog( + 'Too many failures, assuming metric is stuck and resetting state', + ['action' => $step->getAction()] + ); + $this->resetMetric(); + } + } + } + + /** + * Helper function to retrieve the driver being used to run the test + * + * @return WebDriver + * @throws ModuleRequireException + */ + protected function getDriver() { + return $this->extension->getDriver(); + } + + /** + * Helper function to execute javascript code, see WebDriver::executeJs for more information + * + * @param string $script + * @param array $arguments + * @return mixed + * @throws UnexpectedAlertOpenException + * @throws ModuleRequireException + */ + protected function executeJs($script, $arguments = []) { + return $this->getDriver()->executeJS($script, $arguments); + } + + /** + * Gets the current state of the given variable + * Fetches an updated value if not known or $refresh is true + * + * @param bool $refresh + * @return mixed + * @throws UnexpectedAlertOpenException + */ + private function getCurrentValue($refresh = false) { + if ($refresh) { + unset($this->currentValue); + } + if (!isset($this->currentValue)) { + $this->currentValue = $this->fetchValueFromPage(); + } + return $this->currentValue; + } + + /** + * Returns the value of the given variable for the previous check + * + * @return mixed + */ + public function getStoredValue() { + return $this->test->getMetadata()->getCurrent($this->getName()); + } + + /** + * The current count of sequential identical failures + * Used to detect potentially stuck metrics + * + * @return int + */ + public function getFailureCount() { + return $this->test->getMetadata()->getCurrent($this->getName() . '.failCount'); + } + + /** + * Update the state of the page to pass the metric and clear the saved failure state + * Called when a failure is found to be stuck + * + * @return void + */ + private function resetMetric() { + $this->clearFailureOnPage(); + $this->setTracker(); + } + + /** + * Tracks the most recent value and the number of identical failures in a row + * + * @param mixed $value + * @param int $failCount + * @return void + */ + public function setTracker($value = null, $failCount = 0) { + $this->test->getMetadata()->setCurrent([ + $this->getName() => $value, + $this->getName() . '.failCount' => $failCount + ]); + unset ($this->currentValue); + } + + /** + * Log the given message to logger->error including context information + * + * @param string $message + * @param array $context + */ + protected function errorLog($message, $context = []) { + $context = array_merge($this->getLogContext(), $context); + $this->logger->error($message, $context); + } + + /** + * Log the given message to logger->info including context information + * + * @param string $message + * @param array $context + */ + protected function infoLog($message, $context = []) { + $context = array_merge($this->getLogContext(), $context); + $this->logger->info($message, $context); + } + + /** + * If verbose, log the given message to logger->debug including context information + * + * @param string $message + * @param array $context + */ + protected function debugLog($message, $context = []) { + if ($this->verbose) { + $context = array_merge($this->getLogContext(), $context); + $this->logger->debug($message, $context); + } + } + + /** + * Base context information to include in all log messages: test name, current URI, metric state + * Reports most recent stored value, not current value, so call setTracker() first to update + * + * @return array + */ + private function getLogContext() { + $testMeta = $this->test->getMetadata(); + return [ + 'test' => $testMeta->getName(), + 'uri' => $testMeta->getCurrent('uri'), + $this->getName() => $this->getStoredValue(), + $this->getName() . '.failCount' => $this->getFailureCount() + ]; + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php new file mode 100644 index 000000000..cf2a82835 --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php @@ -0,0 +1,45 @@ +executeJS('return document.readyState;'); + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php new file mode 100644 index 000000000..f02574bee --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php @@ -0,0 +1,57 @@ +executeJS( + 'if (!!window.jQuery) { + return window.jQuery.active; + } + return 0;' + ) + ); + } + + /** + * Active request count can get stuck above zero if an exception is thrown during a callback, causing the + * ajax handler method to fail before decrementing the request count + * + * @return void + * @throws UnexpectedAlertOpenException + */ + protected function clearFailureOnPage() + { + $this->executeJS('if (!!window.jQuery) { window.jQuery.active = 0; };'); + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php new file mode 100644 index 000000000..234a6cd0d --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php @@ -0,0 +1,56 @@ +getDriver()->webDriver->findElements($driverLocator); + foreach ($maskElements as $element) { + try { + if ($element->isDisplayed()) { + return "$maskLocator : " . $element ->getID(); + } + } + catch (NoSuchElementException $e) {} + catch (StaleElementReferenceException $e) {} + } + } + return null; + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php new file mode 100644 index 000000000..5db1a2d46 --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php @@ -0,0 +1,57 @@ +executeJS( + 'if (!!window.Prototype) { + return window.Ajax.activeRequestCount; + } + return 0;' + ) + ); + } + + /** + * Active request count can get stuck above zero if an exception is thrown during a callback, causing the + * ajax handler method to fail before decrementing the request count + * + * @return void + * @throws UnexpectedAlertOpenException + */ + protected function clearFailureOnPage() + { + $this->executeJS('if (!!window.Prototype) { window.Ajax.activeRequestCount = 0; };'); + } +} diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php new file mode 100644 index 000000000..848b55cd0 --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php @@ -0,0 +1,62 @@ +executeJS($script); + if ($moduleInProgress === 'null') { + $moduleInProgress = null; + } + return $moduleInProgress; + } +} From 8d7cfe8c7cf7c2dcce8f703219adc170394f0816 Mon Sep 17 00:00:00 2001 From: pdohogne-magento Date: Mon, 9 Jul 2018 11:56:50 -0500 Subject: [PATCH 2/3] MAGETWO-46837: Fixing static test failures in readiness extension --- .../Extension/PageReadinessExtension.php | 40 ++++---- .../ReadinessMetrics/AbstractMetricCheck.php | 95 +++++++++++-------- .../ReadinessMetrics/DocumentReadyState.php | 2 +- .../ReadinessMetrics/JQueryAjaxRequests.php | 9 +- .../ReadinessMetrics/MagentoLoadingMasks.php | 8 +- .../PrototypeAjaxRequests.php | 9 +- .../ReadinessMetrics/RequireJsDefinitions.php | 4 +- 7 files changed, 92 insertions(+), 75 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php index f288c6f4e..006e38db1 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php +++ b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php @@ -44,7 +44,7 @@ class PageReadinessExtension extends Extension /** * Logger verbosity * - * @var bool + * @var boolean */ private $verbose; @@ -91,13 +91,13 @@ public function getDriver() * @param \Codeception\Event\TestEvent $e * @return void */ - public function beforeTest(TestEvent $e) { + public function beforeTest(TestEvent $e) + { $this->test = $e->getTest(); if (isset($this->config['resetFailureThreshold'])) { $failThreshold = intval($this->config['resetFailureThreshold']); - } - else { + } else { $failThreshold = 3; } @@ -116,25 +116,22 @@ public function beforeTest(TestEvent $e) { * @return void * @throws \Exception */ - public function beforeStep(StepEvent $e) { + public function beforeStep(StepEvent $e) + { $step = $e->getStep(); if ($step->getAction() == 'saveScreenshot') { return; } - // $step->getArguments()['skipReadiness'] try { $this->test->getMetadata()->setCurrent(['uri', $this->getDriver()->_getCurrentUri()]); + } catch (\Exception $exception) { + $this->logDebug('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]); } - catch (\Exception $exception) { - // $this->debugLog('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]); - } - if (isset($this->config['timeout'])) { $timeout = intval($this->config['timeout']); - } - else { + } else { $timeout = $this->getDriver()->_getConfig()['pageload_timeout']; } @@ -151,14 +148,14 @@ function () use ($metrics) { if (!$metric->runCheck()) { $passing = false; } + } catch (UnexpectedAlertOpenException $exception) { } - catch (UnexpectedAlertOpenException $exception) {} } return $passing; } ); + } catch (TimeoutException $exception) { } - catch (TimeoutException $exception) {} /** @var AbstractMetricCheck $metric */ foreach ($metrics as $metric) { @@ -172,7 +169,8 @@ function () use ($metrics) { * @param StepEvent $e * @return void */ - public function afterStep(StepEvent $e) { + public function afterStep(StepEvent $e) + { $step = $e->getStep(); if ($step->getAction() == 'saveScreenshot') { return; @@ -180,8 +178,7 @@ public function afterStep(StepEvent $e) { try { $currentUri = $this->getDriver()->_getCurrentUri(); - } - catch (\Exception $e) { + } catch (\Exception $e) { // $this->debugLog('Could not retrieve current URI', ['action' => $step()->getAction()]); return; } @@ -189,7 +186,8 @@ public function afterStep(StepEvent $e) { $previousUri = $this->test->getMetadata()->getCurrent('uri'); if ($previousUri !== $currentUri) { - $this->logDebug('Page URI changed; resetting readiness metric failure tracking', + $this->logDebug( + 'Page URI changed; resetting readiness metric failure tracking', [ 'action' => $step->getAction(), 'newUri' => $currentUri @@ -207,9 +205,11 @@ public function afterStep(StepEvent $e) { * If verbose, log the given message to logger->debug including test context information * * @param string $message - * @param array $context + * @param array $context + * @return void */ - private function logDebug($message, $context = []) { + private function logDebug($message, $context = []) + { if ($this->verbose) { $testMeta = $this->test->getMetadata(); $logContext = [ diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php index 78ba40c4d..65022dc2a 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php @@ -55,7 +55,7 @@ abstract class AbstractMetricCheck protected $logger; /** - * @var bool + * @var boolean */ protected $verbose; @@ -63,11 +63,12 @@ abstract class AbstractMetricCheck * Constructor, called from the beforeTest event * * @param PageReadinessExtension $extension - * @param TestInterface $test - * @param integer $resetFailureThreshold + * @param TestInterface $test + * @param integer $resetFailureThreshold * @throws \Exception */ - public function __construct($extension, $test, $resetFailureThreshold){ + public function __construct($extension, $test, $resetFailureThreshold) + { $this->extension = $extension; $this->test = $test; $this->logger = LoggingUtil::getInstance()->getLogger(get_class($this)); @@ -78,8 +79,7 @@ public function __construct($extension, $test, $resetFailureThreshold){ $reflector = new \ReflectionMethod($this, 'clearFailureOnPage'); if ($reflector->getDeclaringClass()->getName() === get_class($this)) { $this->resetFailureThreshold = $resetFailureThreshold; - } - else { + } else { $this->resetFailureThreshold = -1; } @@ -90,9 +90,9 @@ public function __construct($extension, $test, $resetFailureThreshold){ * Does the given value pass the readiness metric * * @param mixed $value - * @return bool + * @return boolean */ - protected abstract function doesMetricPass($value); + abstract protected function doesMetricPass($value); /** * Retrieve the active value for the metric to check from the page @@ -100,7 +100,7 @@ protected abstract function doesMetricPass($value); * @return mixed * @throws UnexpectedAlertOpenException */ - protected abstract function fetchValueFromPage(); + abstract protected function fetchValueFromPage(); /** * Override this method to reset the actual state of the page to make the metric pass @@ -108,7 +108,8 @@ protected abstract function fetchValueFromPage(); * * @return void */ - protected function clearFailureOnPage() { + protected function clearFailureOnPage() + { return; } @@ -117,7 +118,8 @@ protected function clearFailureOnPage() { * * @return string */ - public function getName() { + public function getName() + { $clazz = get_class($this); $namespaceBreak = strrpos($clazz, '\\'); if ($namespaceBreak !== false) { @@ -133,10 +135,11 @@ public function getName() { * to catch cases where a slow request of one metric can trigger calls for other metrics that were previously * thought ready * - * @return bool + * @return boolean * @throws UnexpectedAlertOpenException */ - public function runCheck() { + public function runCheck() + { if ($this->doesMetricPass($this->getCurrentValue(true))) { $this->setTracker($this->getCurrentValue()); return true; @@ -154,11 +157,11 @@ public function runCheck() { * @param Step $step * @return void */ - public function finalize($step) { + public function finalize($step) + { try { $currentValue = $this->getCurrentValue(); - } - catch (UnexpectedAlertOpenException $exception) { + } catch (UnexpectedAlertOpenException $exception) { $this->debugLog( 'An alert is open, bypassing javascript-based metric check', ['action' => $step->getAction()] @@ -168,13 +171,11 @@ public function finalize($step) { if ($this->doesMetricPass($currentValue)) { $this->setTracker($currentValue); - } - else { + } else { // If failure happened on the same value as before, increment the fail count, otherwise set at 1 if ($currentValue !== $this->getStoredValue()) { $failCount = 1; - } - else { + } else { $failCount = $this->getFailureCount() + 1; } $this->setTracker($currentValue, $failCount); @@ -197,7 +198,8 @@ public function finalize($step) { * @return WebDriver * @throws ModuleRequireException */ - protected function getDriver() { + protected function getDriver() + { return $this->extension->getDriver(); } @@ -205,12 +207,13 @@ protected function getDriver() { * Helper function to execute javascript code, see WebDriver::executeJs for more information * * @param string $script - * @param array $arguments + * @param array $arguments * @return mixed * @throws UnexpectedAlertOpenException * @throws ModuleRequireException */ - protected function executeJs($script, $arguments = []) { + protected function executeJs($script, $arguments = []) + { return $this->getDriver()->executeJS($script, $arguments); } @@ -218,11 +221,12 @@ protected function executeJs($script, $arguments = []) { * Gets the current state of the given variable * Fetches an updated value if not known or $refresh is true * - * @param bool $refresh + * @param boolean $refresh * @return mixed * @throws UnexpectedAlertOpenException */ - private function getCurrentValue($refresh = false) { + private function getCurrentValue($refresh = false) + { if ($refresh) { unset($this->currentValue); } @@ -237,7 +241,8 @@ private function getCurrentValue($refresh = false) { * * @return mixed */ - public function getStoredValue() { + public function getStoredValue() + { return $this->test->getMetadata()->getCurrent($this->getName()); } @@ -245,9 +250,10 @@ public function getStoredValue() { * The current count of sequential identical failures * Used to detect potentially stuck metrics * - * @return int + * @return integer */ - public function getFailureCount() { + public function getFailureCount() + { return $this->test->getMetadata()->getCurrent($this->getName() . '.failCount'); } @@ -257,7 +263,8 @@ public function getFailureCount() { * * @return void */ - private function resetMetric() { + private function resetMetric() + { $this->clearFailureOnPage(); $this->setTracker(); } @@ -265,25 +272,28 @@ private function resetMetric() { /** * Tracks the most recent value and the number of identical failures in a row * - * @param mixed $value - * @param int $failCount + * @param mixed $value + * @param integer $failCount * @return void */ - public function setTracker($value = null, $failCount = 0) { + public function setTracker($value = null, $failCount = 0) + { $this->test->getMetadata()->setCurrent([ $this->getName() => $value, $this->getName() . '.failCount' => $failCount ]); - unset ($this->currentValue); + unset($this->currentValue); } /** * Log the given message to logger->error including context information * * @param string $message - * @param array $context + * @param array $context + * @return void */ - protected function errorLog($message, $context = []) { + protected function errorLog($message, $context = []) + { $context = array_merge($this->getLogContext(), $context); $this->logger->error($message, $context); } @@ -292,9 +302,11 @@ protected function errorLog($message, $context = []) { * Log the given message to logger->info including context information * * @param string $message - * @param array $context + * @param array $context + * @return void */ - protected function infoLog($message, $context = []) { + protected function infoLog($message, $context = []) + { $context = array_merge($this->getLogContext(), $context); $this->logger->info($message, $context); } @@ -303,9 +315,11 @@ protected function infoLog($message, $context = []) { * If verbose, log the given message to logger->debug including context information * * @param string $message - * @param array $context + * @param array $context + * @return void */ - protected function debugLog($message, $context = []) { + protected function debugLog($message, $context = []) + { if ($this->verbose) { $context = array_merge($this->getLogContext(), $context); $this->logger->debug($message, $context); @@ -318,7 +332,8 @@ protected function debugLog($message, $context = []) { * * @return array */ - private function getLogContext() { + private function getLogContext() + { $testMeta = $this->test->getMetadata(); return [ 'test' => $testMeta->getName(), diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php index cf2a82835..e755fd7c1 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php @@ -25,7 +25,7 @@ class DocumentReadyState extends AbstractMetricCheck * Metric passes when document.readyState == 'complete' * * @param string $value - * @return bool + * @return boolean */ protected function doesMetricPass($value) { diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php index f02574bee..c005923d3 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/JQueryAjaxRequests.php @@ -18,8 +18,8 @@ class JQueryAjaxRequests extends AbstractMetricCheck /** * Metric passes once there are no remaining active requests * - * @param int $value - * @return bool + * @param integer $value + * @return boolean */ protected function doesMetricPass($value) { @@ -29,10 +29,11 @@ protected function doesMetricPass($value) /** * Grabs the number of active jQuery ajax requests if available * - * @return int + * @return integer * @throws UnexpectedAlertOpenException */ - protected function fetchValueFromPage() { + protected function fetchValueFromPage() + { return intval( $this->executeJS( 'if (!!window.jQuery) { diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php index 234a6cd0d..abda0cd39 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php @@ -24,11 +24,11 @@ class MagentoLoadingMasks extends AbstractMetricCheck * Metric passes once all loading masks are absent or invisible * * @param string|null $value - * @return bool + * @return boolean */ protected function doesMetricPass($value) { - return is_null($value); + return $value === null; } /** @@ -46,9 +46,9 @@ protected function fetchValueFromPage() if ($element->isDisplayed()) { return "$maskLocator : " . $element ->getID(); } + } catch (NoSuchElementException $e) { + } catch (StaleElementReferenceException $e) { } - catch (NoSuchElementException $e) {} - catch (StaleElementReferenceException $e) {} } } return null; diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php index 5db1a2d46..2fc8f70cb 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/PrototypeAjaxRequests.php @@ -18,8 +18,8 @@ class PrototypeAjaxRequests extends AbstractMetricCheck /** * Metric passes once there are no remaining active requests * - * @param int $value - * @return bool + * @param integer $value + * @return boolean */ protected function doesMetricPass($value) { @@ -29,10 +29,11 @@ protected function doesMetricPass($value) /** * Grabs the number of active prototype ajax requests if available * - * @return int + * @return integer * @throws UnexpectedAlertOpenException */ - protected function fetchValueFromPage() { + protected function fetchValueFromPage() + { return intval( $this->executeJS( 'if (!!window.Prototype) { diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php index 848b55cd0..87f50d118 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php @@ -21,11 +21,11 @@ class RequireJsDefinitions extends AbstractMetricCheck * Metric passes once there are no enabled modules waiting in the registry queue * * @param string|null $value - * @return bool + * @return boolean */ protected function doesMetricPass($value) { - return is_null($value); + return $value === null; } /** From ff467df26965b79d1f38d22a556054c6168cfd98 Mon Sep 17 00:00:00 2001 From: pdohogne-magento Date: Thu, 12 Jul 2018 17:25:25 -0500 Subject: [PATCH 3/3] MAGETWO-46837: Extracting metric tracking from test object and adding wait and comment to ignored action types --- .../Extension/PageReadinessExtension.php | 127 ++++++++++++------ .../ReadinessMetrics/AbstractMetricCheck.php | 72 ++++++---- .../ReadinessMetrics/DocumentReadyState.php | 2 - .../ReadinessMetrics/MagentoLoadingMasks.php | 2 - .../ReadinessMetrics/RequireJsDefinitions.php | 2 - 5 files changed, 128 insertions(+), 77 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php index 006e38db1..393d03644 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php +++ b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php @@ -12,7 +12,7 @@ use Codeception\Exception\ModuleRequireException; use Codeception\Extension; use Codeception\Module\WebDriver; -use Codeception\TestInterface; +use Codeception\Step; use Facebook\WebDriver\Exception\UnexpectedAlertOpenException; use Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\AbstractMetricCheck; use Facebook\WebDriver\Exception\TimeOutException; @@ -32,8 +32,18 @@ class PageReadinessExtension extends Extension */ public static $events = [ Events::TEST_BEFORE => 'beforeTest', - Events::STEP_BEFORE => 'beforeStep', - Events::STEP_AFTER => 'afterStep' + Events::STEP_BEFORE => 'beforeStep' + ]; + + /** + * List of action types that should bypass metric checks + * shouldSkipCheck() also checks for the 'Comment' step type, which doesn't follow the $step->getAction() pattern + * + * @var array + */ + private $ignoredActions = [ + 'saveScreenshot', + 'wait' ]; /** @@ -56,11 +66,18 @@ class PageReadinessExtension extends Extension private $readinessMetrics; /** - * Active test object + * The name of the active test * - * @var TestInterface + * @var string */ - private $test; + private $testName; + + /** + * The current URI of the active page + * + * @var string + */ + private $uri; /** * Initialize local vars @@ -93,17 +110,18 @@ public function getDriver() */ public function beforeTest(TestEvent $e) { - $this->test = $e->getTest(); - if (isset($this->config['resetFailureThreshold'])) { $failThreshold = intval($this->config['resetFailureThreshold']); } else { $failThreshold = 3; } + $this->testName = $e->getTest()->getMetadata()->getName(); + $this->uri = null; + $metrics = []; foreach ($this->config['readinessMetrics'] as $metricClass) { - $metrics[] = new $metricClass($this, $this->test, $failThreshold); + $metrics[] = new $metricClass($this, $failThreshold); } $this->readinessMetrics = $metrics; @@ -119,16 +137,13 @@ public function beforeTest(TestEvent $e) public function beforeStep(StepEvent $e) { $step = $e->getStep(); - if ($step->getAction() == 'saveScreenshot') { + if ($this->shouldSkipCheck($step)) { return; } - try { - $this->test->getMetadata()->setCurrent(['uri', $this->getDriver()->_getCurrentUri()]); - } catch (\Exception $exception) { - $this->logDebug('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]); - } + $this->checkForNewPage($step); + // todo: Implement step parameter to override global timeout configuration if (isset($this->config['timeout'])) { $timeout = intval($this->config['timeout']); } else { @@ -159,46 +174,75 @@ function () use ($metrics) { /** @var AbstractMetricCheck $metric */ foreach ($metrics as $metric) { - $metric->finalize($step); + $metric->finalizeForStep($step); } } /** - * Checks to see if the step changed the uri and resets failure tracking if so + * Check if the URI has changed and reset metric tracking if so * - * @param StepEvent $e + * @param Step $step * @return void */ - public function afterStep(StepEvent $e) + private function checkForNewPage($step) { - $step = $e->getStep(); - if ($step->getAction() == 'saveScreenshot') { - return; - } - try { $currentUri = $this->getDriver()->_getCurrentUri(); + + if ($this->uri !== $currentUri) { + $this->logDebug( + 'Page URI changed; resetting readiness metric failure tracking', + [ + 'step' => $step->__toString(), + 'newUri' => $currentUri + ] + ); + + /** @var AbstractMetricCheck $metric */ + foreach ($this->readinessMetrics as $metric) { + $metric->resetTracker(); + } + + $this->uri = $currentUri; + } } catch (\Exception $e) { - // $this->debugLog('Could not retrieve current URI', ['action' => $step()->getAction()]); - return; + $this->logDebug('Could not retrieve current URI', ['step' => $step->__toString()]); } + } - $previousUri = $this->test->getMetadata()->getCurrent('uri'); + /** + * Gets the active page URI from the start of the most recent step + * + * @return string + */ + public function getUri() + { + return $this->uri; + } - if ($previousUri !== $currentUri) { - $this->logDebug( - 'Page URI changed; resetting readiness metric failure tracking', - [ - 'action' => $step->getAction(), - 'newUri' => $currentUri - ] - ); + /** + * Gets the name of the active test + * + * @return string + */ + public function getTestName() + { + return $this->testName; + } - /** @var AbstractMetricCheck $metric */ - foreach ($this->readinessMetrics as $metric) { - $metric->setTracker(); - } + /** + * Should the given step bypass the readiness checks + * todo: Implement step parameter to bypass specific metrics (or all) instead of basing on action type + * + * @param Step $step + * @return boolean + */ + private function shouldSkipCheck($step) + { + if ($step instanceof Step\Comment || in_array($step->getAction(), $this->ignoredActions)) { + return true; } + return false; } /** @@ -211,10 +255,9 @@ public function afterStep(StepEvent $e) private function logDebug($message, $context = []) { if ($this->verbose) { - $testMeta = $this->test->getMetadata(); $logContext = [ - 'test' => $testMeta->getName(), - 'uri' => $testMeta->getCurrent('uri') + 'test' => $this->testName, + 'uri' => $this->uri ]; foreach ($this->readinessMetrics as $metric) { $logContext[$metric->getName()] = $metric->getStoredValue(); diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php index 65022dc2a..c5dd7891b 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php @@ -9,7 +9,6 @@ use Codeception\Exception\ModuleRequireException; use Codeception\Module\WebDriver; use Codeception\Step; -use Codeception\TestInterface; use Facebook\WebDriver\Exception\UnexpectedAlertOpenException; use Magento\FunctionalTestingFramework\Extension\PageReadinessExtension; use Magento\FunctionalTestingFramework\Util\Logger\LoggingUtil; @@ -29,18 +28,26 @@ abstract class AbstractMetricCheck protected $extension; /** - * The active test object + * Current state of the value the metric tracks * - * @var TestInterface + * @var mixed; */ - protected $test; + protected $currentValue; /** - * Current state of the value the metric tracks + * Most recent saved state of the value the metric tracks + * Updated when the metric passes or is finalized * * @var mixed; */ - protected $currentValue; + protected $storedValue; + + /** + * Current count of sequential identical failures + * + * @var integer; + */ + protected $failCount; /** * Number of sequential identical failures before force-resetting the metric @@ -63,14 +70,12 @@ abstract class AbstractMetricCheck * Constructor, called from the beforeTest event * * @param PageReadinessExtension $extension - * @param TestInterface $test * @param integer $resetFailureThreshold * @throws \Exception */ - public function __construct($extension, $test, $resetFailureThreshold) + public function __construct($extension, $resetFailureThreshold) { $this->extension = $extension; - $this->test = $test; $this->logger = LoggingUtil::getInstance()->getLogger(get_class($this)); $this->verbose = MftfApplicationConfig::getConfig()->verboseEnabled(); @@ -83,7 +88,7 @@ public function __construct($extension, $test, $resetFailureThreshold) $this->resetFailureThreshold = -1; } - $this->setTracker(); + $this->resetTracker(); } /** @@ -141,7 +146,7 @@ public function getName() public function runCheck() { if ($this->doesMetricPass($this->getCurrentValue(true))) { - $this->setTracker($this->getCurrentValue()); + $this->setTracker($this->getCurrentValue(), 0); return true; } @@ -157,35 +162,35 @@ public function runCheck() * @param Step $step * @return void */ - public function finalize($step) + public function finalizeForStep($step) { try { $currentValue = $this->getCurrentValue(); } catch (UnexpectedAlertOpenException $exception) { $this->debugLog( 'An alert is open, bypassing javascript-based metric check', - ['action' => $step->getAction()] + ['step' => $step->__toString()] ); return; } if ($this->doesMetricPass($currentValue)) { - $this->setTracker($currentValue); + $this->setTracker($currentValue, 0); } else { // If failure happened on the same value as before, increment the fail count, otherwise set at 1 - if ($currentValue !== $this->getStoredValue()) { + if (!isset($this->storedValue) || $currentValue !== $this->getStoredValue()) { $failCount = 1; } else { $failCount = $this->getFailureCount() + 1; } $this->setTracker($currentValue, $failCount); - $this->errorLog('Failed readiness check', ['action' => $step->getAction()]); + $this->errorLog('Failed readiness check', ['step' => $step->__toString()]); if ($this->resetFailureThreshold >= 0 && $failCount >= $this->resetFailureThreshold) { $this->debugLog( 'Too many failures, assuming metric is stuck and resetting state', - ['action' => $step->getAction()] + ['step' => $step->__toString()] ); $this->resetMetric(); } @@ -214,7 +219,7 @@ protected function getDriver() */ protected function executeJs($script, $arguments = []) { - return $this->getDriver()->executeJS($script, $arguments); + return $this->extension->getDriver()->executeJS($script, $arguments); } /** @@ -243,7 +248,7 @@ private function getCurrentValue($refresh = false) */ public function getStoredValue() { - return $this->test->getMetadata()->getCurrent($this->getName()); + return $this->storedValue; } /** @@ -254,7 +259,7 @@ public function getStoredValue() */ public function getFailureCount() { - return $this->test->getMetadata()->getCurrent($this->getName() . '.failCount'); + return $this->failCount; } /** @@ -266,7 +271,7 @@ public function getFailureCount() private function resetMetric() { $this->clearFailureOnPage(); - $this->setTracker(); + $this->resetTracker(); } /** @@ -276,13 +281,23 @@ private function resetMetric() * @param integer $failCount * @return void */ - public function setTracker($value = null, $failCount = 0) + public function setTracker($value, $failCount) + { + unset($this->currentValue); + $this->storedValue = $value; + $this->failCount = $failCount; + } + + /** + * Resets the tracked metric values on a new page or stuck failure + * + * @return void + */ + public function resetTracker() { - $this->test->getMetadata()->setCurrent([ - $this->getName() => $value, - $this->getName() . '.failCount' => $failCount - ]); unset($this->currentValue); + unset($this->storedValue); + $this->failCount = 0; } /** @@ -334,10 +349,9 @@ protected function debugLog($message, $context = []) */ private function getLogContext() { - $testMeta = $this->test->getMetadata(); return [ - 'test' => $testMeta->getName(), - 'uri' => $testMeta->getCurrent('uri'), + 'test' => $this->extension->getTestName(), + 'uri' => $this->extension->getUri(), $this->getName() => $this->getStoredValue(), $this->getName() . '.failCount' => $this->getFailureCount() ]; diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php index e755fd7c1..26cf91aa7 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php @@ -10,9 +10,7 @@ * Class DocumentReadyState */ -use Codeception\TestInterface; use Facebook\WebDriver\Exception\UnexpectedAlertOpenException; -use Magento\FunctionalTestingFramework\Extension\PageReadinessExtension; /** * Class DocumentReadyState diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php index abda0cd39..4f15524ba 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/MagentoLoadingMasks.php @@ -6,10 +6,8 @@ namespace Magento\FunctionalTestingFramework\Extension\ReadinessMetrics; -use Codeception\TestInterface; use Facebook\WebDriver\Exception\NoSuchElementException; use Facebook\WebDriver\Exception\StaleElementReferenceException; -use Magento\FunctionalTestingFramework\Extension\PageReadinessExtension; use Magento\FunctionalTestingFramework\Module\MagentoWebDriver; use WebDriverBy; diff --git a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php index 87f50d118..6df470123 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php +++ b/src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/RequireJsDefinitions.php @@ -6,9 +6,7 @@ namespace Magento\FunctionalTestingFramework\Extension\ReadinessMetrics; -use Codeception\TestInterface; use Facebook\WebDriver\Exception\UnexpectedAlertOpenException; -use Magento\FunctionalTestingFramework\Extension\PageReadinessExtension; /** * Class RequireJsDefinitions