Skip to content

Commit bd43c81

Browse files
imeron2433imeron2433
authored
Merge pull request #94 from magento/MQE-790
MQE-790: Error for duplicate step keys in a single action group definition
2 parents 5fe3796 + ab81af6 commit bd43c81

File tree

9 files changed

+174
-18
lines changed

9 files changed

+174
-18
lines changed

‎dev/tests/unit/Magento/FunctionalTestFramework/Test/Config/ActionGroupDomTest.php

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Tests\unit\Magento\FunctionalTestFramework\Test\Config;
7+
8+
use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
9+
use Magento\FunctionalTestingFramework\Test\Config\ActionGroupDom;
10+
use PHPUnit\Framework\TestCase;
11+
12+
class ActionGroupDomTest extends TestCase
13+
{
14+
/**
15+
* Test Action Group duplicate step key validation
16+
*/
17+
public function testActionGroupDomStepKeyValidation()
18+
{
19+
$sampleXml = "<actionGroups>
20+
<actionGroup name=\"actionGroupWithoutArguments\">
21+
<wait time=\"1\" stepKey=\"waitForNothing\" />
22+
<wait time=\"2\" stepKey=\"waitForNothing\" />
23+
</actionGroup>
24+
</actionGroups>";
25+
26+
$exceptionCollector = new ExceptionCollector();
27+
$actionDom = new ActionGroupDom($sampleXml, 'dupeStepKeyActionGroup.xml', $exceptionCollector);
28+
29+
$this->expectException(\Exception::class);
30+
$exceptionCollector->throwException();
31+
}
32+
}

‎dev/tests/verification/TestModule/ActionGroup/XmlDuplicateActionGroup.xml

+4-4
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
<amOnSubdomain stepKey="aosd2" url="1"/>
1717
<amOnUrl stepKey="aou1" url="1"/>
1818
<amOnUrl stepKey="aou2" url="1"/>
19-
<appendField selector="1" stepKey="ap1"/>
20-
<appendField selector="1" stepKey="ap2"/>
19+
<appendField selector="1" stepKey="apf1"/>
20+
<appendField selector="1" stepKey="apf2"/>
2121
<attachFile selector="1" stepKey="atf1"/>
2222
<attachFile selector="1" stepKey="atf2"/>
2323
<cancelPopup stepKey="cp1"/>
@@ -164,8 +164,8 @@
164164
<seeInField stepKey="seeinfield12"/>
165165
<seeInFormFields selector="2" parameterArray="[1]" stepKey="seeinformfields1"/>
166166
<seeInFormFields selector="2" parameterArray="[1]" stepKey="seeinformfields12"/>
167-
<seeInPageSource html="1" stepKey="seeinsource1"/>
168-
<seeInPageSource html="1" stepKey="seeinsource12"/>
167+
<seeInPageSource html="1" stepKey="seeinpgsource1"/>
168+
<seeInPageSource html="1" stepKey="seeinpgsource12"/>
169169
<seeInPopup stepKey="seeinpopup1"/>
170170
<seeInPopup stepKey="seeinpopup12"/>
171171
<seeInSource html="1" stepKey="seeinsource1"/>

‎etc/di.xml

+2-1
Original file line numberDiff line numberDiff line change
@@ -286,11 +286,12 @@
286286
</arguments>
287287
</virtualType>
288288

289-
<virtualType name="Magento\FunctionalTestingFramework\Config\Reader\ActionGroupData" type="Magento\FunctionalTestingFramework\Config\Reader\Filesystem">
289+
<virtualType name="Magento\FunctionalTestingFramework\Config\Reader\ActionGroupData" type="Magento\FunctionalTestingFramework\Test\Config\Reader\Filesystem">
290290
<arguments>
291291
<argument name="fileResolver" xsi:type="object">Magento\FunctionalTestingFramework\Config\FileResolver\Module</argument>
292292
<argument name="converter" xsi:type="object">Magento\FunctionalTestingFramework\Config\ActionGroupDataConverter</argument>
293293
<argument name="schemaLocator" xsi:type="object">Magento\FunctionalTestingFramework\Config\SchemaLocator\ActionGroup</argument>
294+
<argument name="domDocumentClass" xsi:type="string">Magento\FunctionalTestingFramework\Test\Config\ActionGroupDom</argument>
294295
<argument name="idAttributes" xsi:type="array">
295296
<item name="/actionGroups/actionGroup" xsi:type="string">name</item>
296297
<item name="/actionGroups/actionGroup/arguments/argument" xsi:type="string">name</item>

