Skip to content

Commit 17cdd8b

Browse files
soumyausoumyau
committed
MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods
modularized SuiteObjectExtractor.php
1 parent 2160152 commit 17cdd8b

File tree

5 files changed

+108
-58
lines changed

5 files changed

+108
-58
lines changed

‎src/Magento/FunctionalTestingFramework/Config/Converter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function convert($source)
8383
* @param \DOMNodeList|array $elements
8484
* @return array
8585
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
86-
* @TODO ported magento code to be refactored later
86+
* @TODO ported magento code - to be refactored later
8787
*/
8888
protected function convertXml($elements)
8989
{

‎src/Magento/FunctionalTestingFramework/Config/Dom.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ protected function mergeNode(\DOMElement $node, $parentPath)
145145
* @return void
146146
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
147147
* @SuppressWarnings(PHPMD.NPathComplexity)
148-
* @TODO Ported magento code to be refactored later
148+
* @TODO Ported magento code - to be refactored later
149149
*/
150150
protected function mergeMatchingNode(\DomElement $node, $parentPath, $matchedNode, $path)
151151
{

‎src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Module implements FileResolverInterface
2626
* Module constructor.
2727
* @param ModuleResolver|null $moduleResolver
2828
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
29-
* @TODO ported magento code to be refactored later
29+
* @TODO ported magento code - to be refactored later
3030
*/
3131
public function __construct(ModuleResolver $moduleResolver = null)
3232
{

‎src/Magento/FunctionalTestingFramework/Console/GenerateSuiteCommand.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Magento\FunctionalTestingFramework\Console;
99

10+
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
1011
use Magento\FunctionalTestingFramework\Suite\SuiteGenerator;
1112
use Symfony\Component\Console\Input\InputArgument;
1213
use Symfony\Component\Console\Input\InputInterface;

‎src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php‎

Lines changed: 104 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,19 @@ class SuiteObjectExtractor extends BaseObjectExtractor
2626
const TEST_TAG_NAME = 'test';
2727
const GROUP_TAG_NAME = 'group';
2828

29+
/**
30+
* TestHookObjectExtractor initialized in constructor.
31+
*
32+
* @var TestHookObjectExtractor
33+
*/
34+
private $testHookObjectExtractor;
35+
2936
/**
3037
* SuiteObjectExtractor constructor
3138
*/
3239
public function __construct()
3340
{
34-
// empty constructor
41+
$this->testHookObjectExtractor = new TestHookObjectExtractor();
3542
}
3643

3744
/**
@@ -42,14 +49,11 @@ public function __construct()
4249
* @throws XmlException
4350
* @throws \Exception
4451
*
45-
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
4652
* @SuppressWarnings(PHPMD.NPathComplexity)
47-
* Revisited to reduce cyclomatic complexity, left unrefactored for readability
4853
*/
4954
public function parseSuiteDataIntoObjects($parsedSuiteData)
5055
{
5156
$suiteObjects = [];
52-
$testHookObjectExtractor = new TestHookObjectExtractor();
5357

5458
// make sure there are suites defined before trying to parse as objects.
5559
if (!array_key_exists(self::SUITE_ROOT_TAG, $parsedSuiteData)) {
@@ -61,71 +65,27 @@ public function parseSuiteDataIntoObjects($parsedSuiteData)
6165
// skip non array items parsed from suite (suite objects will always be arrays)
6266
continue;
6367
}
64-
65-
// validate the name used isn't using special char or the "default" reserved name
66-
NameValidationUtil::validateName($parsedSuite[self::NAME], 'Suite');
67-
if ($parsedSuite[self::NAME] == 'default') {
68-
throw new XmlException("A Suite can not have the name \"default\"");
69-
}
70-
71-
$suiteHooks = [];
72-
73-
//Check for collisions between suite name and existing group name
74-
$suiteName = $parsedSuite[self::NAME];
75-
$testGroupConflicts = TestObjectHandler::getInstance()->getTestsByGroup($suiteName);
76-
if (!empty($testGroupConflicts)) {
77-
$testGroupConflictsFileNames = "";
78-
foreach ($testGroupConflicts as $test) {
79-
$testGroupConflictsFileNames .= $test->getFilename() . "\n";
80-
}
81-
$exceptionmessage = "\"Suite names and Group names can not have the same value. \t\n" .
82-
"Suite: \"{$suiteName}\" also exists as a group annotation in: \n{$testGroupConflictsFileNames}";
83-
throw new XmlException($exceptionmessage);
84-
}
68+
//check for collisions between suite and existing group names
69+
$this->validateSuiteName($parsedSuite);
8570

8671
//extract include and exclude references
8772
$groupTestsToInclude = $parsedSuite[self::INCLUDE_TAG_NAME] ?? [];
8873
$groupTestsToExclude = $parsedSuite[self::EXCLUDE_TAG_NAME] ?? [];
8974

90-
// resolve references as test objects
75+
//resolve references as test objects
9176
$includeTests = $this->extractTestObjectsFromSuiteRef($groupTestsToInclude);
9277
$excludeTests = $this->extractTestObjectsFromSuiteRef($groupTestsToExclude);
9378

9479
// parse any object hooks
95-
if (array_key_exists(TestObjectExtractor::TEST_BEFORE_HOOK, $parsedSuite)) {
96-
$suiteHooks[TestObjectExtractor::TEST_BEFORE_HOOK] = $testHookObjectExtractor->extractHook(
97-
$parsedSuite[self::NAME],
98-
TestObjectExtractor::TEST_BEFORE_HOOK,
99-
$parsedSuite[TestObjectExtractor::TEST_BEFORE_HOOK]
100-
);
101-
}
102-
if (array_key_exists(TestObjectExtractor::TEST_AFTER_HOOK, $parsedSuite)) {
103-
$suiteHooks[TestObjectExtractor::TEST_AFTER_HOOK] = $testHookObjectExtractor->extractHook(
104-
$parsedSuite[self::NAME],
105-
TestObjectExtractor::TEST_AFTER_HOOK,
106-
$parsedSuite[TestObjectExtractor::TEST_AFTER_HOOK]
107-
);
108-
}
80+
$suiteHooks = $this->parseObjectHooks($parsedSuite);
10981

110-
if (count($suiteHooks) == 1) {
111-
throw new XmlException(sprintf(
112-
"Suites that contain hooks must contain both a 'before' and an 'after' hook. Suite: \"%s\"",
113-
$parsedSuite[self::NAME]
114-
));
115-
}
116-
// check if suite hooks are empty/not included and there are no included tests/groups/modules
117-
$noHooks = count($suiteHooks) == 0 ||
118-
(
119-
empty($suiteHooks['before']->getActions()) &&
120-
empty($suiteHooks['after']->getActions())
121-
);
122-
// if suite body is empty throw error
123-
if ($noHooks && empty($includeTests) && empty($excludeTests)) {
82+
//throw an exception if suite is empty
83+
if ($this->isSuiteEmpty($suiteHooks, $includeTests, $excludeTests)){
12484
throw new XmlException(sprintf(
12585
"Suites must not be empty. Suite: \"%s\"",
12686
$parsedSuite[self::NAME]
12787
));
128-
}
88+
};
12989

13090
// add all test if include tests is completely empty
13191
if (empty($includeTests)) {
@@ -144,6 +104,95 @@ public function parseSuiteDataIntoObjects($parsedSuiteData)
144104
return $suiteObjects;
145105
}
146106

107+
/**
108+
* Throws exception for suite names meeting the below conditions:
109+
* 1. the name used is using special char or the "default" reserved name
110+
* 2. collisions between suite name and existing group name
111+
*
112+
* @param array $parsedSuite
113+
* @return void
114+
* @throws XmlException
115+
*/
116+
private function validateSuiteName($parsedSuite)
117+
{
118+
NameValidationUtil::validateName($parsedSuite[self::NAME], 'Suite');
119+
if ($parsedSuite[self::NAME] == 'default') {
120+
throw new XmlException("A Suite can not have the name \"default\"");
121+
}
122+
123+
$suiteName = $parsedSuite[self::NAME];
124+
$testGroupConflicts = TestObjectHandler::getInstance()->getTestsByGroup($suiteName);
125+
if (!empty($testGroupConflicts)) {
126+
$testGroupConflictsFileNames = "";
127+
foreach ($testGroupConflicts as $test) {
128+
$testGroupConflictsFileNames .= $test->getFilename() . "\n";
129+
}
130+
$exceptionmessage = "\"Suite names and Group names can not have the same value. \t\n" .
131+
"Suite: \"{$suiteName}\" also exists as a group annotation in: \n{$testGroupConflictsFileNames}";
132+
throw new XmlException($exceptionmessage);
133+
}
134+
135+
}
136+
137+
/**
138+
* Parse object hooks
139+
*
140+
* @param array $parsedSuite
141+
* @return array
142+
* @throws XmlException
143+
*/
144+
private function parseObjectHooks($parsedSuite)
145+
{
146+
$suiteHooks = [];
147+
148+
if (array_key_exists(TestObjectExtractor::TEST_BEFORE_HOOK, $parsedSuite)) {
149+
$suiteHooks[TestObjectExtractor::TEST_BEFORE_HOOK] = $this->testHookObjectExtractor->extractHook(
150+
$parsedSuite[self::NAME],
151+
TestObjectExtractor::TEST_BEFORE_HOOK,
152+
$parsedSuite[TestObjectExtractor::TEST_BEFORE_HOOK]
153+
);
154+
}
155+
if (array_key_exists(TestObjectExtractor::TEST_AFTER_HOOK, $parsedSuite)) {
156+
$suiteHooks[TestObjectExtractor::TEST_AFTER_HOOK] = $this->testHookObjectExtractor->extractHook(
157+
$parsedSuite[self::NAME],
158+
TestObjectExtractor::TEST_AFTER_HOOK,
159+
$parsedSuite[TestObjectExtractor::TEST_AFTER_HOOK]
160+
);
161+
}
162+
163+
if (count($suiteHooks) == 1) {
164+
165+
throw new XmlException(sprintf(
166+
"Suites that contain hooks must contain both a 'before' and an 'after' hook. Suite: \"%s\"",
167+
$parsedSuite[self::NAME]
168+
));
169+
}
170+
return $suiteHooks;
171+
}
172+
173+
/**
174+
* Check if suite hooks are empty/not included and there are no included tests/groups/modules
175+
*
176+
* @param array $suiteHooks
177+
* @param array $includeTests
178+
* @param array $excludeTests
179+
* @return bool
180+
*/
181+
private function isSuiteEmpty($suiteHooks, $includeTests, $excludeTests)
182+
{
183+
184+
$noHooks = count($suiteHooks) == 0 ||
185+
(
186+
empty($suiteHooks['before']->getActions()) &&
187+
empty($suiteHooks['after']->getActions())
188+
);
189+
190+
if ($noHooks && empty($includeTests) && empty($excludeTests)) {
191+
return true;
192+
}
193+
return false;
194+
}
195+
147196
/**
148197
* Wrapper method for resolving suite reference data, checks type of suite reference and calls corresponding
149198
* resolver for each suite reference.

0 commit comments

Comments
 (0)