Skip to content

Commit 070dbeb

Browse files
okolesnykokolesnyk
authored
Merge pull request #266 from M2Coach/brief-code-cleanup
General refactor: ext-curl dependency + review of singletones (refactor constructs)
2 parents 1dc684a + fe05fe1 commit 070dbeb

File tree

9 files changed

+107
-78
lines changed

9 files changed

+107
-78
lines changed

‎composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
},
1111
"require": {
1212
"php": "7.0.2|7.0.4|~7.0.6|~7.1.0|~7.2.0",
13+
"ext-curl": "*",
1314
"allure-framework/allure-codeception": "~1.2.6",
1415
"codeception/codeception": "~2.3.4",
1516
"consolidation/robo": "^1.0.0",

‎dev/tests/unit/Magento/FunctionalTestFramework/Suite/Handlers/SuiteObjectHandlerTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private function setMockTestAndSuiteParserOutput($testData, $suiteData)
8585
$property->setValue(null);
8686

8787
// clear suite object handler value to inject parsed content
88-
$property = new \ReflectionProperty(SuiteObjectHandler::class, 'SUITE_OBJECT_HANLDER_INSTANCE');
88+
$property = new \ReflectionProperty(SuiteObjectHandler::class, 'instance');
8989
$property->setAccessible(true);
9090
$property->setValue(null);
9191

‎dev/tests/unit/Magento/FunctionalTestFramework/Suite/SuiteGeneratorTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public function testGenerateEmptySuite()
149149
*/
150150
private function setMockTestAndSuiteParserOutput($testData, $suiteData)
151151
{
152-
$property = new \ReflectionProperty(SuiteGenerator::class, 'SUITE_GENERATOR_INSTANCE');
152+
$property = new \ReflectionProperty(SuiteGenerator::class, 'instance');
153153
$property->setAccessible(true);
154154
$property->setValue(null);
155155

@@ -159,7 +159,7 @@ private function setMockTestAndSuiteParserOutput($testData, $suiteData)
159159
$property->setValue(null);
160160

161161
// clear suite object handler value to inject parsed content
162-
$property = new \ReflectionProperty(SuiteObjectHandler::class, 'SUITE_OBJECT_HANLDER_INSTANCE');
162+
$property = new \ReflectionProperty(SuiteObjectHandler::class, 'instance');
163163
$property->setAccessible(true);
164164
$property->setValue(null);
165165

‎dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ObjectExtensionUtilTest.php

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace tests\unit\Magento\FunctionalTestFramework\Test\Util;
78

89
use AspectMock\Proxy\Verifier;
@@ -31,26 +32,35 @@ public function setUp()
3132
TestLoggingUtil::getInstance()->setMockLoggingUtil();
3233
}
3334

35+
/**
36+
* After class functionality
37+
* @return void
38+
*/
39+
public static function tearDownAfterClass()
40+
{
41+
TestLoggingUtil::getInstance()->clearMockLoggingUtil();
42+
}
43+
3444
/**
3545
* Tests generating a test that extends another test
3646
* @throws \Exception
3747
*/
3848
public function testGenerateExtendedTest()
3949
{
4050
$mockActions = [
41-
"mockStep" => ["nodeName" => "mockNode", "stepKey" => "mockStep"]
51+
"mockStep" => ["nodeName" => "mockNode", "stepKey" => "mockStep"]
4252
];
4353

4454
$testDataArrayBuilder = new TestDataArrayBuilder();
4555
$mockSimpleTest = $testDataArrayBuilder
4656
->withName('simpleTest')
47-
->withAnnotations(['title'=>[['value' => 'simpleTest']]])
57+
->withAnnotations(['title' => [['value' => 'simpleTest']]])
4858
->withTestActions($mockActions)
4959
->build();
5060

5161
$mockExtendedTest = $testDataArrayBuilder
5262
->withName('extendedTest')
53-
->withAnnotations(['title'=>[['value' => 'extendedTest']]])
63+
->withAnnotations(['title' => [['value' => 'extendedTest']]])
5464
->withTestReference("simpleTest")
5565
->build();
5666

@@ -88,14 +98,14 @@ public function testGenerateExtendedWithHooks()
8898
$testDataArrayBuilder = new TestDataArrayBuilder();
8999
$mockSimpleTest = $testDataArrayBuilder
90100
->withName('simpleTest')
91-
->withAnnotations(['title'=>[['value' => 'simpleTest']]])
101+
->withAnnotations(['title' => [['value' => 'simpleTest']]])
92102
->withBeforeHook($mockBeforeHooks)
93103
->withAfterHook($mockAfterHooks)
94104
->build();
95105

96106
$mockExtendedTest = $testDataArrayBuilder
97107
->withName('extendedTest')
98-
->withAnnotations(['title'=>[['value' => 'extendedTest']]])
108+
->withAnnotations(['title' => [['value' => 'extendedTest']]])
99109
->withTestReference("simpleTest")
100110
->build();
101111

@@ -117,7 +127,7 @@ public function testGenerateExtendedWithHooks()
117127
$this->assertArrayHasKey("mockStepBefore", $testObject->getHooks()['before']->getActions());
118128
$this->assertArrayHasKey("mockStepAfter", $testObject->getHooks()['after']->getActions());
119129
}
120-
130+
121131
/**
122132
* Tests generating a test that extends another test
123133
* @throws \Exception
@@ -158,14 +168,14 @@ public function testExtendingExtendedTest()
158168

159169
$mockSimpleTest = $testDataArrayBuilder
160170
->withName('simpleTest')
161-
->withAnnotations(['title'=>[['value' => 'simpleTest']]])
171+
->withAnnotations(['title' => [['value' => 'simpleTest']]])
162172
->withTestActions()
163173
->withTestReference("anotherTest")
164174
->build();
165175

166176
$mockExtendedTest = $testDataArrayBuilder
167177
->withName('extendedTest')
168-
->withAnnotations(['title'=>[['value' => 'extendedTest']]])
178+
->withAnnotations(['title' => [['value' => 'extendedTest']]])
169179
->withTestReference("simpleTest")
170180
->build();
171181

@@ -347,7 +357,7 @@ private function setMockTestOutput($testData = null, $actionGroupData = null)
347357
$property->setValue(null);
348358

349359
// clear test object handler value to inject parsed content
350-
$property = new \ReflectionProperty(ActionGroupObjectHandler::class, 'ACTION_GROUP_OBJECT_HANDLER');
360+
$property = new \ReflectionProperty(ActionGroupObjectHandler::class, 'instance');
351361
$property->setAccessible(true);
352362
$property->setValue(null);
353363

@@ -358,28 +368,21 @@ private function setMockTestOutput($testData = null, $actionGroupData = null)
358368
)->make();
359369
$instance = AspectMock::double(
360370
ObjectManager::class,
361-
['create' => function ($clazz) use (
362-
$mockDataParser,
363-
$mockActionGroupParser
364-
) {
365-
if ($clazz == TestDataParser::class) {
366-
return $mockDataParser;
371+
[
372+
'create' => function ($className) use (
373+
$mockDataParser,
374+
$mockActionGroupParser
375+
) {
376+
if ($className == TestDataParser::class) {
377+
return $mockDataParser;
378+
}
379+
if ($className == ActionGroupDataParser::class) {
380+
return $mockActionGroupParser;
381+
}
367382
}
368-
if ($clazz == ActionGroupDataParser::class) {
369-
return $mockActionGroupParser;
370-
}
371-
}]
383+
]
372384
)->make();
373385
// bypass the private constructor
374386
AspectMock::double(ObjectManagerFactory::class, ['getObjectManager' => $instance]);
375387
}
376-
377-
/**
378-
* After class functionality
379-
* @return void
380-
*/
381-
public static function tearDownAfterClass()
382-
{
383-
TestLoggingUtil::getInstance()->clearMockLoggingUtil();
384-
}
385388
}

