Skip to content

Commit 8d7cfe8

Browse files
pdohogne-magentopdohogne-magento
committed
MAGETWO-46837: Fixing static test failures in readiness extension
1 parent fadb951 commit 8d7cfe8

File tree

7 files changed

+92
-75
lines changed

7 files changed

+92
-75
lines changed

‎src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class PageReadinessExtension extends Extension
4444
/**
4545
* Logger verbosity
4646
*
47-
* @var bool
47+
* @var boolean
4848
*/
4949
private $verbose;
5050

@@ -91,13 +91,13 @@ public function getDriver()
9191
* @param \Codeception\Event\TestEvent $e
9292
* @return void
9393
*/
94-
public function beforeTest(TestEvent $e) {
94+
public function beforeTest(TestEvent $e)
95+
{
9596
$this->test = $e->getTest();
9697

9798
if (isset($this->config['resetFailureThreshold'])) {
9899
$failThreshold = intval($this->config['resetFailureThreshold']);
99-
}
100-
else {
100+
} else {
101101
$failThreshold = 3;
102102
}
103103

@@ -116,25 +116,22 @@ public function beforeTest(TestEvent $e) {
116116
* @return void
117117
* @throws \Exception
118118
*/
119-
public function beforeStep(StepEvent $e) {
119+
public function beforeStep(StepEvent $e)
120+
{
120121
$step = $e->getStep();
121122
if ($step->getAction() == 'saveScreenshot') {
122123
return;
123124
}
124-
// $step->getArguments()['skipReadiness']
125125

126126
try {
127127
$this->test->getMetadata()->setCurrent(['uri', $this->getDriver()->_getCurrentUri()]);
128+
} catch (\Exception $exception) {
129+
$this->logDebug('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]);
128130
}
129-
catch (\Exception $exception) {
130-
// $this->debugLog('Could not retrieve current URI', ['action' => $e->getStep()->getAction()]);
131-
}
132-
133131

134132
if (isset($this->config['timeout'])) {
135133
$timeout = intval($this->config['timeout']);
136-
}
137-
else {
134+
} else {
138135
$timeout = $this->getDriver()->_getConfig()['pageload_timeout'];
139136
}
140137

@@ -151,14 +148,14 @@ function () use ($metrics) {
151148
if (!$metric->runCheck()) {
152149
$passing = false;
153150
}
151+
} catch (UnexpectedAlertOpenException $exception) {
154152
}
155-
catch (UnexpectedAlertOpenException $exception) {}
156153
}
157154
return $passing;
158155
}
159156
);
157+
} catch (TimeoutException $exception) {
160158
}
161-
catch (TimeoutException $exception) {}
162159

163160
/** @var AbstractMetricCheck $metric */
164161
foreach ($metrics as $metric) {
@@ -172,24 +169,25 @@ function () use ($metrics) {
172169
* @param StepEvent $e
173170
* @return void
174171
*/
175-
public function afterStep(StepEvent $e) {
172+
public function afterStep(StepEvent $e)
173+
{
176174
$step = $e->getStep();
177175
if ($step->getAction() == 'saveScreenshot') {
178176
return;
179177
}
180178

181179
try {
182180
$currentUri = $this->getDriver()->_getCurrentUri();
183-
}
184-
catch (\Exception $e) {
181+
} catch (\Exception $e) {
185182
// $this->debugLog('Could not retrieve current URI', ['action' => $step()->getAction()]);
186183
return;
187184
}
188185

189186
$previousUri = $this->test->getMetadata()->getCurrent('uri');
190187

191188
if ($previousUri !== $currentUri) {
192-
$this->logDebug('Page URI changed; resetting readiness metric failure tracking',
189+
$this->logDebug(
190+
'Page URI changed; resetting readiness metric failure tracking',
193191
[
194192
'action' => $step->getAction(),
195193
'newUri' => $currentUri
@@ -207,9 +205,11 @@ public function afterStep(StepEvent $e) {
207205
* If verbose, log the given message to logger->debug including test context information
208206
*
209207
* @param string $message
210-
* @param array $context
208+
* @param array $context
209+
* @return void
211210
*/
212-
private function logDebug($message, $context = []) {
211+
private function logDebug($message, $context = [])
212+
{
213213
if ($this->verbose) {
214214
$testMeta = $this->test->getMetadata();
215215
$logContext = [

‎src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/AbstractMetricCheck.php‎

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,20 @@ abstract class AbstractMetricCheck
5555
protected $logger;
5656

5757
/**
58-
* @var bool
58+
* @var boolean
5959
*/
6060
protected $verbose;
6161

6262
/**
6363
* Constructor, called from the beforeTest event
6464
*
6565
* @param PageReadinessExtension $extension
66-
* @param TestInterface $test
67-
* @param integer $resetFailureThreshold
66+
* @param TestInterface $test
67+
* @param integer $resetFailureThreshold
6868
* @throws \Exception
6969
*/
70-
public function __construct($extension, $test, $resetFailureThreshold){
70+
public function __construct($extension, $test, $resetFailureThreshold)
71+
{
7172
$this->extension = $extension;
7273
$this->test = $test;
7374
$this->logger = LoggingUtil::getInstance()->getLogger(get_class($this));
@@ -78,8 +79,7 @@ public function __construct($extension, $test, $resetFailureThreshold){
7879
$reflector = new \ReflectionMethod($this, 'clearFailureOnPage');
7980
if ($reflector->getDeclaringClass()->getName() === get_class($this)) {
8081
$this->resetFailureThreshold = $resetFailureThreshold;
81-
}
82-
else {
82+
} else {
8383
$this->resetFailureThreshold = -1;
8484
}
8585

@@ -90,25 +90,26 @@ public function __construct($extension, $test, $resetFailureThreshold){
9090
* Does the given value pass the readiness metric
9191
*
9292
* @param mixed $value
93-
* @return bool
93+
* @return boolean
9494
*/
95-
protected abstract function doesMetricPass($value);
95+
abstract protected function doesMetricPass($value);
9696

9797
/**
9898
* Retrieve the active value for the metric to check from the page
9999
*
100100
* @return mixed
101101
* @throws UnexpectedAlertOpenException
102102
*/
103-
protected abstract function fetchValueFromPage();
103+
abstract protected function fetchValueFromPage();
104104

105105
/**
106106
* Override this method to reset the actual state of the page to make the metric pass
107107
* This method is called when too many identical failures were encountered in a row
108108
*
109109
* @return void
110110
*/
111-
protected function clearFailureOnPage() {
111+
protected function clearFailureOnPage()
112+
{
112113
return;
113114
}
114115

@@ -117,7 +118,8 @@ protected function clearFailureOnPage() {
117118
*
118119
* @return string
119120
*/
120-
public function getName() {
121+
public function getName()
122+
{
121123
$clazz = get_class($this);
122124
$namespaceBreak = strrpos($clazz, '\\');
123125
if ($namespaceBreak !== false) {
@@ -133,10 +135,11 @@ public function getName() {
133135
* to catch cases where a slow request of one metric can trigger calls for other metrics that were previously
134136
* thought ready
135137
*
136-
* @return bool
138+
* @return boolean
137139
* @throws UnexpectedAlertOpenException
138140
*/
139-
public function runCheck() {
141+
public function runCheck()
142+
{
140143
if ($this->doesMetricPass($this->getCurrentValue(true))) {
141144
$this->setTracker($this->getCurrentValue());
142145
return true;
@@ -154,11 +157,11 @@ public function runCheck() {
154157
* @param Step $step
155158
* @return void
156159
*/
157-
public function finalize($step) {
160+
public function finalize($step)
161+
{
158162
try {
159163
$currentValue = $this->getCurrentValue();
160-
}
161-
catch (UnexpectedAlertOpenException $exception) {
164+
} catch (UnexpectedAlertOpenException $exception) {
162165
$this->debugLog(
163166
'An alert is open, bypassing javascript-based metric check',
164167
['action' => $step->getAction()]
@@ -168,13 +171,11 @@ public function finalize($step) {
168171

169172
if ($this->doesMetricPass($currentValue)) {
170173
$this->setTracker($currentValue);
171-
}
172-
else {
174+
} else {
173175
// If failure happened on the same value as before, increment the fail count, otherwise set at 1
174176
if ($currentValue !== $this->getStoredValue()) {
175177
$failCount = 1;
176-
}
177-
else {
178+
} else {
178179
$failCount = $this->getFailureCount() + 1;
179180
}
180181
$this->setTracker($currentValue, $failCount);
@@ -197,32 +198,35 @@ public function finalize($step) {
197198
* @return WebDriver
198199
* @throws ModuleRequireException
199200
*/
200-
protected function getDriver() {
201+
protected function getDriver()
202+
{
201203
return $this->extension->getDriver();
202204
}
203205

204206
/**
205207
* Helper function to execute javascript code, see WebDriver::executeJs for more information
206208
*
207209
* @param string $script
208-
* @param array $arguments
210+
* @param array $arguments
209211
* @return mixed
210212
* @throws UnexpectedAlertOpenException
211213
* @throws ModuleRequireException
212214
*/
213-
protected function executeJs($script, $arguments = []) {
215+
protected function executeJs($script, $arguments = [])
216+
{
214217
return $this->getDriver()->executeJS($script, $arguments);
215218
}
216219

217220
/**
218221
* Gets the current state of the given variable
219222
* Fetches an updated value if not known or $refresh is true
220223
*
221-
* @param bool $refresh
224+
* @param boolean $refresh
222225
* @return mixed
223226
* @throws UnexpectedAlertOpenException
224227
*/
225-
private function getCurrentValue($refresh = false) {
228+
private function getCurrentValue($refresh = false)
229+
{
226230
if ($refresh) {
227231
unset($this->currentValue);
228232
}
@@ -237,17 +241,19 @@ private function getCurrentValue($refresh = false) {
237241
*
238242
* @return mixed
239243
*/
240-
public function getStoredValue() {
244+
public function getStoredValue()
245+
{
241246
return $this->test->getMetadata()->getCurrent($this->getName());
242247
}
243248

244249
/**
245250
* The current count of sequential identical failures
246251
* Used to detect potentially stuck metrics
247252
*
248-
* @return int
253+
* @return integer
249254
*/
250-
public function getFailureCount() {
255+
public function getFailureCount()
256+
{
251257
return $this->test->getMetadata()->getCurrent($this->getName() . '.failCount');
252258
}
253259

@@ -257,33 +263,37 @@ public function getFailureCount() {
257263
*
258264
* @return void
259265
*/
260-
private function resetMetric() {
266+
private function resetMetric()
267+
{
261268
$this->clearFailureOnPage();
262269
$this->setTracker();
263270
}
264271

265272
/**
266273
* Tracks the most recent value and the number of identical failures in a row
267274
*
268-
* @param mixed $value
269-
* @param int $failCount
275+
* @param mixed $value
276+
* @param integer $failCount
270277
* @return void
271278
*/
272-
public function setTracker($value = null, $failCount = 0) {
279+
public function setTracker($value = null, $failCount = 0)
280+
{
273281
$this->test->getMetadata()->setCurrent([
274282
$this->getName() => $value,
275283
$this->getName() . '.failCount' => $failCount
276284
]);
277-
unset ($this->currentValue);
285+
unset($this->currentValue);
278286
}
279287

280288
/**
281289
* Log the given message to logger->error including context information
282290
*
283291
* @param string $message
284-
* @param array $context
292+
* @param array $context
293+
* @return void
285294
*/
286-
protected function errorLog($message, $context = []) {
295+
protected function errorLog($message, $context = [])
296+
{
287297
$context = array_merge($this->getLogContext(), $context);
288298
$this->logger->error($message, $context);
289299
}
@@ -292,9 +302,11 @@ protected function errorLog($message, $context = []) {
292302
* Log the given message to logger->info including context information
293303
*
294304
* @param string $message
295-
* @param array $context
305+
* @param array $context
306+
* @return void
296307
*/
297-
protected function infoLog($message, $context = []) {
308+
protected function infoLog($message, $context = [])
309+
{
298310
$context = array_merge($this->getLogContext(), $context);
299311
$this->logger->info($message, $context);
300312
}
@@ -303,9 +315,11 @@ protected function infoLog($message, $context = []) {
303315
* If verbose, log the given message to logger->debug including context information
304316
*
305317
* @param string $message
306-
* @param array $context
318+
* @param array $context
319+
* @return void
307320
*/
308-
protected function debugLog($message, $context = []) {
321+
protected function debugLog($message, $context = [])
322+
{
309323
if ($this->verbose) {
310324
$context = array_merge($this->getLogContext(), $context);
311325
$this->logger->debug($message, $context);
@@ -318,7 +332,8 @@ protected function debugLog($message, $context = []) {
318332
*
319333
* @return array
320334
*/
321-
private function getLogContext() {
335+
private function getLogContext()
336+
{
322337
$testMeta = $this->test->getMetadata();
323338
return [
324339
'test' => $testMeta->getName(),

‎src/Magento/FunctionalTestingFramework/Extension/ReadinessMetrics/DocumentReadyState.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class DocumentReadyState extends AbstractMetricCheck
2525
* Metric passes when document.readyState == 'complete'
2626
*
2727
* @param string $value
28-
* @return bool
28+
* @return boolean
2929
*/
3030
protected function doesMetricPass($value)
3131
{

0 commit comments

Comments
 (0)