MQE-1676: Add a static-check that ensures action groups do not have unused arguments

Refactoring + added logic to exclude arguments found in parent action group

Author: soumyau

src/Magento/FunctionalTestingFramework/StaticCheck/UnusedArgumentsCheck.php renamed to src/Magento/FunctionalTestingFramework/StaticCheck/ActionGroupArgumentsCheck.php

@@ -14,12 +14,13 @@
 use Exception;
 
 /**
- * Class UnusedArgumentsCheck
+ * Class ActionGroupArgumentsCheck
  * @package Magento\FunctionalTestingFramework\StaticCheck
- * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-class UnusedArgumentsCheck implements StaticCheckInterface
+class ActionGroupArgumentsCheck implements StaticCheckInterface
 {
+    const ERROR_LOG_FILENAME = 'mftf-arguments-checks';
+    const ERROR_LOG_MESSAGE = 'MFTF Unused Arguments Check';
 
     /**
      * Array containing all errors found after running the execute() function.
@@ -38,10 +39,11 @@ class UnusedArgumentsCheck implements StaticCheckInterface
      *
      * @param  InputInterface $input
      * @return void
-     * @throws Exception;
+     * @throws Exception
      */
     public function execute(InputInterface $input)
     {
+
         MftfApplicationConfig::create(
             true,
             MftfApplicationConfig::UNIT_TEST_PHASE,
@@ -56,7 +58,8 @@ public function execute(InputInterface $input)
 
         $this->errors += $this->setErrorOutput($unusedArgumentList);
 
-        $this->output = $this->printErrorsToFile();
+        $this->output = StaticCheckHelper::printErrorsToFile($this->errors,
+            self::ERROR_LOG_FILENAME, self::ERROR_LOG_MESSAGE);
     }
 
     /**
@@ -109,15 +112,45 @@ private function findUnusedArguments($actionGroup)
         foreach ($argumentList as $argument) {
             $argumentName = $argument->getName();
             //pattern to match all argument references
-            $pattern = '(.*[\W]+(?<!\.)' . $argumentName . '[\W]+.*)';
-            if (preg_grep($pattern, $actionAttributeValues)) {
+            $patterns = [
+                '(\{{2}' . $argumentName . '(\.[a-zA-Z0-9_\[\]\(\).,\'\/ ]+)?}{2})',
+                '([(,\s\']' . $argumentName . '(\.[a-zA-Z0-9_\[\]]+)?[),\s\'])'
+            ];
+            // matches entity references
+            if (preg_grep($patterns[0], $actionAttributeValues)) {
+                continue;
+            }
+            //matches parametrized references
+            if (preg_grep($patterns[1], $actionAttributeValues)) {
                 continue;
             }
-            $unusedArguments[] = $argument->getName();
+            //exclude arguments that are also defined in parent action group for extending action groups
+            if ($this->isParentActionGroupArgument($argument, $actionGroup)) {
+                continue;
+            }
+            $unusedArguments[] = $argumentName;
         }
         return $unusedArguments;
     }
 
+    /**
+     * Checks if the argument is also defined in the parent for extending action groups.
+     * @param string $argument
+     * @param ActionGroupObject $actionGroup
+     * @return bool
+     */
+    private function isParentActionGroupArgument($argument, $actionGroup) {
+
+        if ($actionGroup->getParentName() !== null) {
+            $parentActionGroup = ActionGroupObjectHandler::getInstance()->getObject($actionGroup->getParentName());
+            $parentArguments = $parentActionGroup->getArguments();
+            if ($parentArguments !== null) {
+                return in_array($argument, $parentArguments);
+            }
+            return false;
+        }
+    }
+
     /**
      * Returns array of all action attribute values in an action group.
      * @param ActionGroupObject $actionGroup
@@ -163,7 +196,6 @@ private function extractAttributeValues($action)
     private function setErrorOutput($unusedArgumentList)
     {
         $testErrors = [];
-
         if (!empty($unusedArgumentList)) {
             // Build error output
             foreach ($unusedArgumentList as $path => $actionGroupToArguments) {
@@ -178,32 +210,4 @@ private function setErrorOutput($unusedArgumentList)
         }
         return $testErrors;
     }
-
-    /**
-     * Prints out given errors to file, and returns summary result string
-     * @return string
-     */
-    private function printErrorsToFile()
-    {
-        $errors = $this->getErrors();
-
-        if (empty($errors)) {
-            return "No unused arguments found.";
-        }
-
-        $outputPath = getcwd() . DIRECTORY_SEPARATOR . "mftf-arguments-checks.txt";
-        $fileResource = fopen($outputPath, 'w');
-        $header = "MFTF ActionGroup Arguments Check:\n";
-        fwrite($fileResource, $header);
-
-        foreach ($errors as $test => $error) {
-            fwrite($fileResource, $error[0] . PHP_EOL);
-        }
-
-        fclose($fileResource);
-        $errorCount = count($errors);
-        $output = "Unused arguments found across {$errorCount} actionGroup(s). Error details output to {$outputPath}";
-
-        return $output;
-    }
 }

