-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MAGETWO-46837: Implementing extension to wait for readiness metrics t… #161
Merged
KevinBKozan
merged 5 commits into
magento:develop
from
magento-borg:MAGETWO-46837-readiness-flag-develop
Jul 16, 2018
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
5c2d01a
MAGETWO-46837: Implementing extension to wait for readiness metrics t…
pdohogne-magento fadb951
Merge remote-tracking branch 'mainline/develop' into MAGETWO-46837-re…
pdohogne-magento 8d7cfe8
MAGETWO-46837: Fixing static test failures in readiness extension
pdohogne-magento ff467df
MAGETWO-46837: Extracting metric tracking from test object and adding…
pdohogne-magento b462aa8
Merge branch 'develop' into MAGETWO-46837-readiness-flag-develop
KevinBKozan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
227 changes: 227 additions & 0 deletions
227
src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,227 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
|
||
namespace Magento\FunctionalTestingFramework\Extension; | ||
|
||
use Codeception\Event\StepEvent; | ||
use Codeception\Event\TestEvent; | ||
use Codeception\Events; | ||
use Codeception\Exception\ModuleRequireException; | ||
use Codeception\Extension; | ||
use Codeception\Module\WebDriver; | ||
use Codeception\TestInterface; | ||
use Facebook\WebDriver\Exception\UnexpectedAlertOpenException; | ||
use Magento\FunctionalTestingFramework\Extension\ReadinessMetrics\AbstractMetricCheck; | ||
use Facebook\WebDriver\Exception\TimeOutException; | ||
use Magento\FunctionalTestingFramework\Util\Logger\LoggingUtil; | ||
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; | ||
use Monolog\Logger; | ||
|
||
/** | ||
* Class PageReadinessExtension | ||
*/ | ||
class PageReadinessExtension extends Extension | ||
{ | ||
/** | ||
* Codeception Events Mapping to methods | ||
* | ||
* @var array | ||
*/ | ||
public static $events = [ | ||
Events::TEST_BEFORE => 'beforeTest', | ||
Events::STEP_BEFORE => 'beforeStep', | ||
Events::STEP_AFTER => 'afterStep' | ||
]; | ||
|
||
/** | ||
* @var Logger | ||
*/ | ||
private $logger; | ||
|
||
/** | ||
* Logger verbosity | ||
* | ||
* @var boolean | ||
*/ | ||
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; | ||
} | ||
|
||
try { | ||
$this->test->getMetadata()->setCurrent(['uri', $this->getDriver()->_getCurrentUri()]); | ||
} catch (\Exception $exception) { | ||
$this->logDebug('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 | ||
* @return void | ||
*/ | ||
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); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of storing the test param, what do you think about just keeping it as a function variable for each of your event functions? I see the only place you use it where you don't have access to an event is in the logDebug function, but you could pass it in as an arg instead of relying on the class to store the test variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That depends on how Codeception handles events. I use the test object from the beforeTest event to track metric failure data ( $test->getMetadata()->setCurrent() ), so if the test object attached to the step events gets recreated or the metadata isn't otherwise persisted between events then that tracking functionality is lost.
I could move that tracking out of the test object and into an extension-specific construct directly instead, though I'd still be passing that around between metrics. Any objections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm that's a good point. I guess I'd only be concerned about tethering ourselves to codeception's test object for your metadata, but I don't feel that strongly about it atm. @okolesnyk or @magterskine any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pdohogne-magento @imeron2433 @magterskine
I think in our case when we need to release this next week we can leave it as it is for now. And probably do the refactoring when the story about skip metrics attribute for actions will be in progress.