Skip to content

Commit 7c3bd33

Browse files
jilu1jilu1
committed
MQE-1750: unit tests and some code refactor.
1 parent 9f6cffc commit 7c3bd33

File tree

2 files changed

+177
-48
lines changed

2 files changed

+177
-48
lines changed

‎dev/tests/unit/Magento/FunctionalTestFramework/Util/ModuleResolverTest.php‎

Lines changed: 106 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,92 @@ public function testGetComposerInstalledTestModulePathsForPathInvocation()
345345
*
346346
* @throws \Exception
347347
*/
348-
public function testMergeFlipAndFilterModulePaths()
348+
public function testMergeFlipAndFilterModulePathsNoForceGenerate()
349349
{
350350
$this->mockForceGenerate(false);
351+
$this->setMockResolverClass(
352+
false,
353+
null,
354+
null,
355+
null,
356+
null,
357+
null,
358+
null,
359+
null,
360+
null,
361+
[
362+
'composer' . DIRECTORY_SEPARATOR . 'json' . DIRECTORY_SEPARATOR . 'pathA' =>
363+
[
364+
'Magento_ModuleA'
365+
],
366+
'composer' . DIRECTORY_SEPARATOR . 'json' . DIRECTORY_SEPARATOR . 'pathB' =>
367+
[
368+
'Magento_ModuleB'
369+
],
370+
],
371+
[
372+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathD' =>
373+
[
374+
'Magento_ModuleD'
375+
],
376+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathE' =>
377+
[
378+
'Magento_ModuleE'
379+
],
380+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathC' =>
381+
[
382+
'Magento_ModuleC',
383+
'Magento_ModuleB',
384+
],
385+
],
386+
[
387+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'example' => ['Magento_Example'],
388+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'sample' => ['Magento_Sample'],
389+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path1' => ['Magento_Path1'],
390+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path2' => ['Magento_Path2'],
391+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path3' => ['Magento_Path3'],
392+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path4' => ['Magento_Path4'],
393+
]
394+
);
395+
396+
$resolver = ModuleResolver::getInstance();
397+
$this->setMockResolverProperties(
398+
$resolver,
399+
null,
400+
[
401+
0 => 'Magento_Path1',
402+
1 => 'Magento_Path2',
403+
2 => 'Magento_Path4',
404+
3 => 'Magento_Example',
405+
4 => 'Magento_ModuleB',
406+
5 => 'Magento_ModuleD',
407+
6 => 'Magento_Otherexample',
408+
7 => 'Magento_ModuleC',
409+
]
410+
);
411+
$this->assertEquals(
412+
[
413+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path1',
414+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path2',
415+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'path4',
416+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'example',
417+
'composer' . DIRECTORY_SEPARATOR . 'json' . DIRECTORY_SEPARATOR . 'pathB',
418+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathD',
419+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathC',
420+
421+
],
422+
$resolver->getModulesPath()
423+
);
424+
}
425+
426+
/**
427+
* Validate mergeModulePaths() and flipAndFilterModulePathsArray()
428+
*
429+
* @throws \Exception
430+
*/
431+
public function testMergeFlipAndFilterModulePathsForceGenerate()
432+
{
433+
$this->mockForceGenerate(true);
351434
$this->setMockResolverClass(
352435
false,
353436
null,
@@ -381,8 +464,8 @@ public function testMergeFlipAndFilterModulePaths()
381464
],
382465
],
383466
[
384-
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'example' => ['Magento_example'],
385-
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'sample' => ['Magento_sample'],
467+
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'example' => ['Magento_Example'],
468+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'sample' => ['Magento_Sample'],
386469
]
387470
);
388471

@@ -394,16 +477,18 @@ public function testMergeFlipAndFilterModulePaths()
394477
0 => 'Magento_ModuleB',
395478
1 => 'Magento_ModuleC',
396479
2 => 'Magento_ModuleD',
397-
3 => 'Magento_example',
398-
4 => 'Magento_otherexample'
480+
3 => 'Magento_Example',
481+
4 => 'Magento_Otherexample'
399482
]
400483
);
401484
$this->assertEquals(
402485
[
403486
'some' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'example',
487+
'composer' . DIRECTORY_SEPARATOR . 'json' . DIRECTORY_SEPARATOR . 'pathA',
404488
'composer' . DIRECTORY_SEPARATOR . 'json' . DIRECTORY_SEPARATOR . 'pathB',
405489
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathA',
406-
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathB'
490+
'composer' . DIRECTORY_SEPARATOR . 'install' . DIRECTORY_SEPARATOR . 'pathB',
491+
'other' . DIRECTORY_SEPARATOR . 'path' . DIRECTORY_SEPARATOR . 'sample'
407492
],
408493
$resolver->getModulesPath()
409494
);
@@ -508,6 +593,8 @@ public function testApplyCustomModuleMethods()
508593

