MQE-1606: Mftf static check script should return error exit code when check fails

- Refactoring from code review

Author: tomreece

src/Magento/FunctionalTestingFramework/Console/StaticChecksCommand.php

@@ -9,18 +9,19 @@
 namespace Magento\FunctionalTestingFramework\Console;
 
 use Magento\FunctionalTestingFramework\StaticCheck\StaticChecksList;
+use Magento\FunctionalTestingFramework\StaticCheck\StaticCheckListInterface;
 use Magento\FunctionalTestingFramework\Util\Logger\LoggingUtil;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Output\OutputInterface;
+use Exception;
 
 class StaticChecksCommand extends Command
 {
     /**
      * Pool of static check scripts to run
      *
-     * @var \Magento\FunctionalTestingFramework\StaticCheck\StaticCheckListInterface
+     * @var StaticCheckListInterface
      */
     private $staticChecksList;
 
@@ -41,8 +42,8 @@ protected function configure()
      *
      * @param InputInterface $input
      * @param OutputInterface $output
-     * @return int|null|void
-     * @throws \Exception
+     * @return int
+     * @throws Exception
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
@@ -51,7 +52,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $errors = [];
 
         foreach ($staticCheckObjects as $staticCheck) {
-            $staticOutput = $staticCheck->execute($input);
+            $staticCheck->execute($input);
+
+            $staticOutput = $staticCheck->getOutput();
             LoggingUtil::getInstance()->getLogger(get_class($staticCheck))->info($staticOutput);
             $output->writeln($staticOutput);
             $errors += $staticCheck->getErrors();

src/Magento/FunctionalTestingFramework/StaticCheck/StaticCheckInterface.php

@@ -16,7 +16,7 @@ interface StaticCheckInterface
     /**
      * Executes static check script, returns output.
      * @param InputInterface $input
-     * @return string
+     * @return void
      */
     public function execute(InputInterface $input);
 
@@ -25,4 +25,10 @@ public function execute(InputInterface $input);
      * @return array
      */
     public function getErrors();
+
+    /**
+     * Return string of a short human readable result of the check. For example: "No Dependency errors found."
+     * @return string
+     */
+    public function getOutput();
 }

src/Magento/FunctionalTestingFramework/StaticCheck/StaticCheckListInterface.php

@@ -14,7 +14,7 @@ interface StaticCheckListInterface
     /**
      * Gets list of static check script instances
      *
-     * @return \Magento\FunctionalTestingFramework\StaticCheck\StaticCheckListInterface[]
+     * @return StaticCheckInterface[]
      */
     public function getStaticChecks();
 }

src/Magento/FunctionalTestingFramework/StaticCheck/StaticChecksList.php

@@ -16,14 +16,14 @@ class StaticChecksList implements StaticCheckListInterface
     /**
      * Property contains all static check scripts.
      *
-     * @var \Magento\FunctionalTestingFramework\StaticCheck\StaticCheckListInterface[]
+     * @var StaticCheckInterface[]
      */
     private $checks;
 
     /**
      * Constructor
      *
-     * @param array $scripts
+     * @param array $checks
      */
     public function __construct(array $checks = [])
     {

src/Magento/FunctionalTestingFramework/StaticCheck/TestDependencyCheck.php

@@ -6,20 +6,20 @@
 
 namespace Magento\FunctionalTestingFramework\StaticCheck;
 
-use Magento\FunctionalTestingFramework\Config\Data;
 use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
 use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler;
+use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException;
+use Magento\FunctionalTestingFramework\Exceptions\XmlException;
 use Magento\FunctionalTestingFramework\Page\Handlers\PageObjectHandler;
 use Magento\FunctionalTestingFramework\Page\Handlers\SectionObjectHandler;
-use Magento\FunctionalTestingFramework\Page\Objects\SectionObject;
 use Magento\FunctionalTestingFramework\Test\Handlers\ActionGroupObjectHandler;
 use Magento\FunctionalTestingFramework\Test\Handlers\TestObjectHandler;
 use Magento\FunctionalTestingFramework\Test\Objects\ActionObject;
 use Magento\FunctionalTestingFramework\Util\ModuleResolver;
 use Magento\FunctionalTestingFramework\Util\TestGenerator;
 use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Finder\Finder;
+use Exception;
 
 /**
  * Class TestDependencyCheck
@@ -68,11 +68,18 @@ class TestDependencyCheck implements StaticCheckInterface
      */
     private $errors;
 
+    /**
+     * String representing the output summary found after running the execute() function.
+     * @var string
+     */
+    private $output;
+
     /**
      * 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)
@@ -109,8 +116,8 @@ public function execute(InputInterface $input)
         $this->errors += $this->findErrorsInFileSet($actionGroupXmlFiles);
         $this->errors += $this->findErrorsInFileSet($dataXmlFiles);
 
-        //print all errors to file
-        return $this->printErrorsToFile($this->getErrors());
+        // hold on to the output and print any errors to a file
+        $this->output = $this->printErrorsToFile();
     }
 
     /**
@@ -122,12 +129,17 @@ public function getErrors()
         return $this->errors;
     }
 
+    public function getOutput()
+    {
+        return $this->output;
+    }
+
     /**
      * Finds all reference errors in given set of files
      * @param Finder $files
      * @return array
-     * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException
-     * @throws \Magento\FunctionalTestingFramework\Exceptions\XmlException
+     * @throws TestReferenceException
+     * @throws XmlException
      */
     private function findErrorsInFileSet($files)
     {
@@ -348,8 +360,7 @@ private function buildFileList($modulePaths, $path)
      * Attempts to find any MFTF entity by its name. Returns null if none are found.
      * @param string $name
      * @return mixed
-     * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException
-     * @throws \Magento\FunctionalTestingFramework\Exceptions\XmlException
+     * @throws XmlException
      */
     private function findEntity($name)
     {
@@ -378,24 +389,29 @@ private function findEntity($name)
 
     /**
      * Prints out given errors to file, and returns summary result string
-     * @param array $errors
      * @return string
      */
-    private function printErrorsToFile($errors)
+    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;
     }
 }