Skip to content

Commit 1fdb5b9

Browse files
bohdan-harniukbohdan-harniuk
committed
33305: Code refactoring after code review
1 parent 15a240e commit 1fdb5b9

File tree

2 files changed

+76
-90
lines changed

2 files changed

+76
-90
lines changed

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

Lines changed: 65 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Magento\FunctionalTestingFramework\Test\Parsers\TestDataParser;
1616
use PHPUnit\Framework\TestCase;
1717
use ReflectionProperty;
18-
use tests\unit\Util\MockModuleResolverBuilder;
1918
use tests\unit\Util\TestDataArrayBuilder;
2019
use tests\unit\Util\TestLoggingUtil;
2120

@@ -30,8 +29,6 @@ class ObjectExtensionUtilTest extends TestCase
3029
protected function setUp(): void
3130
{
3231
TestLoggingUtil::getInstance()->setMockLoggingUtil();
33-
$resolverMock = new MockModuleResolverBuilder();
34-
$resolverMock->setup();
3532
}
3633

3734
/**
@@ -53,7 +50,7 @@ public static function tearDownAfterClass(): void
5350
public function testGenerateExtendedTest(): void
5451
{
5552
$mockActions = [
56-
"mockStep" => ["nodeName" => "mockNode", "stepKey" => "mockStep"]
53+
'mockStep' => ['nodeName' => 'mockNode', 'stepKey' => 'mockStep']
5754
];
5855

5956
$testDataArrayBuilder = new TestDataArrayBuilder();
@@ -66,7 +63,7 @@ public function testGenerateExtendedTest(): void
6663
$mockExtendedTest = $testDataArrayBuilder
6764
->withName('extendedTest')
6865
->withAnnotations(['title' => [['value' => 'extendedTest']]])
69-
->withTestReference("simpleTest")
66+
->withTestReference('simpleTest')
7067
->build();
7168

7269
$mockTestData = array_merge($mockSimpleTest, $mockExtendedTest);
@@ -83,8 +80,8 @@ public function testGenerateExtendedTest(): void
8380
);
8481

8582
// assert that expected test is generated
86-
$this->assertEquals($testObject->getParentName(), "simpleTest");
87-
$this->assertArrayHasKey("mockStep", $testObject->getOrderedActions());
83+
$this->assertEquals($testObject->getParentName(), 'simpleTest');
84+
$this->assertArrayHasKey('mockStep', $testObject->getOrderedActions());
8885
}
8986

9087
/**
@@ -96,10 +93,10 @@ public function testGenerateExtendedTest(): void
9693
public function testGenerateExtendedWithHooks(): void
9794
{
9895
$mockBeforeHooks = [
99-
"beforeHookAction" => ["nodeName" => "mockNodeBefore", "stepKey" => "mockStepBefore"]
96+
'beforeHookAction' => ['nodeName' => 'mockNodeBefore', 'stepKey' => 'mockStepBefore']
10097
];
10198
$mockAfterHooks = [
102-
"afterHookAction" => ["nodeName" => "mockNodeAfter", "stepKey" => "mockStepAfter"]
99+
'afterHookAction' => ['nodeName' => 'mockNodeAfter', 'stepKey' => 'mockStepAfter']
103100
];
104101

105102
$testDataArrayBuilder = new TestDataArrayBuilder();
@@ -113,7 +110,7 @@ public function testGenerateExtendedWithHooks(): void
113110
$mockExtendedTest = $testDataArrayBuilder
114111
->withName('extendedTest')
115112
->withAnnotations(['title' => [['value' => 'extendedTest']]])
116-
->withTestReference("simpleTest")
113+
->withTestReference('simpleTest')
117114
->build();
118115

119116
$mockTestData = array_merge($mockSimpleTest, $mockExtendedTest);
@@ -130,9 +127,9 @@ public function testGenerateExtendedWithHooks(): void
130127
);
131128

132129
// assert that expected test is generated
133-
$this->assertEquals($testObject->getParentName(), "simpleTest");
134-
$this->assertArrayHasKey("mockStepBefore", $testObject->getHooks()['before']->getActions());
135-
$this->assertArrayHasKey("mockStepAfter", $testObject->getHooks()['after']->getActions());
130+
$this->assertEquals($testObject->getParentName(), 'simpleTest');
131+
$this->assertArrayHasKey('mockStepBefore', $testObject->getHooks()['before']->getActions());
132+
$this->assertArrayHasKey('mockStepAfter', $testObject->getHooks()['after']->getActions());
136133
}
137134

138135
/**
@@ -146,7 +143,7 @@ public function testExtendedTestNoParent(): void
146143
$testDataArrayBuilder = new TestDataArrayBuilder();
147144
$mockExtendedTest = $testDataArrayBuilder
148145
->withName('extendedTest')
149-
->withTestReference("simpleTest")
146+
->withTestReference('simpleTest')
150147
->build();
151148

152149
$mockTestData = array_merge($mockExtendedTest);
@@ -158,7 +155,7 @@ public function testExtendedTestNoParent(): void
158155
// validate log statement
159156
TestLoggingUtil::getInstance()->validateMockLogStatement(
160157
'debug',
161-
"parent test not defined. test will be skipped",
158+
'parent test not defined. test will be skipped',
162159
['parent' => 'simpleTest', 'test' => 'extendedTest']
163160
);
164161
}
@@ -181,30 +178,30 @@ public function testExtendingExtendedTest(): void
181178
->withName('simpleTest')
182179
->withAnnotations(['title' => [['value' => 'simpleTest']]])
183180
->withTestActions()
184-
->withTestReference("anotherTest")
181+
->withTestReference('anotherTest')
185182
->build();
186183

187184
$mockExtendedTest = $testDataArrayBuilder
188185
->withName('extendedTest')
189186
->withAnnotations(['title' => [['value' => 'extendedTest']]])
190-
->withTestReference("simpleTest")
187+
->withTestReference('simpleTest')
191188
->build();
192189

193190
$mockTestData = array_merge($mockParentTest, $mockSimpleTest, $mockExtendedTest);
194191
$this->setMockTestOutput($mockTestData);
195192

196-
$this->expectExceptionMessage("Cannot extend a test that already extends another test. Test: simpleTest");
193+
$this->expectExceptionMessage('Cannot extend a test that already extends another test. Test: simpleTest');
197194

198195
// parse and generate test object with mocked data
199196
TestObjectHandler::getInstance()->getObject('extendedTest');
200197

201198
// validate log statement
202199
TestLoggingUtil::getInstance()->validateMockLogStatement(
203200
'debug',
204-
"parent test not defined. test will be skipped",
201+
'parent test not defined. test will be skipped',
205202
['parent' => 'simpleTest', 'test' => 'extendedTest']
206203
);
207-
$this->expectOutputString("Extending Test: anotherTest => simpleTest" . PHP_EOL);
204+
$this->expectOutputString('Extending Test: anotherTest => simpleTest' . PHP_EOL);
208205
}
209206

210207
/**
@@ -216,30 +213,30 @@ public function testExtendingExtendedTest(): void
216213
public function testGenerateExtendedActionGroup(): void
217214
{
218215
$mockSimpleActionGroup = [
219-
"nodeName" => "actionGroup",
220-
"name" => "mockSimpleActionGroup",
221-
"filename" => "someFile",
222-
"commentHere" => [
223-
"nodeName" => "comment",
224-
"selector" => "selector",
225-
"stepKey" => "commentHere"
216+
'nodeName' => 'actionGroup',
217+
'name' => 'mockSimpleActionGroup',
218+
'filename' => 'someFile',
219+
'commentHere' => [
220+
'nodeName' => 'comment',
221+
'selector' => 'selector',
222+
'stepKey' => 'commentHere'
226223
],
227-
"parentComment" => [
228-
"nodeName" => "comment",
229-
"selector" => "parentSelector",
230-
"stepKey" => "parentComment"
224+
'parentComment' => [
225+
'nodeName' => 'comment',
226+
'selector' => 'parentSelector',
227+
'stepKey' => 'parentComment'
231228
],
232229
];
233230

234231
$mockExtendedActionGroup = [
235-
"nodeName" => "actionGroup",
236-
"name" => "mockExtendedActionGroup",
237-
"filename" => "someFile",
238-
"extends" => "mockSimpleActionGroup",
239-
"commentHere" => [
240-
"nodeName" => "comment",
241-
"selector" => "otherSelector",
242-
"stepKey" => "commentHere"
232+
'nodeName' => 'actionGroup',
233+
'name' => 'mockExtendedActionGroup',
234+
'filename' => 'someFile',
235+
'extends' => 'mockSimpleActionGroup',
236+
'commentHere' => [
237+
'nodeName' => 'comment',
238+
'selector' => 'otherSelector',
239+
'stepKey' => 'commentHere'
243240
],
244241
];
245242

@@ -262,10 +259,10 @@ public function testGenerateExtendedActionGroup(): void
262259
);
263260

264261
// assert that expected test is generated
265-
$this->assertEquals("mockSimpleActionGroup", $actionGroupObject->getParentName());
262+
$this->assertEquals('mockSimpleActionGroup', $actionGroupObject->getParentName());
266263
$actions = $actionGroupObject->getActions();
267-
$this->assertEquals("otherSelector", $actions["commentHere"]->getCustomActionAttributes()["selector"]);
268-
$this->assertEquals("parentSelector", $actions["parentComment"]->getCustomActionAttributes()["selector"]);
264+
$this->assertEquals('otherSelector', $actions['commentHere']->getCustomActionAttributes()['selector']);
265+
$this->assertEquals('parentSelector', $actions['parentComment']->getCustomActionAttributes()['selector']);
269266
}
270267

271268
/**
@@ -277,14 +274,14 @@ public function testGenerateExtendedActionGroup(): void
277274
public function testGenerateExtendedActionGroupNoParent(): void
278275
{
279276
$mockExtendedActionGroup = [
280-
"nodeName" => "actionGroup",
281-
"name" => "mockExtendedActionGroup",
282-
"filename" => "someFile",
283-
"extends" => "mockSimpleActionGroup",
284-
"commentHere" => [
285-
"nodeName" => "comment",
286-
"selector" => "otherSelector",
287-
"stepKey" => "commentHere"
277+
'nodeName' => 'actionGroup',
278+
'name' => 'mockExtendedActionGroup',
279+
'filename' => 'someFile',
280+
'extends' => 'mockSimpleActionGroup',
281+
'commentHere' => [
282+
'nodeName' => 'comment',
283+
'selector' => 'otherSelector',
284+
'stepKey' => 'commentHere'
288285
],
289286
];
290287

@@ -296,7 +293,7 @@ public function testGenerateExtendedActionGroupNoParent(): void
296293
$this->setMockTestOutput(null, $mockActionGroupData);
297294

298295
$this->expectExceptionMessage(
299-
"Parent Action Group mockSimpleActionGroup not defined for Test " . $mockExtendedActionGroup['name']
296+
'Parent Action Group mockSimpleActionGroup not defined for Test ' . $mockExtendedActionGroup['name']
300297
);
301298

302299
// parse and generate test object with mocked data
@@ -312,23 +309,23 @@ public function testGenerateExtendedActionGroupNoParent(): void
312309
public function testExtendingExtendedActionGroup(): void
313310
{
314311
$mockParentActionGroup = [
315-
"nodeName" => "actionGroup",
316-
"name" => "mockParentActionGroup",
317-
"filename" => "someFile"
312+
'nodeName' => 'actionGroup',
313+
'name' => 'mockParentActionGroup',
314+
'filename' => 'someFile'
318315
];
319316

320317
$mockSimpleActionGroup = [
321-
"nodeName" => "actionGroup",
322-
"name" => "mockSimpleActionGroup",
323-
"filename" => "someFile",
324-
"extends" => "mockParentActionGroup",
318+
'nodeName' => 'actionGroup',
319+
'name' => 'mockSimpleActionGroup',
320+
'filename' => 'someFile',
321+
'extends' => 'mockParentActionGroup'
325322
];
326323

327324
$mockExtendedActionGroup = [
328-
"nodeName" => "actionGroup",
329-
"name" => "mockExtendedActionGroup",
330-
"filename" => "someFile",
331-
"extends" => "mockSimpleActionGroup",
325+
'nodeName' => 'actionGroup',
326+
'name' => 'mockExtendedActionGroup',
327+
'filename' => 'someFile',
328+
'extends' => 'mockSimpleActionGroup'
332329
];
333330

334331
$mockActionGroupData = [
@@ -341,22 +338,22 @@ public function testExtendingExtendedActionGroup(): void
341338
$this->setMockTestOutput(null, $mockActionGroupData);
342339

343340
$this->expectExceptionMessage(
344-
"Cannot extend an action group that already extends another action group. " . $mockSimpleActionGroup['name']
341+
'Cannot extend an action group that already extends another action group. ' . $mockSimpleActionGroup['name']
345342
);
346343

347344
// parse and generate test object with mocked data
348345
try {
349346
ActionGroupObjectHandler::getInstance()->getObject('mockExtendedActionGroup');
350-
} catch (Exception $e) {
347+
} catch (Exception $exception) {
351348
// validate log statement
352349
TestLoggingUtil::getInstance()->validateMockLogStatement(
353350
'error',
354-
"Cannot extend an action group that already extends another action group. " .
351+
'Cannot extend an action group that already extends another action group. ' .
355352
$mockSimpleActionGroup['name'],
356353
['parent' => $mockSimpleActionGroup['name'], 'actionGroup' => $mockExtendedActionGroup['name']]
357354
);
358355

359-
throw $e;
356+
throw $exception;
360357
}
361358
}
362359

@@ -379,7 +376,7 @@ public function testExtendedTestSkippedParent(): void
379376
$testDataArrayBuilder->reset();
380377
$mockExtendedTest = $testDataArrayBuilder
381378
->withName('extendTest')
382-
->withTestReference("baseTest")
379+
->withTestReference('baseTest')
383380
->build();
384381

385382
$mockTestData = array_merge($mockParentTest, $mockExtendedTest);
@@ -391,7 +388,7 @@ public function testExtendedTestSkippedParent(): void
391388
// validate log statement
392389
TestLoggingUtil::getInstance()->validateMockLogStatement(
393390
'debug',
394-
"extendTest is skipped due to ParentTestIsSkipped",
391+
'extendTest is skipped due to ParentTestIsSkipped',
395392
[]
396393
);
397394
}

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,48 +3,41 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
declare(strict_types=1);
7-
86
namespace tests\unit\Util;
97

108
use AspectMock\Test as AspectMock;
11-
use Exception;
12-
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
139
use Magento\FunctionalTestingFramework\ObjectManager;
10+
use Magento\FunctionalTestingFramework\ObjectManagerFactory;
1411
use Magento\FunctionalTestingFramework\Util\ModuleResolver;
15-
use ReflectionProperty;
12+
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
1613

1714
class MockModuleResolverBuilder
1815
{
1916
/**
20-
* Default paths for mock ModuleResolver.
17+
* Default paths for mock ModuleResolver
2118
*
2219
* @var array
2320
*/
2421
private $defaultPaths = ['Magento_Module' => '/base/path/some/other/path/Magento/Module'];
2522