‎dev/tests/unit/Util/TestLoggingUtil.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class TestLoggingUtil extends Assert
1818
/**
1919
* @var TestLoggingUtil
2020
*/
21-
private static $INSTANCE;
21+
private static $instance;
2222

2323
/**
2424
* @var TestHandler
@@ -40,11 +40,11 @@ private function __construct()
4040
*/
4141
public static function getInstance()
4242
{
43-
if (self::$INSTANCE == null) {
44-
self::$INSTANCE = new TestLoggingUtil();
43+
if (self::$instance == null) {
44+
self::$instance = new TestLoggingUtil();
4545
}
4646

47-
return self::$INSTANCE;
47+
return self::$instance;
4848
}
4949

5050
/**
@@ -61,7 +61,7 @@ public function setMockLoggingUtil()
6161
LoggingUtil::class,
6262
['getLogger' => $testLogger]
6363
)->make();
64-
$property = new \ReflectionProperty(LoggingUtil::class, 'INSTANCE');
64+
$property = new \ReflectionProperty(LoggingUtil::class, 'instance');
6565
$property->setAccessible(true);
6666
$property->setValue($mockLoggingUtil);
6767
}

‎src/Magento/FunctionalTestingFramework/Suite/Handlers/SuiteObjectHandler.php

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class SuiteObjectHandler implements ObjectHandlerInterface
2323
*
2424
* @var SuiteObjectHandler
2525
*/
26-
private static $SUITE_OBJECT_HANLDER_INSTANCE;
26+
private static $instance;
2727

2828
/**
2929
* Array of suite objects keyed by suite name.
@@ -33,11 +33,19 @@ class SuiteObjectHandler implements ObjectHandlerInterface
3333
private $suiteObjects;
3434

3535
/**
36-
* SuiteObjectHandler constructor.
36+
* Avoids instantiation of SuiteObjectHandler by new.
37+
* @return void
3738
*/
3839
private function __construct()
3940
{
40-
// empty constructor
41+
}
42+
43+
/**
44+
* Avoids instantiation of SuiteObjectHandler by clone.
45+
* @return void
46+
*/
47+
private function __clone()
48+
{
4149
}
4250

4351
/**
@@ -46,14 +54,14 @@ private function __construct()
4654
* @return ObjectHandlerInterface
4755
* @throws XmlException
4856
*/
49-
public static function getInstance()
57+
public static function getInstance(): ObjectHandlerInterface
5058
{
51-
if (self::$SUITE_OBJECT_HANLDER_INSTANCE == null) {
52-
self::$SUITE_OBJECT_HANLDER_INSTANCE = new SuiteObjectHandler();
53-
self::$SUITE_OBJECT_HANLDER_INSTANCE->initSuiteData();
59+
if (self::$instance == null) {
60+
self::$instance = new SuiteObjectHandler();
61+
self::$instance->initSuiteData();
5462
}
5563

56-
return self::$SUITE_OBJECT_HANLDER_INSTANCE;
64+
return self::$instance;
5765
}
5866

5967
/**
@@ -62,7 +70,7 @@ public static function getInstance()
6270
* @param string $objectName
6371
* @return SuiteObject
6472
*/
65-
public function getObject($objectName)
73+
public function getObject($objectName): SuiteObject
6674
{
6775
if (!array_key_exists($objectName, $this->suiteObjects)) {
6876
trigger_error("Suite ${objectName} is not defined.", E_USER_ERROR);
@@ -75,7 +83,7 @@ public function getObject($objectName)
7583
*
7684
* @return array
7785
*/
78-
public function getAllObjects()
86+
public function getAllObjects(): array
7987
{
8088
return $this->suiteObjects;
8189
}
@@ -85,7 +93,7 @@ public function getAllObjects()
8593
*
8694
* @return array
8795
*/
88-
public function getAllTestReferences()
96+
public function getAllTestReferences(): array
8997
{
9098
$testsReferencedInSuites = [];
9199
$suites = $this->getAllObjects();

‎src/Magento/FunctionalTestingFramework/Suite/SuiteGenerator.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class SuiteGenerator
3333
*
3434
* @var SuiteGenerator
3535
*/
36-
private static $SUITE_GENERATOR_INSTANCE;
36+
private static $instance;
3737

3838
/**
3939
* Group Class Generator initialized in constructor.
@@ -43,28 +43,37 @@ class SuiteGenerator
4343
private $groupClassGenerator;
4444

4545
/**
46-
* SuiteGenerator constructor.
46+
* Avoids instantiation of LoggingUtil by new.
47+
* @return void
4748
*/
4849
private function __construct()
4950
{
5051
$this->groupClassGenerator = new GroupClassGenerator();
5152
}
5253

54+
/**
55+
* Avoids instantiation of SuiteGenerator by clone.
56+
* @return void
57+
*/
58+
private function __clone()
59+
{
60+
}
61+
5362
/**
5463
* Singleton method which is used to retrieve the instance of the suite generator.
5564
*
5665
* @return SuiteGenerator
5766
*/
58-
public static function getInstance()
67+
public static function getInstance(): SuiteGenerator
5968
{
60-
if (!self::$SUITE_GENERATOR_INSTANCE) {
69+
if (!self::$instance) {
6170
// clear any previous configurations before any generation occurs.
6271
self::clearPreviousGroupPreconditions();
6372
self::clearPreviousSessionConfigEntries();
64-
self::$SUITE_GENERATOR_INSTANCE = new SuiteGenerator();
73+
self::$instance = new SuiteGenerator();
6574
}
6675

67-
return self::$SUITE_GENERATOR_INSTANCE;
76+
return self::$instance;
6877
}
6978

7079
/**

‎src/Magento/FunctionalTestingFramework/Test/Handlers/ActionGroupObjectHandler.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class ActionGroupObjectHandler implements ObjectHandlerInterface
2727
*
2828
* @var ActionGroupObjectHandler
2929
*/
30-
private static $ACTION_GROUP_OBJECT_HANDLER;
30+
private static $instance;
3131

3232
/**
3333
* Array of action groups indexed by name
@@ -48,14 +48,13 @@ class ActionGroupObjectHandler implements ObjectHandlerInterface
4848
*
4949
* @return ActionGroupObjectHandler
5050
*/
51-
public static function getInstance()
51+
public static function getInstance(): ActionGroupObjectHandler
5252
{
53-
if (!self::$ACTION_GROUP_OBJECT_HANDLER) {
54-
self::$ACTION_GROUP_OBJECT_HANDLER = new ActionGroupObjectHandler();
55-
self::$ACTION_GROUP_OBJECT_HANDLER->initActionGroups();
53+
if (!self::$instance) {
54+
self::$instance = new ActionGroupObjectHandler();
5655
}
5756

58-
return self::$ACTION_GROUP_OBJECT_HANDLER;
57+
return self::$instance;
5958
}
6059

6160
/**
@@ -64,6 +63,7 @@ public static function getInstance()
6463
private function __construct()
6564
{
6665
$this->extendUtil = new ObjectExtensionUtil();
66+
$this->initActionGroups();
6767
}
6868

6969
/**
@@ -87,7 +87,7 @@ public function getObject($actionGroupName)
8787
*
8888
* @return array
8989
*/
90-
public function getAllObjects()
90+
public function getAllObjects(): array
9191
{
9292
foreach ($this->actionGroups as $actionGroupName => $actionGroup) {
9393
$this->actionGroups[$actionGroupName] = $this->extendActionGroup($actionGroup);
@@ -125,7 +125,7 @@ private function initActionGroups()
125125
* @param ActionGroupObject $actionGroupObject
126126
* @return ActionGroupObject
127127
*/
128-
private function extendActionGroup($actionGroupObject)
128+
private function extendActionGroup($actionGroupObject): ActionGroupObject
129129
{
130130
if ($actionGroupObject->getParentName() !== null) {
131131
return $this->extendUtil->extendActionGroup($actionGroupObject);

0 commit comments

Comments
 (0)