MQE-790: Error for duplicate step keys in a single action group definition

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

@@ -0,0 +1,32 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Tests\unit\Magento\FunctionalTestFramework\Test\Config;
+
+use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
+use Magento\FunctionalTestingFramework\Test\Config\ActionGroupDom;
+use PHPUnit\Framework\TestCase;
+
+class ActionGroupDomTest extends TestCase
+{
+    /**
+     * Test Action Group duplicate step key validation
+     */
+    public function testActionGroupDomStepKeyValidation()
+    {
+        $sampleXml = "<actionGroups>
+            <actionGroup name=\"actionGroupWithoutArguments\">
+                <wait time=\"1\" stepKey=\"waitForNothing\" />
+                <wait time=\"2\" stepKey=\"waitForNothing\" />
+            </actionGroup>
+         </actionGroups>";
+
+        $exceptionCollector = new ExceptionCollector();
+        $actionDom = new ActionGroupDom($sampleXml, 'dupeStepKeyActionGroup.xml', $exceptionCollector);
+
+        $this->expectException(\Exception::class);
+        $exceptionCollector->throwException();
+    }
+}

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

@@ -16,8 +16,8 @@
         <amOnSubdomain stepKey="aosd2" url="1"/>
         <amOnUrl stepKey="aou1" url="1"/>
         <amOnUrl stepKey="aou2" url="1"/>
-        <appendField selector="1" stepKey="ap1"/>
-        <appendField selector="1" stepKey="ap2"/>
+        <appendField selector="1" stepKey="apf1"/>
+        <appendField selector="1" stepKey="apf2"/>
         <attachFile selector="1" stepKey="atf1"/>
         <attachFile selector="1" stepKey="atf2"/>
         <cancelPopup stepKey="cp1"/>
@@ -164,8 +164,8 @@
         <seeInField stepKey="seeinfield12"/>
         <seeInFormFields selector="2" parameterArray="[1]"  stepKey="seeinformfields1"/>
         <seeInFormFields selector="2" parameterArray="[1]"  stepKey="seeinformfields12"/>
-        <seeInPageSource html="1" stepKey="seeinsource1"/>
-        <seeInPageSource html="1" stepKey="seeinsource12"/>
+        <seeInPageSource html="1" stepKey="seeinpgsource1"/>
+        <seeInPageSource html="1" stepKey="seeinpgsource12"/>
         <seeInPopup stepKey="seeinpopup1"/>
         <seeInPopup stepKey="seeinpopup12"/>
         <seeInSource html="1" stepKey="seeinsource1"/>

etc/di.xml

@@ -286,11 +286,12 @@
         </arguments>
     </virtualType>
 
-    <virtualType name="Magento\FunctionalTestingFramework\Config\Reader\ActionGroupData" type="Magento\FunctionalTestingFramework\Config\Reader\Filesystem">
+    <virtualType name="Magento\FunctionalTestingFramework\Config\Reader\ActionGroupData" type="Magento\FunctionalTestingFramework\Test\Config\Reader\Filesystem">
         <arguments>
             <argument name="fileResolver" xsi:type="object">Magento\FunctionalTestingFramework\Config\FileResolver\Module</argument>
             <argument name="converter" xsi:type="object">Magento\FunctionalTestingFramework\Config\ActionGroupDataConverter</argument>
             <argument name="schemaLocator" xsi:type="object">Magento\FunctionalTestingFramework\Config\SchemaLocator\ActionGroup</argument>
+            <argument name="domDocumentClass" xsi:type="string">Magento\FunctionalTestingFramework\Test\Config\ActionGroupDom</argument>
             <argument name="idAttributes" xsi:type="array">
                 <item name="/actionGroups/actionGroup" xsi:type="string">name</item>
                 <item name="/actionGroups/actionGroup/arguments/argument" xsi:type="string">name</item>

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

@@ -0,0 +1,67 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\FunctionalTestingFramework\Exceptions\Collector;
+
+class ExceptionCollector
+{
+    /**
+     * Private array containing all errors to be thrown as part of the exception.
+     *
+     * @var array
+     */
+    private $errors = [];
+
+    /**
+     * Function to add a filename and message for the filename
+     *
+     * @param string $filename
+     * @param string $message
+     * @return void
+     */
+    public function addError($filename, $message)
+    {
+        $error[$filename] = $message;
+        $this->errors = array_merge_recursive($this->errors, $error);
+    }
+
+    /**
+     * Function which throws an exception when there are errors present.
+     *
+     * @return void
+     * @throws \Exception
+     */
+    public function throwException()
+    {
+        if (empty($this->errors)) {
+            return;
+        }
+
+        $errorMsg = implode("\n\n", $this->formatErrors($this->errors));
+        throw new \Exception("\n" . $errorMsg);
+    }
+
+    /**
+     * If there are multiple exceptions for a single file, the function flattens the array so they can be printed
+     * as separate messages.
+     *
+     * @param array $errors
+     * @return array
+     */
+    private function formatErrors($errors)
+    {
+        $flattenedErrors = [];
+        foreach ($errors as $key => $errorMsg) {
+            if (is_array($errorMsg)) {
+                $flattenedErrors = array_merge($flattenedErrors, $this->formatErrors($errorMsg));
+                continue;
+            }
+
+            $flattenedErrors[] = $errorMsg;
+        }
+
+        return $flattenedErrors;
+    }
+}

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