src/Magento/FunctionalTestingFramework/StaticCheck/StaticCheckHelper.php

@@ -0,0 +1,36 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\FunctionalTestingFramework\StaticCheck;
+
+class StaticCheckHelper
+{
+    /**
+     * Prints out given errors to file, and returns summary result string
+     * @param array $errors
+     * @param string $filename
+     * @param string $message
+     * @return string
+     */
+    public static function printErrorsToFile($errors, $filename, $message)
+    {
+        if (empty($errors)) {
+            return $message . ": No errors found.";
+        }
+
+        $outputPath = getcwd() . DIRECTORY_SEPARATOR . $filename . ".txt";
+        $fileResource = fopen($outputPath, 'w');
+
+        foreach ($errors as $test => $error) {
+            fwrite($fileResource, $error[0] . PHP_EOL);
+        }
+
+        fclose($fileResource);
+        $errorCount = count($errors);
+        $output = $message . ": Errors found across {$errorCount} file(s). Error details output to {$outputPath}";
+
+        return $output;
+    }
+}

src/Magento/FunctionalTestingFramework/StaticCheck/StaticChecksList.php

@@ -29,7 +29,7 @@ public function __construct(array $checks = [])
     {
         $this->checks = [
             'testDependencies' => new TestDependencyCheck(),
-            'unusedArgumentsCheck' => new UnusedArgumentsCheck(),
+            'actionGroupArguments' => new ActionGroupArgumentsCheck(),
         ] + $checks;
     }
 

src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php

@@ -6,6 +6,7 @@
 
 namespace Magento\FunctionalTestingFramework\StaticCheck;
 
+use Magento\FunctionalTestingFramework\StaticCheck\StaticCheckHelper;
 use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
 use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler;
 use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException;
@@ -32,6 +33,9 @@ class TestDependencyCheck implements StaticCheckInterface
     const ACTIONGROUP_REGEX_PATTERN = '/ref=["\']([^\'"]*)/';
     const ACTIONGROUP_ARGUMENT_REGEX_PATTERN = '/<argument[^\/>]*name="([^"\']*)/';
 
+    const ERROR_LOG_FILENAME = 'mftf-dependency-checks';
+    const ERROR_LOG_MESSAGE = 'MFTF File Dependency Check';
+
     /**
      * Array of FullModuleName => [dependencies]
      * @var array
@@ -123,7 +127,8 @@ public function execute(InputInterface $input)
         $this->errors += $this->findErrorsInFileSet($dataXmlFiles);
 
         // hold on to the output and print any errors to a file
-        $this->output = $this->printErrorsToFile();
+        $this->output = StaticCheckHelper::printErrorsToFile($this->errors,
+            self::ERROR_LOG_FILENAME, self::ERROR_LOG_MESSAGE);
     }
 
     /**
@@ -461,32 +466,4 @@ private function findEntity($name)
         }
         return null;
     }
-
-    /**
-     * Prints out given errors to file, and returns summary result string
-     * @return string
-     */
-    private function printErrorsToFile()
-    {
-        $errors = $this->getErrors();
-
-        if (empty($errors)) {
-            return "No Dependency errors found.";
-        }
-
-        $outputPath = getcwd() . DIRECTORY_SEPARATOR . "mftf-dependency-checks.txt";
-        $fileResource = fopen($outputPath, 'w');
-        $header = "MFTF File Dependency Check:\n";
-        fwrite($fileResource, $header);
-
-        foreach ($errors as $test => $error) {
-            fwrite($fileResource, $error[0] . PHP_EOL);
-        }
-
-        fclose($fileResource);
-        $errorCount = count($errors);
-        $output = "Dependency errors found across {$errorCount} file(s). Error details output to {$outputPath}";
-
-        return $output;
-    }
 }