From 37eb197ad33ea5b8696488ad6cab747ce39443b9 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Fri, 20 Sep 2019 15:11:16 -0500 Subject: [PATCH 1/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods Refactored to keep Cyclomatic complexity <= 10. Added comments to below files to refactor later ** Config/FileResolver/Module.php ** Config/Converter.php ** Config/Dom.php --- .../Allure/Adapter/MagentoAllureAdapter.php | 26 ++- .../Codeception/Subscriber/Console.php | 2 +- .../Config/Converter.php | 1 + .../FunctionalTestingFramework/Config/Dom.php | 1 + .../Config/FileResolver/Module.php | 1 + .../Objects/EntityDataObject.php | 18 +- .../Persist/OperationDataArrayResolver.php | 13 +- .../Extension/PageReadinessExtension.php | 40 +++- .../ObjectManager/Config/Mapper/Dom.php | 66 +++--- .../Factory/Dynamic/Developer.php | 1 - .../StaticCheck/TestDependencyCheck.php | 210 ++++++++++++------ .../Util/DocGenerator.php | 1 - 12 files changed, 257 insertions(+), 123 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php b/src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php index 3bcbf0b26..4bce08d9f 100644 --- a/src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php +++ b/src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php @@ -219,7 +219,6 @@ public function testError(FailEvent $failEvent) * Override of parent method, polls stepStorage for testcase and formats it according to actionGroup nesting. * * @return void - * @SuppressWarnings(PHPMD) */ public function testEnd() { @@ -245,11 +244,7 @@ public function testEnd() $step->setName(str_replace(ActionGroupObject::ACTION_GROUP_CONTEXT_START, '', $step->getName())); $actionGroupStepContainer = $step; - - preg_match(TestGenerator::ACTION_GROUP_STEP_KEY_REGEX, $step->getName(), $matches); - if (!empty($matches['actionGroupStepKey'])) { - $actionGroupStepKey = ucfirst($matches['actionGroupStepKey']); - } + $actionGroupStepKey = $this->retrieveActionGroupStepKey($step); continue; } @@ -289,6 +284,25 @@ function () use ($rootStep, $formattedSteps) { $this->getLifecycle()->fire(new TestCaseFinishedEvent()); } + /** + * Reads action group stepKey from step. + * + * @param Step $step + * @return string|null + */ + private function retrieveActionGroupStepKey($step) + { + $actionGroupStepKey = null; + + preg_match(TestGenerator::ACTION_GROUP_STEP_KEY_REGEX, $step->getName(), $matches); + + if (!empty($matches['actionGroupStepKey'])) { + $actionGroupStepKey = ucfirst($matches['actionGroupStepKey']); + } + + return $actionGroupStepKey; + } + /** * Reading stepKey from file. * diff --git a/src/Magento/FunctionalTestingFramework/Codeception/Subscriber/Console.php b/src/Magento/FunctionalTestingFramework/Codeception/Subscriber/Console.php index 6e61189bc..0ba1a3b2f 100644 --- a/src/Magento/FunctionalTestingFramework/Codeception/Subscriber/Console.php +++ b/src/Magento/FunctionalTestingFramework/Codeception/Subscriber/Console.php @@ -38,7 +38,7 @@ class Console extends \Codeception\Subscriber\Console * @param array $extensionOptions * @param array $options * - * @SuppressWarnings(PHPMD) + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function __construct($extensionOptions = [], $options = []) { diff --git a/src/Magento/FunctionalTestingFramework/Config/Converter.php b/src/Magento/FunctionalTestingFramework/Config/Converter.php index dcc67e5cf..8c805c94e 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Converter.php +++ b/src/Magento/FunctionalTestingFramework/Config/Converter.php @@ -83,6 +83,7 @@ public function convert($source) * @param \DOMNodeList|array $elements * @return array * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @TODO ported magento code to be refactored later */ protected function convertXml($elements) { diff --git a/src/Magento/FunctionalTestingFramework/Config/Dom.php b/src/Magento/FunctionalTestingFramework/Config/Dom.php index 566fbd5b3..1b87dd14f 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Dom.php +++ b/src/Magento/FunctionalTestingFramework/Config/Dom.php @@ -145,6 +145,7 @@ protected function mergeNode(\DOMElement $node, $parentPath) * @return void * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * @TODO Ported magento code to be refactored later */ protected function mergeMatchingNode(\DomElement $node, $parentPath, $matchedNode, $path) { diff --git a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php index 7cff19c87..970a79636 100644 --- a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php +++ b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php @@ -26,6 +26,7 @@ class Module implements FileResolverInterface * Module constructor. * @param ModuleResolver|null $moduleResolver * @SuppressWarnings(PHPMD.UnusedFormalParameter) + * @TODO ported magento code to be refactored later */ public function __construct(ModuleResolver $moduleResolver = null) { diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php index 34edeee1d..03199b860 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php +++ b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php @@ -161,7 +161,6 @@ public function getAllData() * @param integer $uniquenessFormat * @return string|null * @throws TestFrameworkException - * @SuppressWarnings(PHPMD) */ public function getDataByName($name, $uniquenessFormat) { @@ -177,12 +176,25 @@ public function getDataByName($name, $uniquenessFormat) throw new TestFrameworkException($exceptionMessage); } - $name_lower = strtolower($name); - if ($this->data === null) { return null; } + return $this->resolveDataReferences($name, $uniquenessFormat); + } + /** + * Resolves data references in entities while generating static test files. + * + * @param string $name + * @param integer $uniquenessFormat + * @return string|null + * @throws TestFrameworkException + * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + */ + + private function resolveDataReferences($name, $uniquenessFormat) + { + $name_lower = strtolower($name); $dataReferenceResolver = new GenerationDataReferenceResolver(); if (array_key_exists($name_lower, $this->data)) { if (is_array($this->data[$name_lower])) { diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php index 6d1ebc3fc..12ce3ebb4 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php +++ b/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php @@ -66,7 +66,6 @@ public function __construct($dependentEntities = null) * @param boolean $fromArray * @return array * @throws \Exception - * @SuppressWarnings(PHPMD) */ public function resolveOperationDataArray($entityObject, $operationMetadata, $operation, $fromArray = false) { @@ -120,7 +119,19 @@ public function resolveOperationDataArray($entityObject, $operationMetadata, $op ); } } + return $this->resolveRunTimeDataReferences($operationDataArray, $entityObject); + } + /** + * Resolve data references at run time. + * @param array $operationDataArray + * @param EntityDataObject $entityObject + * @return mixed + * @throws TestFrameworkException + * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + */ + private function resolveRunTimeDataReferences($operationDataArray, $entityObject) + { $dataReferenceResolver = new RuntimeDataReferenceResolver(); foreach ($operationDataArray as $key => $operationDataValue) { if (is_array($operationDataValue)) { diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php index 8d49ad86c..8fb1ffe38 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php +++ b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php @@ -106,8 +106,6 @@ public function beforeTest(TestEvent $e) * @param StepEvent $e * @return void * @throws \Exception - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function beforeStep(StepEvent $e) { @@ -117,7 +115,26 @@ public function beforeStep(StepEvent $e) return; } - // Check if page has changed and reset metric tracking if so + $this->resetMetricTracker($step); + + $metrics = $this->readinessMetrics; + + $this->waitForReadiness($metrics); + + /** @var AbstractMetricCheck $metric */ + foreach ($metrics as $metric) { + $metric->finalizeForStep($step); + } + } + + /** + * Check if page has changed, if so reset metric tracking + * + * @param Step $step + * return void + */ + private function resetMetricTracker($step) + { if ($this->pageChanged($step)) { $this->logDebug( 'Page URI changed; resetting readiness metric failure tracking', @@ -131,7 +148,17 @@ public function beforeStep(StepEvent $e) $metric->resetTracker(); } } + } + /** + * Wait for page readiness. + * @param $metrics + * @return void + * @throws \Codeception\Exception\ModuleRequireException + * @throws \Facebook\WebDriver\Exception\NoSuchElementException + */ + private function waitForReadiness($metrics) + { // todo: Implement step parameter to override global timeout configuration if (isset($this->config['timeout'])) { $timeout = intval($this->config['timeout']); @@ -139,8 +166,6 @@ public function beforeStep(StepEvent $e) $timeout = $this->getDriver()->_getConfig()['pageload_timeout']; } - $metrics = $this->readinessMetrics; - try { $this->getDriver()->webDriver->wait($timeout)->until( function () use ($metrics) { @@ -160,11 +185,6 @@ function () use ($metrics) { ); } catch (TimeoutException $exception) { } - - /** @var AbstractMetricCheck $metric */ - foreach ($metrics as $metric) { - $metric->finalizeForStep($step); - } } /** diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php index bf11e0003..599f1565d 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php @@ -48,9 +48,6 @@ public function __construct( * @return array * @throws \Exception * @todo this method has high cyclomatic complexity in order to avoid performance issues - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function convert($config) { @@ -83,33 +80,7 @@ public function convert($config) $typeData['type'] = $attributeType->nodeValue; } } - $typeArguments = []; - /** @var \DOMNode $typeChildNode */ - foreach ($node->childNodes as $typeChildNode) { - if ($typeChildNode->nodeType != XML_ELEMENT_NODE) { - continue; - } - switch ($typeChildNode->nodeName) { - case 'arguments': - /** @var \DOMNode $argumentNode */ - foreach ($typeChildNode->childNodes as $argumentNode) { - if ($argumentNode->nodeType != XML_ELEMENT_NODE) { - continue; - } - $argumentName = $argumentNode->attributes->getNamedItem('name')->nodeValue; - $argumentData = $this->argumentParser->parse($argumentNode); - $typeArguments[$argumentName] = $this->argumentInterpreter->evaluate( - $argumentData - ); - } - break; - default: - throw new \Exception( - "Invalid application config. Unknown node: {$typeChildNode->nodeName}." - ); - } - } - + $typeArguments = $this->parseTypeArguments($node); $typeData['arguments'] = $typeArguments; $output[$typeNodeAttributes->getNamedItem('name')->nodeValue] = $typeData; break; @@ -120,4 +91,39 @@ public function convert($config) return $output; } + + /** Read typeChildNodes and set typeArguments + * @param $node + * @return mixed + * @throws \Exception + */ + private function parseTypeArguments($node) + { + foreach ($node->childNodes as $typeChildNode) { + /** @var \DOMNode $typeChildNode */ + if ($typeChildNode->nodeType != XML_ELEMENT_NODE) { + continue; + } + switch ($typeChildNode->nodeName) { + case 'arguments': + /** @var \DOMNode $argumentNode */ + foreach ($typeChildNode->childNodes as $argumentNode) { + if ($argumentNode->nodeType != XML_ELEMENT_NODE) { + continue; + } + $argumentName = $argumentNode->attributes->getNamedItem('name')->nodeValue; + $argumentData = $this->argumentParser->parse($argumentNode); + $typeArguments[$argumentName] = $this->argumentInterpreter->evaluate( + $argumentData + ); + } + return $typeArguments; + + default: + throw new \Exception( + "Invalid application config. Unknown node: {$typeChildNode->nodeName}." + ); + } + } + } } diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php index dbe6aa497..73c5cc92a 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php @@ -171,7 +171,6 @@ protected function parseArray(&$array) * @return object * @throws \Exception * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPCPD) */ public function create($requestedType, array $arguments = []) diff --git a/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php b/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php index 3abbd40c2..f212cad10 100644 --- a/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php +++ b/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php @@ -24,7 +24,7 @@ /** * Class TestDependencyCheck * @package Magento\FunctionalTestingFramework\StaticCheck - * @SuppressWarnings(PHPMD) + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class TestDependencyCheck implements StaticCheckInterface { @@ -74,13 +74,18 @@ class TestDependencyCheck implements StaticCheckInterface */ private $output; + /** + * Array containing all entities after resolving references. + * @var array + */ + private $allEntities = []; + /** * Checks test dependencies, determined by references in tests versus the dependencies listed in the Magento module * * @param InputInterface $input * @return string * @throws Exception; - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function execute(InputInterface $input) { @@ -158,7 +163,6 @@ private function findErrorsInFileSet($files) } $contents = file_get_contents($filePath); - $allEntities = []; preg_match_all(ActionObject::ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN, $contents, $braceReferences); preg_match_all(self::ACTIONGROUP_REGEX_PATTERN, $contents, $actionGroupReferences); preg_match_all(self::EXTENDS_REGEX_PATTERN, $contents, $extendReferences); @@ -169,10 +173,52 @@ private function findErrorsInFileSet($files) $braceReferences[1] = array_unique($braceReferences[1]); $braceReferences[2] = array_filter(array_unique($braceReferences[2])); - // Check `data` entities in {{data.field}} or {{data.field('param')}} - foreach ($braceReferences[0] as $reference) { - // trim `{{data.field}}` to `data` - preg_match('/{{([^.]+)/', $reference, $entityName); + // resolve data entity references + $this->resolveDataEntityReferences($braceReferences[0], $contents); + + //resolve entity references + $this->resolveParametrizedReferences($braceReferences[2], $contents); + + // Check actionGroup references + $this->resolveEntityReferences($actionGroupReferences[1]); + + // Check extended objects + $this->resolveEntityReferences($extendReferences[1]); + + // Find violating references and set error output + $violatingReferences = $this->findViolatingReferences($filePath); + $testErrors = $this->setErrorOutput($violatingReferences, $filePath); + } + return $testErrors; + } + + /** + * Drill down into params in {{ref.params('string', $data.key$, entity.reference)}} + * and resolve references. + * + * @param array $braceReferences + * @param string $contents + * @return void + * @throws XmlException + */ + private function resolveParametrizedReferences($braceReferences, $contents) + { + foreach ($braceReferences as $parameterizedReference) { + preg_match( + ActionObject::ACTION_ATTRIBUTE_VARIABLE_REGEX_PARAMETER, + $parameterizedReference, + $arguments + ); + $splitArguments = explode(',', ltrim(rtrim($arguments[0], ")"), "(")); + foreach ($splitArguments as $argument) { + // Do nothing for 'string' or $persisted.data$ + if (preg_match(ActionObject::STRING_PARAMETER_REGEX, $argument)) { + continue; + } elseif (preg_match(TestGenerator::PERSISTED_OBJECT_NOTATION_REGEX, $argument)) { + continue; + } + // trim `data.field` to `data` + preg_match('/([^.]+)/', $argument, $entityName); // Double check that {{data.field}} isn't an argument for an ActionGroup $entity = $this->findEntity($entityName[1]); preg_match_all(self::ACTIONGROUP_ARGUMENT_REGEX_PATTERN, $contents, $possibleArgument); @@ -180,81 +226,105 @@ private function findErrorsInFileSet($files) continue; } if ($entity !== null) { - $allEntities[$entity->getName()] = $entity; + $this->allEntities[$entity->getName()] = $entity; } } + } + } - // Drill down into params in {{ref.params('string', $data.key$, entity.reference)}} - foreach ($braceReferences[2] as $parameterizedReference) { - preg_match( - ActionObject::ACTION_ATTRIBUTE_VARIABLE_REGEX_PARAMETER, - $parameterizedReference, - $arguments - ); - $splitArguments = explode(',', ltrim(rtrim($arguments[0], ")"), "(")); - foreach ($splitArguments as $argument) { - // Do nothing for 'string' or $persisted.data$ - if (preg_match(ActionObject::STRING_PARAMETER_REGEX, $argument)) { - continue; - } elseif (preg_match(TestGenerator::PERSISTED_OBJECT_NOTATION_REGEX, $argument)) { - continue; - } - // trim `data.field` to `data` - preg_match('/([^.]+)/', $argument, $entityName); - // Double check that {{data.field}} isn't an argument for an ActionGroup - $entity = $this->findEntity($entityName[1]); - preg_match_all(self::ACTIONGROUP_ARGUMENT_REGEX_PATTERN, $contents, $possibleArgument); - if (array_search($entityName[1], $possibleArgument[1]) !== false) { - continue; - } - if ($entity !== null) { - $allEntities[$entity->getName()] = $entity; - } - } + /** + * Check `data` entities in {{data.field}} or {{data.field('param')}} and resolve references + * + * @param array $braceReferences + * @param string $contents + * @return void + * @throws XmlException + + */ + private function resolveDataEntityReferences($braceReferences, $contents) + { + foreach ($braceReferences as $reference) { + // trim `{{data.field}}` to `data` + preg_match('/{{([^.]+)/', $reference, $entityName); + // Double check that {{data.field}} isn't an argument for an ActionGroup + $entity = $this->findEntity($entityName[1]); + preg_match_all(self::ACTIONGROUP_ARGUMENT_REGEX_PATTERN, $contents, $possibleArgument); + if (array_search($entityName[1], $possibleArgument[1]) !== false) { + continue; } - // Check actionGroup references - foreach ($actionGroupReferences[1] as $reference) { - $entity = $this->findEntity($reference); - if ($entity !== null) { - $allEntities[$entity->getName()] = $entity; - } + if ($entity !== null) { + $this->allEntities[$entity->getName()] = $entity; } - // Check extended objects - foreach ($extendReferences[1] as $reference) { - $entity = $this->findEntity($reference); - if ($entity !== null) { - $allEntities[$entity->getName()] = $entity; - } + } + } + + /** + * Resolve entity references + * + * @param array $references + * @return void + * @throws XmlException + */ + private function resolveEntityReferences($references) + { + foreach ($references as $reference) { + $entity = $this->findEntity($reference); + if ($entity !== null) { + $this->allEntities[$entity->getName()] = $entity; } + } + } - $currentModule = $this->moduleNameToComposerName[$moduleFullName]; - $modulesReferencedInTest = $this->getModuleDependenciesFromReferences($allEntities, $currentModule); - $moduleDependencies = $this->flattenedDependencies[$moduleFullName]; - // Find Violations - $violatingReferences = []; - foreach ($modulesReferencedInTest as $entityName => $files) { - $valid = false; - foreach ($files as $module) { - if (array_key_exists($module, $moduleDependencies) || $module == $currentModule) { - $valid = true; - break; - } - } - if (!$valid) { - $violatingReferences[$entityName] = $files; + /** + * Find violating references + * + * @param string $moduleName + * @return array + */ + private function findViolatingReferences($moduleName) + { + // Find Violations + $violatingReferences = []; + $currentModule = $this->moduleNameToComposerName[$moduleName]; + $modulesReferencedInTest = $this->getModuleDependenciesFromReferences($this->allEntities); + $moduleDependencies = $this->flattenedDependencies[$moduleName]; + foreach ($modulesReferencedInTest as $entityName => $files) { + $valid = false; + foreach ($files as $module) { + if (array_key_exists($module, $moduleDependencies) || $module == $currentModule) { + $valid = true; + break; } } + if (!$valid) { + $violatingReferences[$entityName] = $files; + } + } - if (!empty($violatingReferences)) { - // Build error output - $errorOutput = "\nFile \"{$filePath->getRealPath()}\""; - $errorOutput .= "\ncontains entity references that violate dependency constraints:\n\t\t"; - foreach ($violatingReferences as $entityName => $files) { - $errorOutput .= "\n\t {$entityName} from module(s): " . implode(", ", $files); - } - $testErrors[$filePath->getRealPath()][] = $errorOutput; + return $violatingReferences; + } + + /** + * Builds and returns error output for violating references + * + * @param array $violatingReferences + * @param string $path + * @return mixed + */ + private function setErrorOutput($violatingReferences, $path) + { + $testErrors = []; + + if (!empty($violatingReferences)) { + // Build error output + $errorOutput = "\nFile \"{$path->getRealPath()}\""; + $errorOutput .= "\ncontains entity references that violate dependency constraints:\n\t\t"; + foreach ($violatingReferences as $entityName => $files) { + $errorOutput .= "\n\t {$entityName} from module(s): " . implode(", ", $files); } + $testErrors[$path->getRealPath()][] = $errorOutput; } + return $testErrors; } diff --git a/src/Magento/FunctionalTestingFramework/Util/DocGenerator.php b/src/Magento/FunctionalTestingFramework/Util/DocGenerator.php index 2642a715f..7a28f8969 100644 --- a/src/Magento/FunctionalTestingFramework/Util/DocGenerator.php +++ b/src/Magento/FunctionalTestingFramework/Util/DocGenerator.php @@ -15,7 +15,6 @@ /** * Class TestGenerator - * @SuppressWarnings(PHPMD) */ class DocGenerator { From 15668f2bf5d3e1f326ce6f99dbc1f30fdd65f6c3 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Mon, 23 Sep 2019 09:11:25 -0500 Subject: [PATCH 2/9] MQE-610: [PHPMD] Reduce Cyclomatic Complexity in Problem Methods fixing unit tests --- .../DataGenerator/Objects/EntityDataObject.php | 1 - .../Extension/PageReadinessExtension.php | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php index 03199b860..040016f28 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php +++ b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php @@ -191,7 +191,6 @@ public function getDataByName($name, $uniquenessFormat) * @throws TestFrameworkException * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException */ - private function resolveDataReferences($name, $uniquenessFormat) { $name_lower = strtolower($name); diff --git a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php index 8fb1ffe38..2dfcbec1a 100644 --- a/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php +++ b/src/Magento/FunctionalTestingFramework/Extension/PageReadinessExtension.php @@ -131,7 +131,7 @@ public function beforeStep(StepEvent $e) * Check if page has changed, if so reset metric tracking * * @param Step $step - * return void + * @return void */ private function resetMetricTracker($step) { @@ -152,7 +152,7 @@ private function resetMetricTracker($step) /** * Wait for page readiness. - * @param $metrics + * @param array $metrics * @return void * @throws \Codeception\Exception\ModuleRequireException * @throws \Facebook\WebDriver\Exception\NoSuchElementException From fa96ab1ba4d2bc10631233ec19aa97b0039be463 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Mon, 23 Sep 2019 11:01:48 -0500 Subject: [PATCH 3/9] MQE-610: [PHPMD] Reduce Cyclomatic Complexity in Problem Methods Refactored to keep Cyclomatic complexity <= 10. Added comments to functions that are not refactored for readability --- .../Config/Converter/Dom/Flat.php | 1 + .../ObjectManager/Config/Config.php | 57 +++++++++++-------- .../ObjectManager/Factory.php | 5 +- .../Factory/Dynamic/Developer.php | 1 + .../Suite/Util/SuiteObjectExtractor.php | 4 +- .../Test/Config/Converter/Dom/Flat.php | 1 + 6 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Config/Converter/Dom/Flat.php b/src/Magento/FunctionalTestingFramework/Config/Converter/Dom/Flat.php index 4350633e7..001a811d9 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Converter/Dom/Flat.php +++ b/src/Magento/FunctionalTestingFramework/Config/Converter/Dom/Flat.php @@ -70,6 +70,7 @@ protected function getNodeAttributes(\DOMNode $node) * @return string|array * @throws \UnexpectedValueException * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ public function convert(\DOMNode $source, $basePath = '') { diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php index c8e6c8292..9f8084b4d 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php @@ -152,7 +152,7 @@ public function getPreference($type) * * @param string $type * @return array - * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ protected function collectConfiguration($type) { @@ -194,7 +194,6 @@ protected function collectConfiguration($type) * * @param array $configuration * @return void - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ protected function mergeConfiguration(array $configuration) { @@ -207,32 +206,44 @@ protected function mergeConfiguration(array $configuration) break; default: - $key = ltrim($key, '\\'); - if (isset($curConfig['type'])) { - $this->virtualTypes[$key] = ltrim($curConfig['type'], '\\'); - } - if (isset($curConfig['arguments'])) { - if (!empty($this->mergedArguments)) { - $this->mergedArguments = []; - } - if (isset($this->arguments[$key])) { - $this->arguments[$key] = array_replace($this->arguments[$key], $curConfig['arguments']); - } else { - $this->arguments[$key] = $curConfig['arguments']; - } - } - if (isset($curConfig['shared'])) { - if (!$curConfig['shared']) { - $this->nonShared[$key] = 1; - } else { - unset($this->nonShared[$key]); - } - } + $this->setArguments($key, $curConfig); break; } } } + /** + * Set arguments + * + * @param string $key + * @param array $config + * @return void + */ + private function setArguments($key, $config) + { + $key = ltrim($key, '\\'); + if (isset($config['type'])) { + $this->virtualTypes[$key] = ltrim($config['type'], '\\'); + } + if (isset($config['arguments'])) { + if (!empty($this->mergedArguments)) { + $this->mergedArguments = []; + } + if (isset($this->arguments[$key])) { + $this->arguments[$key] = array_replace($this->arguments[$key], $config['arguments']); + } else { + $this->arguments[$key] = $config['arguments']; + } + } + if (isset($config['shared'])) { + if (!$config['shared']) { + $this->nonShared[$key] = 1; + } else { + unset($this->nonShared[$key]); + } + } + } + /** * Extend configuration * diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory.php index 9b795d58f..c5882d035 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory.php @@ -103,6 +103,7 @@ public function prepareArguments($object, $method, array $arguments = []) * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ protected function resolveArguments($requestedType, array $parameters, array $arguments = []) { @@ -179,8 +180,10 @@ protected function resolveArguments($requestedType, array $parameters, array $ar * * @param array $array * @return void + * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ protected function parseArray(&$array) { @@ -219,8 +222,6 @@ protected function parseArray(&$array) * @param array $arguments * @return object * @throws \Exception - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function create($requestedType, array $arguments = []) { diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php index 73c5cc92a..937be7da6 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Factory/Dynamic/Developer.php @@ -87,6 +87,7 @@ public function setObjectManager(\Magento\FunctionalTestingFramework\ObjectManag * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ protected function resolveArguments($requestedType, array $parameters, array $arguments = []) { diff --git a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php index 642671246..676f37995 100644 --- a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php @@ -40,9 +40,11 @@ public function __construct() * @param array $parsedSuiteData * @return array * @throws XmlException + * @throws \Exception + * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) - * @throws \Exception + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ public function parseSuiteDataIntoObjects($parsedSuiteData) { diff --git a/src/Magento/FunctionalTestingFramework/Test/Config/Converter/Dom/Flat.php b/src/Magento/FunctionalTestingFramework/Test/Config/Converter/Dom/Flat.php index f888518aa..0769f5ebf 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Config/Converter/Dom/Flat.php +++ b/src/Magento/FunctionalTestingFramework/Test/Config/Converter/Dom/Flat.php @@ -70,6 +70,7 @@ public function convert($source) * @return string|array * @throws \UnexpectedValueException * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ public function convertXml(\DOMNode $source, $basePath = '') { From 216015258824b280b244a4aea09d7ba6949b445d Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Mon, 23 Sep 2019 11:20:16 -0500 Subject: [PATCH 4/9] MQE-610: [PHPMD] Reduce Cyclomatic Complexity in Problem Methods Fixed unit tests --- .../FunctionalTestingFramework/ObjectManager/Config/Config.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php index 9f8084b4d..486e6d6a1 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php @@ -152,6 +152,7 @@ public function getPreference($type) * * @param string $type * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ protected function collectConfiguration($type) From 17cdd8b921aae0ce11ce61d050b1d36ad7bfe183 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Mon, 23 Sep 2019 15:13:26 -0500 Subject: [PATCH 5/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods modularized SuiteObjectExtractor.php --- .../Config/Converter.php | 2 +- .../FunctionalTestingFramework/Config/Dom.php | 2 +- .../Config/FileResolver/Module.php | 2 +- .../Console/GenerateSuiteCommand.php | 1 + .../Suite/Util/SuiteObjectExtractor.php | 159 ++++++++++++------ 5 files changed, 108 insertions(+), 58 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Config/Converter.php b/src/Magento/FunctionalTestingFramework/Config/Converter.php index 8c805c94e..cf61805dc 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Converter.php +++ b/src/Magento/FunctionalTestingFramework/Config/Converter.php @@ -83,7 +83,7 @@ public function convert($source) * @param \DOMNodeList|array $elements * @return array * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * @TODO ported magento code to be refactored later + * @TODO ported magento code - to be refactored later */ protected function convertXml($elements) { diff --git a/src/Magento/FunctionalTestingFramework/Config/Dom.php b/src/Magento/FunctionalTestingFramework/Config/Dom.php index 1b87dd14f..6d10811ca 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Dom.php +++ b/src/Magento/FunctionalTestingFramework/Config/Dom.php @@ -145,7 +145,7 @@ protected function mergeNode(\DOMElement $node, $parentPath) * @return void * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) - * @TODO Ported magento code to be refactored later + * @TODO Ported magento code - to be refactored later */ protected function mergeMatchingNode(\DomElement $node, $parentPath, $matchedNode, $path) { diff --git a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php index 970a79636..7d2493f3a 100644 --- a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php +++ b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php @@ -26,7 +26,7 @@ class Module implements FileResolverInterface * Module constructor. * @param ModuleResolver|null $moduleResolver * @SuppressWarnings(PHPMD.UnusedFormalParameter) - * @TODO ported magento code to be refactored later + * @TODO ported magento code - to be refactored later */ public function __construct(ModuleResolver $moduleResolver = null) { diff --git a/src/Magento/FunctionalTestingFramework/Console/GenerateSuiteCommand.php b/src/Magento/FunctionalTestingFramework/Console/GenerateSuiteCommand.php index 7dd3b8cb4..911a4a0cb 100644 --- a/src/Magento/FunctionalTestingFramework/Console/GenerateSuiteCommand.php +++ b/src/Magento/FunctionalTestingFramework/Console/GenerateSuiteCommand.php @@ -7,6 +7,7 @@ namespace Magento\FunctionalTestingFramework\Console; +use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; use Magento\FunctionalTestingFramework\Suite\SuiteGenerator; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; diff --git a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php index 676f37995..f34aac136 100644 --- a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php @@ -26,12 +26,19 @@ class SuiteObjectExtractor extends BaseObjectExtractor const TEST_TAG_NAME = 'test'; const GROUP_TAG_NAME = 'group'; + /** + * TestHookObjectExtractor initialized in constructor. + * + * @var TestHookObjectExtractor + */ + private $testHookObjectExtractor; + /** * SuiteObjectExtractor constructor */ public function __construct() { - // empty constructor + $this->testHookObjectExtractor = new TestHookObjectExtractor(); } /** @@ -42,14 +49,11 @@ public function __construct() * @throws XmlException * @throws \Exception * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) - * Revisited to reduce cyclomatic complexity, left unrefactored for readability */ public function parseSuiteDataIntoObjects($parsedSuiteData) { $suiteObjects = []; - $testHookObjectExtractor = new TestHookObjectExtractor(); // make sure there are suites defined before trying to parse as objects. if (!array_key_exists(self::SUITE_ROOT_TAG, $parsedSuiteData)) { @@ -61,71 +65,27 @@ public function parseSuiteDataIntoObjects($parsedSuiteData) // skip non array items parsed from suite (suite objects will always be arrays) continue; } - - // validate the name used isn't using special char or the "default" reserved name - NameValidationUtil::validateName($parsedSuite[self::NAME], 'Suite'); - if ($parsedSuite[self::NAME] == 'default') { - throw new XmlException("A Suite can not have the name \"default\""); - } - - $suiteHooks = []; - - //Check for collisions between suite name and existing group name - $suiteName = $parsedSuite[self::NAME]; - $testGroupConflicts = TestObjectHandler::getInstance()->getTestsByGroup($suiteName); - if (!empty($testGroupConflicts)) { - $testGroupConflictsFileNames = ""; - foreach ($testGroupConflicts as $test) { - $testGroupConflictsFileNames .= $test->getFilename() . "\n"; - } - $exceptionmessage = "\"Suite names and Group names can not have the same value. \t\n" . - "Suite: \"{$suiteName}\" also exists as a group annotation in: \n{$testGroupConflictsFileNames}"; - throw new XmlException($exceptionmessage); - } + //check for collisions between suite and existing group names + $this->validateSuiteName($parsedSuite); //extract include and exclude references $groupTestsToInclude = $parsedSuite[self::INCLUDE_TAG_NAME] ?? []; $groupTestsToExclude = $parsedSuite[self::EXCLUDE_TAG_NAME] ?? []; - // resolve references as test objects + //resolve references as test objects $includeTests = $this->extractTestObjectsFromSuiteRef($groupTestsToInclude); $excludeTests = $this->extractTestObjectsFromSuiteRef($groupTestsToExclude); // parse any object hooks - if (array_key_exists(TestObjectExtractor::TEST_BEFORE_HOOK, $parsedSuite)) { - $suiteHooks[TestObjectExtractor::TEST_BEFORE_HOOK] = $testHookObjectExtractor->extractHook( - $parsedSuite[self::NAME], - TestObjectExtractor::TEST_BEFORE_HOOK, - $parsedSuite[TestObjectExtractor::TEST_BEFORE_HOOK] - ); - } - if (array_key_exists(TestObjectExtractor::TEST_AFTER_HOOK, $parsedSuite)) { - $suiteHooks[TestObjectExtractor::TEST_AFTER_HOOK] = $testHookObjectExtractor->extractHook( - $parsedSuite[self::NAME], - TestObjectExtractor::TEST_AFTER_HOOK, - $parsedSuite[TestObjectExtractor::TEST_AFTER_HOOK] - ); - } + $suiteHooks = $this->parseObjectHooks($parsedSuite); - if (count($suiteHooks) == 1) { - throw new XmlException(sprintf( - "Suites that contain hooks must contain both a 'before' and an 'after' hook. Suite: \"%s\"", - $parsedSuite[self::NAME] - )); - } - // check if suite hooks are empty/not included and there are no included tests/groups/modules - $noHooks = count($suiteHooks) == 0 || - ( - empty($suiteHooks['before']->getActions()) && - empty($suiteHooks['after']->getActions()) - ); - // if suite body is empty throw error - if ($noHooks && empty($includeTests) && empty($excludeTests)) { + //throw an exception if suite is empty + if ($this->isSuiteEmpty($suiteHooks, $includeTests, $excludeTests)){ throw new XmlException(sprintf( "Suites must not be empty. Suite: \"%s\"", $parsedSuite[self::NAME] )); - } + }; // add all test if include tests is completely empty if (empty($includeTests)) { @@ -144,6 +104,95 @@ public function parseSuiteDataIntoObjects($parsedSuiteData) return $suiteObjects; } + /** + * Throws exception for suite names meeting the below conditions: + * 1. the name used is using special char or the "default" reserved name + * 2. collisions between suite name and existing group name + * + * @param array $parsedSuite + * @return void + * @throws XmlException + */ + private function validateSuiteName($parsedSuite) + { + NameValidationUtil::validateName($parsedSuite[self::NAME], 'Suite'); + if ($parsedSuite[self::NAME] == 'default') { + throw new XmlException("A Suite can not have the name \"default\""); + } + + $suiteName = $parsedSuite[self::NAME]; + $testGroupConflicts = TestObjectHandler::getInstance()->getTestsByGroup($suiteName); + if (!empty($testGroupConflicts)) { + $testGroupConflictsFileNames = ""; + foreach ($testGroupConflicts as $test) { + $testGroupConflictsFileNames .= $test->getFilename() . "\n"; + } + $exceptionmessage = "\"Suite names and Group names can not have the same value. \t\n" . + "Suite: \"{$suiteName}\" also exists as a group annotation in: \n{$testGroupConflictsFileNames}"; + throw new XmlException($exceptionmessage); + } + + } + + /** + * Parse object hooks + * + * @param array $parsedSuite + * @return array + * @throws XmlException + */ + private function parseObjectHooks($parsedSuite) + { + $suiteHooks = []; + + if (array_key_exists(TestObjectExtractor::TEST_BEFORE_HOOK, $parsedSuite)) { + $suiteHooks[TestObjectExtractor::TEST_BEFORE_HOOK] = $this->testHookObjectExtractor->extractHook( + $parsedSuite[self::NAME], + TestObjectExtractor::TEST_BEFORE_HOOK, + $parsedSuite[TestObjectExtractor::TEST_BEFORE_HOOK] + ); + } + if (array_key_exists(TestObjectExtractor::TEST_AFTER_HOOK, $parsedSuite)) { + $suiteHooks[TestObjectExtractor::TEST_AFTER_HOOK] = $this->testHookObjectExtractor->extractHook( + $parsedSuite[self::NAME], + TestObjectExtractor::TEST_AFTER_HOOK, + $parsedSuite[TestObjectExtractor::TEST_AFTER_HOOK] + ); + } + + if (count($suiteHooks) == 1) { + + throw new XmlException(sprintf( + "Suites that contain hooks must contain both a 'before' and an 'after' hook. Suite: \"%s\"", + $parsedSuite[self::NAME] + )); + } + return $suiteHooks; + } + + /** + * Check if suite hooks are empty/not included and there are no included tests/groups/modules + * + * @param array $suiteHooks + * @param array $includeTests + * @param array $excludeTests + * @return bool + */ + private function isSuiteEmpty($suiteHooks, $includeTests, $excludeTests) + { + + $noHooks = count($suiteHooks) == 0 || + ( + empty($suiteHooks['before']->getActions()) && + empty($suiteHooks['after']->getActions()) + ); + + if ($noHooks && empty($includeTests) && empty($excludeTests)) { + return true; + } + return false; + } + /** * Wrapper method for resolving suite reference data, checks type of suite reference and calls corresponding * resolver for each suite reference. From 7fa1e22bfedd77a5afe0efa30f8d4b60dbf8050e Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Mon, 23 Sep 2019 15:54:20 -0500 Subject: [PATCH 6/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods updated with develop made changes to comments of SuiteObjectExtractor.php --- .../Suite/Util/SuiteObjectExtractor.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php index f34aac136..0cf9f0470 100644 --- a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php @@ -65,7 +65,7 @@ public function parseSuiteDataIntoObjects($parsedSuiteData) // skip non array items parsed from suite (suite objects will always be arrays) continue; } - //check for collisions between suite and existing group names + $this->validateSuiteName($parsedSuite); //extract include and exclude references @@ -115,12 +115,14 @@ public function parseSuiteDataIntoObjects($parsedSuiteData) */ private function validateSuiteName($parsedSuite) { + //check if name used is using special char or the "default" reserved name NameValidationUtil::validateName($parsedSuite[self::NAME], 'Suite'); if ($parsedSuite[self::NAME] == 'default') { throw new XmlException("A Suite can not have the name \"default\""); } $suiteName = $parsedSuite[self::NAME]; + //check for collisions between suite and existing group names $testGroupConflicts = TestObjectHandler::getInstance()->getTestsByGroup($suiteName); if (!empty($testGroupConflicts)) { $testGroupConflictsFileNames = ""; From f167834af13bc067f5a590d5ced5d86b5bd0f6d4 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Wed, 25 Sep 2019 11:42:17 -0500 Subject: [PATCH 7/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods fixed unit tests --- .../Suite/Util/SuiteObjectExtractor.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php index 0cf9f0470..157e54023 100644 --- a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php @@ -80,7 +80,7 @@ public function parseSuiteDataIntoObjects($parsedSuiteData) $suiteHooks = $this->parseObjectHooks($parsedSuite); //throw an exception if suite is empty - if ($this->isSuiteEmpty($suiteHooks, $includeTests, $excludeTests)){ + if ($this->isSuiteEmpty($suiteHooks, $includeTests, $excludeTests)) { throw new XmlException(sprintf( "Suites must not be empty. Suite: \"%s\"", $parsedSuite[self::NAME] @@ -133,7 +133,6 @@ private function validateSuiteName($parsedSuite) "Suite: \"{$suiteName}\" also exists as a group annotation in: \n{$testGroupConflictsFileNames}"; throw new XmlException($exceptionmessage); } - } /** @@ -163,7 +162,6 @@ private function parseObjectHooks($parsedSuite) } if (count($suiteHooks) == 1) { - throw new XmlException(sprintf( "Suites that contain hooks must contain both a 'before' and an 'after' hook. Suite: \"%s\"", $parsedSuite[self::NAME] @@ -190,7 +188,7 @@ private function isSuiteEmpty($suiteHooks, $includeTests, $excludeTests) ); if ($noHooks && empty($includeTests) && empty($excludeTests)) { - return true; + return true; } return false; } From 8c9b667d73e959dccfa6c69fd2a62cbf50ad0879 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Wed, 25 Sep 2019 13:07:54 -0500 Subject: [PATCH 8/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods fixed unit tests --- .../Suite/Util/SuiteObjectExtractor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php index 157e54023..7ef3d5e8c 100644 --- a/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Suite/Util/SuiteObjectExtractor.php @@ -176,7 +176,7 @@ private function parseObjectHooks($parsedSuite) * @param array $suiteHooks * @param array $includeTests * @param array $excludeTests - * @return bool + * @return boolean */ private function isSuiteEmpty($suiteHooks, $includeTests, $excludeTests) { From f1e2b378f82b1c25e957470f64355196d14610d3 Mon Sep 17 00:00:00 2001 From: Soumya Unnikrishnan Date: Thu, 26 Sep 2019 12:00:17 -0500 Subject: [PATCH 9/9] MQE-610:[PHPMD] Reduce Cyclomatic Complexity in Problem Methods fixed as per review comments --- .../Config/FileResolver/Module.php | 1 - .../DataGenerator/Objects/EntityDataObject.php | 3 ++- .../Persist/OperationDataArrayResolver.php | 5 +++-- .../ObjectManager/Config/Config.php | 7 +++---- .../ObjectManager/Config/Mapper/Dom.php | 12 +++++++----- .../StaticCheck/TestDependencyCheck.php | 2 +- 6 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php index 7d2493f3a..7cff19c87 100644 --- a/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php +++ b/src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php @@ -26,7 +26,6 @@ class Module implements FileResolverInterface * Module constructor. * @param ModuleResolver|null $moduleResolver * @SuppressWarnings(PHPMD.UnusedFormalParameter) - * @TODO ported magento code - to be refactored later */ public function __construct(ModuleResolver $moduleResolver = null) { diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php index 040016f28..47a966f84 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php +++ b/src/Magento/FunctionalTestingFramework/DataGenerator/Objects/EntityDataObject.php @@ -9,6 +9,7 @@ use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; use Magento\FunctionalTestingFramework\DataGenerator\Util\GenerationDataReferenceResolver; use Magento\FunctionalTestingFramework\Exceptions\TestFrameworkException; +use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException; use Magento\FunctionalTestingFramework\Util\Logger\LoggingUtil; /** @@ -189,7 +190,7 @@ public function getDataByName($name, $uniquenessFormat) * @param integer $uniquenessFormat * @return string|null * @throws TestFrameworkException - * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + * @throws TestReferenceException */ private function resolveDataReferences($name, $uniquenessFormat) { diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php index 12ce3ebb4..b2b4fb0e4 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php +++ b/src/Magento/FunctionalTestingFramework/DataGenerator/Persist/OperationDataArrayResolver.php @@ -13,6 +13,7 @@ use Magento\FunctionalTestingFramework\DataGenerator\Util\OperationElementExtractor; use Magento\FunctionalTestingFramework\DataGenerator\Util\RuntimeDataReferenceResolver; use Magento\FunctionalTestingFramework\Exceptions\TestFrameworkException; +use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException; class OperationDataArrayResolver { @@ -126,9 +127,9 @@ public function resolveOperationDataArray($entityObject, $operationMetadata, $op * Resolve data references at run time. * @param array $operationDataArray * @param EntityDataObject $entityObject - * @return mixed + * @return array * @throws TestFrameworkException - * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + * @throws TestReferenceException */ private function resolveRunTimeDataReferences($operationDataArray, $entityObject) { diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php index 486e6d6a1..3b2c827d5 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Config.php @@ -207,20 +207,19 @@ protected function mergeConfiguration(array $configuration) break; default: - $this->setArguments($key, $curConfig); - break; + $this->setConfiguration($key, $curConfig); } } } /** - * Set arguments + * Set configuration * * @param string $key * @param array $config * @return void */ - private function setArguments($key, $config) + private function setConfiguration($key, $config) { $key = ltrim($key, '\\'); if (isset($config['type'])) { diff --git a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php index 599f1565d..ba7bc64cd 100644 --- a/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php +++ b/src/Magento/FunctionalTestingFramework/ObjectManager/Config/Mapper/Dom.php @@ -80,8 +80,7 @@ public function convert($config) $typeData['type'] = $attributeType->nodeValue; } } - $typeArguments = $this->parseTypeArguments($node); - $typeData['arguments'] = $typeArguments; + $typeData['arguments'] = $this->setTypeArguments($node); $output[$typeNodeAttributes->getNamedItem('name')->nodeValue] = $typeData; break; default: @@ -93,12 +92,14 @@ public function convert($config) } /** Read typeChildNodes and set typeArguments - * @param $node + * @param DOMNode $node * @return mixed * @throws \Exception */ - private function parseTypeArguments($node) + private function setTypeArguments($node) { + $typeArguments = []; + foreach ($node->childNodes as $typeChildNode) { /** @var \DOMNode $typeChildNode */ if ($typeChildNode->nodeType != XML_ELEMENT_NODE) { @@ -117,7 +118,7 @@ private function parseTypeArguments($node) $argumentData ); } - return $typeArguments; + break; default: throw new \Exception( @@ -125,5 +126,6 @@ private function parseTypeArguments($node) ); } } + return $typeArguments; } } diff --git a/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php b/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php index f212cad10..6cfa4e9a8 100644 --- a/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php +++ b/src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php @@ -186,7 +186,7 @@ private function findErrorsInFileSet($files) $this->resolveEntityReferences($extendReferences[1]); // Find violating references and set error output - $violatingReferences = $this->findViolatingReferences($filePath); + $violatingReferences = $this->findViolatingReferences($moduleFullName); $testErrors = $this->setErrorOutput($violatingReferences, $filePath); } return $testErrors;