@@ -0,0 +1,38 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\FunctionalTestingFramework\Test\Config;
+
+use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
+
+class ActionGroupDom extends Dom
+{
+    const ACTION_GROUP_FILE_NAME_ENDING = "ActionGroup.xml";
+
+    /**
+     * Takes a dom element from xml and appends the filename based on location while also validating the action group
+     * step key.
+     *
+     * @param string $xml
+     * @param string|null $filename
+     * @param ExceptionCollector $exceptionCollector
+     * @return \DOMDocument
+     */
+    public function initDom($xml, $filename = null, $exceptionCollector = null)
+    {
+        $dom = parent::initDom($xml);
+
+        if (strpos($filename, self::ACTION_GROUP_FILE_NAME_ENDING)) {
+            $actionGroupNodes = $dom->getElementsByTagName('actionGroup');
+            foreach ($actionGroupNodes as $actionGroupNode) {
+                /** @var \DOMElement $actionGroupNode */
+                $actionGroupNode->setAttribute(self::TEST_META_FILENAME_ATTRIBUTE, $filename);
+                $this->validateDomStepKeys($actionGroupNode, $filename, 'Action Group', $exceptionCollector);
+            }
+        }
+
+        return $dom;
+    }
+}

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

@@ -6,6 +6,7 @@
 
 namespace Magento\FunctionalTestingFramework\Test\Config;
 
+use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
 use Magento\FunctionalTestingFramework\Exceptions\XmlException;
 use Magento\FunctionalTestingFramework\Config\Dom\NodeMergingConfig;
 use Magento\FunctionalTestingFramework\Config\Dom\NodePathMatcher;
@@ -21,6 +22,7 @@ class Dom extends \Magento\FunctionalTestingFramework\Config\Dom
      * TestDom constructor.
      * @param string $xml
      * @param string $filename
+     * @param ExceptionCollector $exceptionCollector
      * @param array $idAttributes
      * @param string $typeAttributeName
      * @param string $schemaFile
@@ -29,6 +31,7 @@ class Dom extends \Magento\FunctionalTestingFramework\Config\Dom
     public function __construct(
         $xml,
         $filename,
+        $exceptionCollector,
         array $idAttributes = [],
         $typeAttributeName = null,
         $schemaFile = null,
@@ -38,7 +41,7 @@ public function __construct(
         $this->nodeMergingConfig = new NodeMergingConfig(new NodePathMatcher(), $idAttributes);
         $this->typeAttributeName = $typeAttributeName;
         $this->errorFormat = $errorFormat;
-        $this->dom = $this->initDom($xml, $filename);
+        $this->dom = $this->initDom($xml, $filename, $exceptionCollector);
         $this->rootNamespace = $this->dom->lookupNamespaceUri($this->dom->namespaceURI);
     }
 
@@ -47,9 +50,10 @@ public function __construct(
      *
      * @param string $xml
      * @param string|null $filename
+     * @param ExceptionCollector $exceptionCollector
      * @return \DOMDocument
      */
-    public function initDom($xml, $filename = null)
+    public function initDom($xml, $filename = null, $exceptionCollector = null)
     {
         $dom = parent::initDom($xml);
 
@@ -58,7 +62,7 @@ public function initDom($xml, $filename = null)
             foreach ($testNodes as $testNode) {
                 /** @var \DOMElement $testNode */
                 $testNode->setAttribute(self::TEST_META_FILENAME_ATTRIBUTE, $filename);
-                $this->validateTestDomStepKeys($testNode, $filename);
+                $this->validateDomStepKeys($testNode, $filename, 'Test', $exceptionCollector);
             }
         }
 
@@ -70,11 +74,12 @@ public function initDom($xml, $filename = null)
      *
      * @param string $xml
      * @param string|null $filename
+     * @param ExceptionCollector $exceptionCollector
      * @return void
      */
-    public function merge($xml, $filename = null)
+    public function merge($xml, $filename = null, $exceptionCollector = null)
     {
-        $dom = $this->initDom($xml, $filename);
+        $dom = $this->initDom($xml, $filename, $exceptionCollector);
         $this->mergeNode($dom->documentElement, '');
     }
 
@@ -83,10 +88,12 @@ public function merge($xml, $filename = null)
      *
      * @param \DOMElement $testNode
      * @param string $filename
+     * @param string $type
+     * @param ExceptionCollector $exceptionCollector
      * @return void
      * @throws XmlException
      */
-    private function validateTestDomStepKeys($testNode, $filename)
+    protected function validateDomStepKeys($testNode, $filename, $type, $exceptionCollector)
     {
         $childNodes = $testNode->childNodes;
 
@@ -99,7 +106,7 @@ private function validateTestDomStepKeys($testNode, $filename)
             }
 
             if (in_array($currentNode->nodeName, self::TEST_HOOK_NAMES)) {
-                $this->validateTestDomStepKeys($currentNode, $filename);
+                $this->validateDomStepKeys($currentNode, $filename, $type, $exceptionCollector);
             }
 
             if ($currentNode->hasAttribute('stepKey')) {
@@ -116,7 +123,8 @@ private function validateTestDomStepKeys($testNode, $filename)
                 $stepKeyError .= "\tstepKey: {$duplicateValue} is used more than once.\n";
             }
 
-            throw new XmlException("Tests cannot use stepKey more than once!\t\n{$stepKeyError}\tin file: {$filename}");
+            $errorMsg = "{$type}s cannot use stepKey more than once.\t\n{$stepKeyError}\tin file: {$filename}";
+            $exceptionCollector->addError($filename, $errorMsg);
         }
     }
 }

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

