From 9fe3f3d235a87dad4dc52556a93044fa15a9ad81 Mon Sep 17 00:00:00 2001 From: pdohogne-magento Date: Thu, 12 Jul 2018 17:25:25 -0500 Subject: [PATCH] MAGETWO-46837: Extracting metric tracking from test object and adding wait and comment to ignored action types --- .../Extension/PageReadinessExtension.php | 127 ++++++++++++------ .../ReadinessMetrics/AbstractMetricCheck.php | 67 +++++---- .../ReadinessMetrics/DocumentReadyState.php | 2 - .../ReadinessMetrics/MagentoLoadingMasks.php | 2 - .../ReadinessMetrics/RequireJsDefinitions.php | 2 - 5 files changed, 123 insertions(+), 77 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php index 006e38db1..0c2ba30b6 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 static $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(), PageReadinessExtension::$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..e08a4f1dd 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,18 @@ 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; + } + + 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 +344,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