509594
/**
510595
* Validate blacklisted modules are removed
596+
* Module paths are sorted according to module name in alphabetically ascending order
597+
*
511598
* @throws \Exception
512599
*/
513600
public function testGetModulePathsBlacklist()
@@ -525,10 +612,10 @@ public function testGetModulePathsBlacklist()
525612
[],
526613
[],
527614
[
528-
'vendor' => ['vendor'],
529-
'appCode' => ['appCode'],
530-
'devTests' => ['devTests'],
531-
'thisPath' => ['thisPath']
615+
'thisPath/some/path4' => ['Some_Module4'],
616+
'devTests/Magento/path3' => ['Magento_Module3'],
617+
'appCode/Magento/path2' => ['Magento_Module2'],
618+
'vendor/amazon/path1' => ['Amazon_Module1'],
532619
],
533620
function ($arg) {
534621
return $arg;
@@ -538,15 +625,15 @@ function ($arg) {
538625
}
539626
);
540627
$resolver = ModuleResolver::getInstance();
541-
$this->setMockResolverProperties($resolver, null, null, ['devTests']);
628+
$this->setMockResolverProperties($resolver, null, null, ['Magento_Module3']);
542629
$this->assertEquals(
543-
['vendor', 'appCode', 'thisPath'],
630+
['vendor/amazon/path1', 'appCode/Magento/path2', 'thisPath/some/path4'],
544631
$resolver->getModulesPath()
545632
);
546633
TestLoggingUtil::getInstance()->validateMockLogStatement(
547634
'info',
548635
'excluding module',
549-
['module' => 'devTests']
636+
['module' => 'Magento_Module3']
550637
);
551638
}
552639