@@ -6,6 +6,7 @@
 
 namespace Magento\FunctionalTestingFramework\Test\Config\Reader;
 
+use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
 use Magento\FunctionalTestingFramework\Util\Iterator\File;
 
 class Filesystem extends \Magento\FunctionalTestingFramework\Config\Reader\Filesystem
@@ -19,6 +20,8 @@ class Filesystem extends \Magento\FunctionalTestingFramework\Config\Reader\Files
      */
     public function readFiles($fileList)
     {
+        $exceptionCollector = new ExceptionCollector();
+        $errors = [];
         /** @var \Magento\FunctionalTestingFramework\Test\Config\Dom $configMerger */
         $configMerger = null;
         foreach ($fileList as $key => $content) {
@@ -27,17 +30,18 @@ public function readFiles($fileList)
                     $configMerger = $this->createConfigMerger(
                         $this->domDocumentClass,
                         $content,
-                        $fileList->getFilename()
+                        $fileList->getFilename(),
+                        $exceptionCollector
                     );
                 } else {
-                    $configMerger->merge($content, $fileList->getFilename());
+                    $configMerger->merge($content, $fileList->getFilename(), $exceptionCollector);
                 }
             } catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) {
                 throw new \Exception("Invalid XML in file " . $key . ":\n" . $e->getMessage());
             }
         }
+        $exceptionCollector->throwException();
         if ($this->validationState->isValidationRequired()) {
-            $errors = [];
             if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) {
                 $message = "Invalid Document \n";
                 throw new \Exception($message . implode("\n", $errors));
@@ -57,14 +61,16 @@ public function readFiles($fileList)
      * @param string $mergerClass
      * @param string $initialContents
      * @param string $filename
+     * @param ExceptionCollector $exceptionCollector
      * @return \Magento\FunctionalTestingFramework\Config\Dom
      * @throws \UnexpectedValueException
      */
-    protected function createConfigMerger($mergerClass, $initialContents, $filename = null)
+    protected function createConfigMerger($mergerClass, $initialContents, $filename = null, $exceptionCollector = null)
     {
         $result = new $mergerClass(
             $initialContents,
             $filename,
+            $exceptionCollector,
             $this->idAttributes,
             null,
             $this->perFileSchema

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

@@ -17,6 +17,7 @@ class ActionGroupObjectExtractor extends BaseObjectExtractor
 {
     const DEFAULT_VALUE = 'defaultValue';
     const ACTION_GROUP_ARGUMENTS = 'arguments';
+    const FILENAME = 'filename';
 
     /**
      * Action Object Extractor for converting actions into objects
@@ -47,9 +48,11 @@ public function extractActionGroup($actionGroupData)
             $actionGroupData,
             self::NODE_NAME,
             self::ACTION_GROUP_ARGUMENTS,
-            self::NAME
+            self::NAME,
+            self::FILENAME
         );
 
+        // TODO filename is now available to the ActionGroupObject, integrate this into debug and error statements
         $actions = $this->actionObjectExtractor->extractActions($actionData);
 
         if (array_key_exists(self::ACTION_GROUP_ARGUMENTS, $actionGroupData)) {

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

@@ -32,6 +32,7 @@
             </xs:element>
         </xs:choice>
         <xs:attribute type="xs:string" name="name" use="required"/>
+        <xs:attribute type="xs:string" name="filename"/>
     </xs:complexType>
     <xs:simpleType name="dataTypeEnum" final="restriction">
         <xs:restriction base="xs:string">