‎src/Magento/FunctionalTestingFramework/Exceptions/Collector/ExceptionCollector.php

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\FunctionalTestingFramework\Exceptions\Collector;
7+
8+
class ExceptionCollector
9+
{
10+
/**
11+
* Private array containing all errors to be thrown as part of the exception.
12+
*
13+
* @var array
14+
*/
15+
private $errors = [];
16+
17+
/**
18+
* Function to add a filename and message for the filename
19+
*
20+
* @param string $filename
21+
* @param string $message
22+
* @return void
23+
*/
24+
public function addError($filename, $message)
25+
{
26+
$error[$filename] = $message;
27+
$this->errors = array_merge_recursive($this->errors, $error);
28+
}
29+
30+
/**
31+
* Function which throws an exception when there are errors present.
32+
*
33+
* @return void
34+
* @throws \Exception
35+
*/
36+
public function throwException()
37+
{
38+
if (empty($this->errors)) {
39+
return;
40+
}
41+
42+
$errorMsg = implode("\n\n", $this->formatErrors($this->errors));
43+
throw new \Exception("\n" . $errorMsg);
44+
}
45+
46+
/**
47+
* If there are multiple exceptions for a single file, the function flattens the array so they can be printed
48+
* as separate messages.
49+
*
50+
* @param array $errors
51+
* @return array
52+
*/
53+
private function formatErrors($errors)
54+
{
55+
$flattenedErrors = [];
56+
foreach ($errors as $key => $errorMsg) {
57+
if (is_array($errorMsg)) {
58+
$flattenedErrors = array_merge($flattenedErrors, $this->formatErrors($errorMsg));
59+
continue;
60+
}
61+
62+
$flattenedErrors[] = $errorMsg;
63+
}
64+
65+
return $flattenedErrors;
66+
}
67+
}

‎src/Magento/FunctionalTestingFramework/Test/Config/ActionGroupDom.php

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\FunctionalTestingFramework\Test\Config;
7+
8+
use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
9+
10+
class ActionGroupDom extends Dom
11+
{
12+
const ACTION_GROUP_FILE_NAME_ENDING = "ActionGroup.xml";
13+
14+
/**
15+
* Takes a dom element from xml and appends the filename based on location while also validating the action group
16+
* step key.
17+
*
18+
* @param string $xml
19+
* @param string|null $filename
20+
* @param ExceptionCollector $exceptionCollector
21+
* @return \DOMDocument
22+
*/
23+
public function initDom($xml, $filename = null, $exceptionCollector = null)
24+
{
25+
$dom = parent::initDom($xml);
26+
27+
if (strpos($filename, self::ACTION_GROUP_FILE_NAME_ENDING)) {
28+
$actionGroupNodes = $dom->getElementsByTagName('actionGroup');
29+
foreach ($actionGroupNodes as $actionGroupNode) {
30+
/** @var \DOMElement $actionGroupNode */
31+
$actionGroupNode->setAttribute(self::TEST_META_FILENAME_ATTRIBUTE, $filename);
32+
$this->validateDomStepKeys($actionGroupNode, $filename, 'Action Group', $exceptionCollector);
33+
}
34+
}
35+
36+
return $dom;
37+
}
38+
}

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

+16-8
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
namespace Magento\FunctionalTestingFramework\Test\Config;
88