2623
/**
27-
* Mock ModuleResolver builder.
28-
*
29-
* @param array|null $paths
24+
* Mock ModuleResolver builder
3025
*
26+
* @param array $paths
3127
* @return void
32-
* @throws Exception
28+
* @throws \Exception
3329
*/
34-
public function setup(array $paths = null): void
30+
public function setup($paths = null)
3531
{
3632
if (empty($paths)) {
3733
$paths = $this->defaultPaths;
3834
}
3935

4036
$mockConfig = AspectMock::double(MftfApplicationConfig::class, ['forceGenerateEnabled' => false]);
4137
$instance = AspectMock::double(ObjectManager::class, ['create' => $mockConfig->make(), 'get' => null])->make();
42-
// clear object manager value to inject expected instance
43-
$property = new ReflectionProperty(ObjectManager::class, 'instance');
44-
$property->setAccessible(true);
45-
$property->setValue($instance);
38+
AspectMock::double(ObjectManagerFactory::class, ['getObjectManager' => $instance]);
4639

47-
$property = new ReflectionProperty(ModuleResolver::class, 'instance');
40+
$property = new \ReflectionProperty(ModuleResolver::class, 'instance');
4841
$property->setAccessible(true);
4942
$property->setValue(null);
5043

@@ -58,14 +51,10 @@ public function setup(array $paths = null): void
5851
);
5952
$instance = AspectMock::double(ObjectManager::class, ['create' => $mockResolver->make(), 'get' => null])
6053
->make();
61-
62-
// clear object manager value to inject expected instance
63-
$property = new ReflectionProperty(ObjectManager::class, 'instance');
64-
$property->setAccessible(true);
65-
$property->setValue($instance);
54+
AspectMock::double(ObjectManagerFactory::class, ['getObjectManager' => $instance]);
6655

6756
$resolver = ModuleResolver::getInstance();
68-
$property = new ReflectionProperty(ModuleResolver::class, 'enabledModuleNameAndPaths');
57+
$property = new \ReflectionProperty(ModuleResolver::class, 'enabledModuleNameAndPaths');
6958
$property->setAccessible(true);
7059
$property->setValue($resolver, $paths);
7160
}

0 commit comments

Comments
 (0)