Skip to content

Commit 731a770

Browse files
soumyausoumyau
committed
MQE-1676: Add a static-check that ensures action groups do not have unused arguments
Refactored to read xml content from files directly to avoid flagging arguments due to step removals during merging/extensions.
1 parent d9bea3b commit 731a770

File tree

4 files changed

+94
-102
lines changed

4 files changed

+94
-102
lines changed

‎src/Magento/FunctionalTestingFramework/StaticCheck/ActionGroupArgumentsCheck.php

Lines changed: 68 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
1010
use Magento\FunctionalTestingFramework\Test\Handlers\ActionGroupObjectHandler;
1111
use Magento\FunctionalTestingFramework\Test\Objects\ActionGroupObject;
12-
use Magento\FunctionalTestingFramework\Test\Objects\ActionObject;
1312
use Symfony\Component\Console\Input\InputInterface;
13+
use Magento\FunctionalTestingFramework\Util\ModuleResolver;
14+
use Symfony\Component\Finder\Finder;
1415
use Exception;
1516

1617
/**
@@ -19,6 +20,11 @@
1920
*/
2021
class ActionGroupArgumentsCheck implements StaticCheckInterface
2122
{
23+
24+
const ACTIONGROUP_XML_REGEX_PATTERN = '/<actionGroup\sname=(?: (?!<\/actionGroup>).)*/mxs';
25+
const ACTIONGROUP_ARGUMENT_REGEX_PATTERN = '/<argument[^\/>]*name="([^"\']*)/mxs';
26+
const ACTIONGROUP_NAME_REGEX_PATTERN = '/<actionGroup name=["\']([^\'"]*)/';
27+
2228
const ERROR_LOG_FILENAME = 'mftf-arguments-checks';
2329
const ERROR_LOG_MESSAGE = 'MFTF Unused Arguments Check';
2430

@@ -43,7 +49,6 @@ class ActionGroupArgumentsCheck implements StaticCheckInterface
4349
*/
4450
public function execute(InputInterface $input)
4551
{
46-
4752
MftfApplicationConfig::create(
4853
true,
4954
MftfApplicationConfig::UNIT_TEST_PHASE,
@@ -52,11 +57,14 @@ public function execute(InputInterface $input)
5257
true
5358
);
5459

55-
$actionGroups = ActionGroupObjectHandler::getInstance()->initActionGroups();
60+
$allModules = ModuleResolver::getInstance()->getModulesPath();
5661

57-
$unusedArgumentList = $this->buildUnusedArgumentList($actionGroups);
62+
$actionGroupXmlFiles = StaticCheckHelper::buildFileList(
63+
$allModules,
64+
DIRECTORY_SEPARATOR . 'ActionGroup' . DIRECTORY_SEPARATOR
65+
);
5866

59-
$this->errors += $this->setErrorOutput($unusedArgumentList);
67+
$this->errors = $this->findErrorsInFileSet($actionGroupXmlFiles);
6068

6169
$this->output = StaticCheckHelper::printErrorsToFile(
6270
$this->errors,
@@ -84,54 +92,73 @@ public function getOutput()
8492
}
8593

8694
/**
87-
* Builds array of action groups => unused arguments
95+
* Finds all unused arguments in given set of actionGroup files
96+
* @param Finder $files
97+
* @return array $testErrors
98+
*/
99+
private function findErrorsInFileSet($files)
100+
{
101+
$actionGroupErrors = [];
102+
foreach ($files as $filePath) {
103+
$contents = file_get_contents($filePath);
104+
preg_match_all(self::ACTIONGROUP_XML_REGEX_PATTERN, $contents, $actionGroups);
105+
$actionGroupToArguments = $this->buildUnusedArgumentList($actionGroups[0]);
106+
$actionGroupErrors += $this->setErrorOutput($actionGroupToArguments, $filePath);
107+
}
108+
return $actionGroupErrors;
109+
}
110+
111+
/**
112+
* Builds array of action group => unused arguments
88113
* @param array $actionGroups
89114
* @return array $actionGroupToArguments
90115
*/
91116
private function buildUnusedArgumentList($actionGroups)
92117
{
93118
$actionGroupToArguments = [];
94119

95-
foreach ($actionGroups as $actionGroup) {
96-
$unusedArguments = $this->findUnusedArguments($actionGroup);
120+
foreach ($actionGroups as $actionGroupXml) {
121+
preg_match(self::ACTIONGROUP_NAME_REGEX_PATTERN, $actionGroupXml, $actionGroupName);
122+
$unusedArguments = $this->findUnusedArguments($actionGroupXml);
97123
if (!empty($unusedArguments)) {
98-
$actionGroupToArguments[$actionGroup->getFilename()][$actionGroup->getName()] = $unusedArguments;
124+
$actionGroupToArguments[$actionGroupName[1]] = $unusedArguments;
99125
}
100126
}
101127
return $actionGroupToArguments;
102128
}
103129

104130
/**
105-
* Returns unused arguments in an action group.
106-
* @param ActionGroupObject $actionGroup
107-
* @return array $unusedArguments
131+
* @param $actionGroupXml
132+
* @return array
108133
*/
109-
private function findUnusedArguments($actionGroup)
134+
private function findUnusedArguments($actionGroupXml)
110135
{
111136
$unusedArguments = [];
112-
//extract all action attribute values
113-
$actionAttributeValues = $this->getAllActionAttributeValues($actionGroup);
114-
$argumentList = $actionGroup->getArguments();
115-
foreach ($argumentList as $argument) {
116-
$argumentName = $argument->getName();
137+
138+
preg_match_all(self::ACTIONGROUP_ARGUMENT_REGEX_PATTERN, $actionGroupXml, $arguments);
139+
preg_match(self::ACTIONGROUP_NAME_REGEX_PATTERN, $actionGroupXml, $actionGroupName);
140+
141+
$actionGroup = ActionGroupObjectHandler::getInstance()->getObject($actionGroupName[1]);
142+
143+
foreach ($arguments[1] as $argument) {
117144
//pattern to match all argument references
118145
$patterns = [
119-
'(\{{2}' . $argumentName . '(\.[a-zA-Z0-9_\[\]\(\).,\'\/ ]+)?}{2})',
120-
'([(,\s\']' . $argumentName . '(\.[a-zA-Z0-9_\[\]]+)?[),\s\'])'
146+
'(\{{2}' . $argument . '(\.[a-zA-Z0-9_\[\]\(\).,\'\/ ]+)?}{2})',
147+
'([(,\s\']' . $argument . '(\.[a-zA-Z0-9_\[\]]+)?[),\s\'])'
121148
];
122149
// matches entity references
123-
if (preg_grep($patterns[0], $actionAttributeValues)) {
150+
if (preg_match($patterns[0], $actionGroupXml)) {
124151
continue;
125152
}
126153
//matches parametrized references
127-
if (preg_grep($patterns[1], $actionAttributeValues)) {
154+
if (preg_match($patterns[1], $actionGroupXml)) {
128155
continue;
129156
}
130-
//exclude arguments that are also defined in parent action group for extending action groups
157+
//for extending action groups, exclude arguments that are also defined in parent action group
131158
if ($this->isParentActionGroupArgument($argument, $actionGroup)) {
132159
continue;
133160
}
134-
$unusedArguments[] = $argumentName;
161+
$unusedArguments[] = $argument;
135162
}
136163
return $unusedArguments;
137164
}
@@ -147,70 +174,34 @@ private function isParentActionGroupArgument($argument, $actionGroup)
147174
if ($actionGroup->getParentName() !== null) {
148175
$parentActionGroup = ActionGroupObjectHandler::getInstance()->getObject($actionGroup->getParentName());
149176
$parentArguments = $parentActionGroup->getArguments();
150-
if ($parentArguments !== null) {
151-
return in_array($argument, $parentArguments);
152-
}
153-
return false;
154-
}
155-
}
156-
157-
/**
158-
* Returns array of all action attribute values in an action group.
159-
* @param ActionGroupObject $actionGroup
160-
* @return array $allAttributeValues
161-
*/
162-
private function getAllActionAttributeValues($actionGroup)
163-
{
164-
$allAttributeValues = [];
165-
$actions = $actionGroup->getActions();
166-
foreach ($actions as $action) {
167-
$actionAttributeValues = $this->extractAttributeValues($action);
168-
$allAttributeValues = array_merge($allAttributeValues, $actionAttributeValues);
169-
}
170-
return array_unique($allAttributeValues);
171-
}
172-
173-
/**
174-
* Builds and returns flattened attribute value list for an action.
175-
* @param ActionObject $action
176-
* @return array $flattenedAttributeValues
177-
*/
178-
private function extractAttributeValues($action)
179-
{
180-
$flattenedAttributeValues = [];
181-
$actionAttributes = $action->getCustomActionAttributes();
182-
//check if action has nodes eg. expectedResult, actualResult and flatten array
183-
foreach ($actionAttributes as $attributeName => $attributeValue) {
184-
if (is_array($attributeValue)) {
185-
$flattenedAttributeValues = array_merge($flattenedAttributeValues, array_values($attributeValue));
186-
} else {
187-
$flattenedAttributeValues[] = $attributeValue;
177+
foreach ($parentArguments as $parentArgument) {
178+
if ($argument === $parentArgument->getName()) {
179+
return true;
180+
}
188181
}
189182
}
190-
return $flattenedAttributeValues;
183+
return false;
191184
}
192185

193186
/**
194-
* Builds and returns error output for unused arguments
187+
* Builds and returns error output for violating references
195188
*
196-
* @param array $unusedArgumentList
189+
* @param array $actionGroupToArguments
190+
* @param string $path
197191
* @return mixed
198192
*/
199-
private function setErrorOutput($unusedArgumentList)
193+
private function setErrorOutput($actionGroupToArguments, $path)
200194
{
201-
$testErrors = [];
202-
if (!empty($unusedArgumentList)) {
195+
$actionGroupErrors = [];
196+
if (!empty($actionGroupToArguments)) {
203197
// Build error output
204-
foreach ($unusedArgumentList as $path => $actionGroupToArguments) {
205-
$errorOutput = "\nFile \"{$path}\"";
206-
$errorOutput .= "\ncontains action group(s) with unused arguments.\n\t\t";
207-
208-
foreach ($actionGroupToArguments as $actionGroup => $arguments) {
209-
$errorOutput .= "\n\t {$actionGroup} has unused argument(s): " . implode(", ", $arguments);
210-
}
211-
$testErrors[$path][] = $errorOutput;
198+
$errorOutput = "\nFile \"{$path->getRealPath()}\"";
199+
$errorOutput .= "\ncontains action group(s) with unused arguments.\n\t\t";
200+
foreach ($actionGroupToArguments as $actionGroup => $arguments) {
201+
$errorOutput .= "\n\t {$actionGroup} has unused argument(s): " . implode(", ", $arguments);
212202
}
203+
$actionGroupErrors[$path->getRealPath()][] = $errorOutput;
213204
}
214-
return $testErrors;
205+
return $actionGroupErrors;
215206
}
216207
}

‎src/Magento/FunctionalTestingFramework/StaticCheck/StaticCheckHelper.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\FunctionalTestingFramework\StaticCheck;
77

8+
use Symfony\Component\Finder\Finder;
9+
810
class StaticCheckHelper
911
{
1012
/**
@@ -33,4 +35,23 @@ public static function printErrorsToFile($errors, $filename, $message)
3335

3436
return $output;
3537
}
38+
39+
/**
40+
* Builds list of all XML files in given modulePaths + path given
41+
* @param array $modulePaths
42+
* @param string $path
43+
* @return Finder
44+
*/
45+
public static function buildFileList($modulePaths, $path)
46+
{
47+
$finder = new Finder();
48+
foreach ($modulePaths as $modulePath) {
49+
if (!realpath($modulePath . $path)) {
50+
continue;
51+
}
52+
$finder->files()->in($modulePath . $path)->name("*.xml");
53+
}
54+
return $finder->files();
55+
}
56+
3657
}

‎src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
namespace Magento\FunctionalTestingFramework\StaticCheck;
88

9-
use Magento\FunctionalTestingFramework\StaticCheck\StaticCheckHelper;
109
use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
1110
use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler;
1211
use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException;
@@ -117,9 +116,9 @@ public function execute(InputInterface $input)
117116
DIRECTORY_SEPARATOR . 'Data' . DIRECTORY_SEPARATOR,
118117
];
119118
// These files can contain references to other modules.
120-
$testXmlFiles = $this->buildFileList($allModules, $filePaths[0]);
121-
$actionGroupXmlFiles = $this->buildFileList($allModules, $filePaths[1]);
122-
$dataXmlFiles= $this->buildFileList($allModules, $filePaths[2]);
119+
$testXmlFiles = StaticCheckHelper::buildFileList($allModules, $filePaths[0]);
120+
$actionGroupXmlFiles = StaticCheckHelper::buildFileList($allModules, $filePaths[1]);
121+
$dataXmlFiles= StaticCheckHelper::buildFileList($allModules, $filePaths[2]);
123122

124123
$this->errors = [];
125124
$this->errors += $this->findErrorsInFileSet($testXmlFiles);
@@ -421,24 +420,6 @@ private function getModuleDependenciesFromReferences($array)
421420
return $filenames;
422421
}
423422

424-
/**
425-
* Builds list of all XML files in given modulePaths + path given
426-
* @param string $modulePaths
427-
* @param string $path
428-
* @return Finder
429-
*/
430-
private function buildFileList($modulePaths, $path)
431-
{
432-
$finder = new Finder();
433-
foreach ($modulePaths as $modulePath) {
434-
if (!realpath($modulePath . $path)) {
435-
continue;
436-
}
437-
$finder->files()->in($modulePath . $path)->name("*.xml");
438-
}
439-
return $finder->files();
440-
}
441-
442423
/**
443424
* Attempts to find any MFTF entity by its name. Returns null if none are found.
444425
* @param string $name

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,10 @@ public function getAllObjects(): array
9999
/**
100100
* Method which populates field array with objects from parsed action_group.xml
101101
*
102-
* @return array
102+
* @return void
103103
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
104104
*/
105-
public function initActionGroups()
105+
private function initActionGroups()
106106
{
107107
$actionGroupParser = ObjectManagerFactory::getObjectManager()->create(ActionGroupDataParser::class);
108108
$parsedActionGroups = $actionGroupParser->readActionGroupData();
@@ -118,7 +118,6 @@ public function initActionGroups()
118118
$this->actionGroups[$actionGroupName] =
119119
$actionGroupObjectExtractor->extractActionGroup($actionGroupData);
120120
}
121-
return $this->actionGroups;
122121
}
123122

124123
/**

0 commit comments

Comments
 (0)