9+
use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
910
use Magento\FunctionalTestingFramework\Exceptions\XmlException;
1011
use Magento\FunctionalTestingFramework\Config\Dom\NodeMergingConfig;
1112
use Magento\FunctionalTestingFramework\Config\Dom\NodePathMatcher;
@@ -21,6 +22,7 @@ class Dom extends \Magento\FunctionalTestingFramework\Config\Dom
2122
* TestDom constructor.
2223
* @param string $xml
2324
* @param string $filename
25+
* @param ExceptionCollector $exceptionCollector
2426
* @param array $idAttributes
2527
* @param string $typeAttributeName
2628
* @param string $schemaFile
@@ -29,6 +31,7 @@ class Dom extends \Magento\FunctionalTestingFramework\Config\Dom
2931
public function __construct(
3032
$xml,
3133
$filename,
34+
$exceptionCollector,
3235
array $idAttributes = [],
3336
$typeAttributeName = null,
3437
$schemaFile = null,
@@ -38,7 +41,7 @@ public function __construct(
3841
$this->nodeMergingConfig = new NodeMergingConfig(new NodePathMatcher(), $idAttributes);
3942
$this->typeAttributeName = $typeAttributeName;
4043
$this->errorFormat = $errorFormat;
41-
$this->dom = $this->initDom($xml, $filename);
44+
$this->dom = $this->initDom($xml, $filename, $exceptionCollector);
4245
$this->rootNamespace = $this->dom->lookupNamespaceUri($this->dom->namespaceURI);
4346
}
4447

@@ -47,9 +50,10 @@ public function __construct(
4750
*
4851
* @param string $xml
4952
* @param string|null $filename
53+
* @param ExceptionCollector $exceptionCollector
5054
* @return \DOMDocument
5155
*/
52-
public function initDom($xml, $filename = null)
56+
public function initDom($xml, $filename = null, $exceptionCollector = null)
5357
{
5458
$dom = parent::initDom($xml);
5559

@@ -58,7 +62,7 @@ public function initDom($xml, $filename = null)
5862
foreach ($testNodes as $testNode) {
5963
/** @var \DOMElement $testNode */
6064
$testNode->setAttribute(self::TEST_META_FILENAME_ATTRIBUTE, $filename);
61-
$this->validateTestDomStepKeys($testNode, $filename);
65+
$this->validateDomStepKeys($testNode, $filename, 'Test', $exceptionCollector);
6266
}
6367
}
6468

@@ -70,11 +74,12 @@ public function initDom($xml, $filename = null)
7074
*
7175
* @param string $xml
7276
* @param string|null $filename
77+
* @param ExceptionCollector $exceptionCollector
7378
* @return void
7479
*/
75-
public function merge($xml, $filename = null)
80+
public function merge($xml, $filename = null, $exceptionCollector = null)
7681
{
77-
$dom = $this->initDom($xml, $filename);
82+
$dom = $this->initDom($xml, $filename, $exceptionCollector);
7883
$this->mergeNode($dom->documentElement, '');
7984
}
8085

@@ -83,10 +88,12 @@ public function merge($xml, $filename = null)
8388
*
8489
* @param \DOMElement $testNode
8590
* @param string $filename
91+
* @param string $type
92+
* @param ExceptionCollector $exceptionCollector
8693
* @return void
8794
* @throws XmlException
8895
*/
89-
private function validateTestDomStepKeys($testNode, $filename)
96+
protected function validateDomStepKeys($testNode, $filename, $type, $exceptionCollector)
9097
{
9198
$childNodes = $testNode->childNodes;
9299

@@ -99,7 +106,7 @@ private function validateTestDomStepKeys($testNode, $filename)
99106
}
100107

101108
if (in_array($currentNode->nodeName, self::TEST_HOOK_NAMES)) {
102-
$this->validateTestDomStepKeys($currentNode, $filename);
109+
$this->validateDomStepKeys($currentNode, $filename, $type, $exceptionCollector);
103110
}
104111

105112
if ($currentNode->hasAttribute('stepKey')) {
@@ -116,7 +123,8 @@ private function validateTestDomStepKeys($testNode, $filename)
116123
$stepKeyError .= "\tstepKey: {$duplicateValue} is used more than once.\n";
117124
}
118125

119-
throw new XmlException("Tests cannot use stepKey more than once!\t\n{$stepKeyError}\tin file: {$filename}");
126+
$errorMsg = "{$type}s cannot use stepKey more than once.\t\n{$stepKeyError}\tin file: {$filename}";
127+
$exceptionCollector->addError($filename, $errorMsg);
120128
}
121129
}
122130
}