@@ -654,6 +741,7 @@ public function testGetAdminTokenWithBadResponse()
654741
* @param string[] $mockAggregateTestModulePaths
655742
* @param string[] $mockNormalizeModuleNames
656743
* @param string[] $mockFlipAndFilterModulePathsArray
744+
* @param string[] $mockFlipAndSortModulePathsArray
657745
* @throws \Exception
658746
* @return Verifier ModuleResolver double
659747
*/
@@ -671,7 +759,8 @@ private function setMockResolverClass(
671759
$mockGetComposerInstalledTestModulePaths = null,
672760
$mockAggregateTestModulePaths = null,
673761
$mockNormalizeModuleNames = null,
674-
$mockFlipAndFilterModulePathsArray = null
762+
$mockFlipAndFilterModulePathsArray = null,
763+
$mockFlipAndSortModulePathsArray = null
675764
) {
676765
$property = new \ReflectionProperty(ModuleResolver::class, 'instance');
677766
$property->setAccessible(true);
@@ -721,7 +810,9 @@ private function setMockResolverClass(
721810
if (isset($mockFlipAndFilterModulePathsArray)) {
722811
$mockMethods['flipAndFilterModulePathsArray'] = $mockFlipAndFilterModulePathsArray;
723812
}
724-
813+
if (isset($mockFlipAndSortModulePathsArray)) {
814+
$mockMethods['flipAndSortModulePathsArray'] = $mockFlipAndSortModulePathsArray;
815+
}
725816
$mockResolver = AspectMock::double(
726817
ModuleResolver::class,
727818
$mockMethods

‎src/Magento/FunctionalTestingFramework/Util/ModuleResolver.php‎

Lines changed: 71 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*
1717
* @api
1818
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
19+
* @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
1920
*/
2021
class ModuleResolver
2122
{
@@ -263,7 +264,7 @@ public function getModulesPath($verbosePath = false)
263264
$allModulePaths = $this->normalizeModuleNames($allModulePaths);
264265

265266
if (MftfApplicationConfig::getConfig()->forceGenerateEnabled()) {
266-
$allModulePaths = $this->flipAndFilterModulePathsArray($allModulePaths);
267+
$allModulePaths = $this->flipAndSortModulePathsArray($allModulePaths, true);
267268
$this->enabledModulePaths = $this->applyCustomModuleMethods($allModulePaths);
268269
return $this->enabledModulePaths;
269270
}
@@ -497,48 +498,85 @@ private function getComposerInstalledTestModulePaths($composerFile)
497498
* @param array $objectArray
498499
* @param array $filterArray
499500
* @return array
500-
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
501501
*/
502-
private function flipAndFilterModulePathsArray($objectArray, $filterArray = null)
502+
private function flipAndFilterModulePathsArray($objectArray, $filterArray)
503503
{
504+
$oneToOneArray = [];
505+
$oneToManyArray = [];
506+
// Filter array by enabled modules
507+
foreach ($objectArray as $path => $modules) {
508+
if (!array_diff($modules, $filterArray)
509+
|| (count($modules) == 1 && isset($this->knownDirectories[$modules[0]]))) {
510+
if (count($modules) == 1) {
511+
$oneToOneArray[$path] = $modules[0];
512+
} else {
513+
$oneToManyArray[$path] = $modules;
514+
}
515+
}
516+
}
517+
504518
$flippedArray = [];
519+
// Set flipped array for "one path => one module" case first to maintain module sequencing
520+
foreach ($filterArray as $moduleName) {
521+
$path = array_search($moduleName, $oneToOneArray);
522+
if ($path !== false) {
523+
if (strpos($moduleName, '_') === false) {
524+
$moduleName = $this->findVendorNameFromPath($path) . '_' . $moduleName;
525+
}
526+
$flippedArray = $this->setArrayValueWithLogging($flippedArray, $moduleName, $path);
527+
unset($oneToOneArray[$path]);
528+
}
529+
}
530+
531+
// Set flipped array for everything else
532+
return $this->flipAndSortModulePathsArray(
533+
array_merge($oneToOneArray, $oneToManyArray),
534+
false,
535+
$flippedArray
536+
);
537+
}
538+
539+
/**
540+
* Flip module code paths and optionally sort in alphabetical order
541+
*
542+
* @param array $objectArray
543+
* @param boolean $sort
544+
* @param array $inFlippedArray
545+
* @return array
546+
*/
547+
private function flipAndSortModulePathsArray($objectArray, $sort, $inFlippedArray = [])
548+
{
549+
$flippedArray = $inFlippedArray;
550+
551+
// Set flipped array from object array
505552
foreach ($objectArray as $path => $modules) {
506-
// One path maps to one module
507-
if (count($modules) == 1) {
508-
if (!is_array($filterArray)
509-
|| (is_array($filterArray) && in_array($modules[0], $filterArray))
510-
|| isset($this->knownDirectories[$modules[0]])) {
511-
if (strpos($modules[0], '_') === false) {
512-
$modules[0] = $this->findVendorNameFromPath($path) . '_' . $modules[0];
513-
}
514-
$flippedArray = $this->setArrayValueWithLogging($flippedArray, $modules[0], $path);
553+
if (is_array($modules) && count($modules) > 1) {
554+
// The "one path => many module names" case is designed to be strictly used when it's
555+
// impossible to write tests in dedicated modules. Due to performance consideration and there
556+
// is no real usage of this currently, we will use the first module name for the path.
557+
// TODO: consider saving all module names if this information is needed in the future.
558+
$module = $modules[0];
559+
} elseif (is_array($modules)) {
560+
if (strpos($modules[0], '_') === false) {
561+
$module = $this->findVendorNameFromPath($path) . '_' . $modules[0];
562+
} else {
563+
$module = $modules[0];
515564
}
516565
} else {
517-
// One path maps to multiple modules
518-
if (!is_array($filterArray)) {
519-
$flippedArray = $this->setArrayValueWithLogging(
520-
$flippedArray,
521-
$this->findVendorAndModuleNameFromPath($path),
522-
$path
523-
);
566+
if (strpos($modules, '_') === false) {
567+
$module = $this->findVendorNameFromPath($path) . '_' . $modules;
524568
} else {
525-
$skip = false;
526-
foreach ($modules as $module) {
527-
if (!in_array($module, $filterArray)) {
528-
$skip = true;
529-
break;
530-
}
531-
}
532-
if (!$skip) {
533-
// The one path to many module names use case is designed to be strictly used when it's
534-
// impossible to write tests in dedicated modules. Due to performance consideration and there
535-
// is no real usage of this currently, we will use the first module name for the path.
536-
// TODO: consider saving all module names if this information is needed in the future.
537-
$flippedArray = $this->setArrayValueWithLogging($flippedArray, $modules[0], $path);
538-
}
569+
$module = $modules;
539570
}
540571
}
572+
$flippedArray = $this->setArrayValueWithLogging($flippedArray, $module, $path);
541573
}
574+
575+
// Sort array in alphabetical order
576+
if ($sort) {
577+
ksort($flippedArray);
578+
}
579+
542580
return $flippedArray;
543581
}
544582

0 commit comments

Comments
 (0)