‎src/Magento/FunctionalTestingFramework/Test/Config/Reader/Filesystem.php

+10-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
namespace Magento\FunctionalTestingFramework\Test\Config\Reader;
88

9+
use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
910
use Magento\FunctionalTestingFramework\Util\Iterator\File;
1011

1112
class Filesystem extends \Magento\FunctionalTestingFramework\Config\Reader\Filesystem
@@ -19,6 +20,8 @@ class Filesystem extends \Magento\FunctionalTestingFramework\Config\Reader\Files
1920
*/
2021
public function readFiles($fileList)
2122
{
23+
$exceptionCollector = new ExceptionCollector();
24+
$errors = [];
2225
/** @var \Magento\FunctionalTestingFramework\Test\Config\Dom $configMerger */
2326
$configMerger = null;
2427
foreach ($fileList as $key => $content) {
@@ -27,17 +30,18 @@ public function readFiles($fileList)
2730
$configMerger = $this->createConfigMerger(
2831
$this->domDocumentClass,
2932
$content,
30-
$fileList->getFilename()
33+
$fileList->getFilename(),
34+
$exceptionCollector
3135
);
3236
} else {
33-
$configMerger->merge($content, $fileList->getFilename());
37+
$configMerger->merge($content, $fileList->getFilename(), $exceptionCollector);
3438
}
3539
} catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) {
3640
throw new \Exception("Invalid XML in file " . $key . ":\n" . $e->getMessage());
3741
}
3842
}
43+
$exceptionCollector->throwException();
3944
if ($this->validationState->isValidationRequired()) {
40-
$errors = [];
4145
if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) {
4246
$message = "Invalid Document \n";
4347
throw new \Exception($message . implode("\n", $errors));
@@ -57,14 +61,16 @@ public function readFiles($fileList)
5761
* @param string $mergerClass
5862
* @param string $initialContents
5963
* @param string $filename
64+
* @param ExceptionCollector $exceptionCollector
6065
* @return \Magento\FunctionalTestingFramework\Config\Dom
6166
* @throws \UnexpectedValueException
6267
*/
63-
protected function createConfigMerger($mergerClass, $initialContents, $filename = null)
68+
protected function createConfigMerger($mergerClass, $initialContents, $filename = null, $exceptionCollector = null)
6469
{
6570
$result = new $mergerClass(
6671
$initialContents,
6772
$filename,
73+
$exceptionCollector,
6874
$this->idAttributes,
6975
null,
7076
$this->perFileSchema

‎src/Magento/FunctionalTestingFramework/Test/Util/ActionGroupObjectExtractor.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class ActionGroupObjectExtractor extends BaseObjectExtractor
1717
{
1818
const DEFAULT_VALUE = 'defaultValue';
1919
const ACTION_GROUP_ARGUMENTS = 'arguments';
20+
const FILENAME = 'filename';
2021

2122
/**
2223
* Action Object Extractor for converting actions into objects
@@ -47,9 +48,11 @@ public function extractActionGroup($actionGroupData)
4748
$actionGroupData,
4849
self::NODE_NAME,
4950
self::ACTION_GROUP_ARGUMENTS,
50-
self::NAME
51+
self::NAME,
52+
self::FILENAME
5153
);
5254

55+
// TODO filename is now available to the ActionGroupObject, integrate this into debug and error statements
5356
$actions = $this->actionObjectExtractor->extractActions($actionData);
5457

5558
if (array_key_exists(self::ACTION_GROUP_ARGUMENTS, $actionGroupData)) {

‎src/Magento/FunctionalTestingFramework/Test/etc/actionGroupSchema.xsd

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
</xs:element>
3333
</xs:choice>
3434
<xs:attribute type="xs:string" name="name" use="required"/>
35+
<xs:attribute type="xs:string" name="filename"/>
3536
</xs:complexType>
3637
<xs:simpleType name="dataTypeEnum" final="restriction">
3738
<xs:restriction base="xs:string">

0 commit comments

Comments
 (0)