MQE-1017: Better Error Messaging When Non-Whitespace Characters Are Outside XML Elements

dev/tests/_bootstrap.php

@@ -31,7 +31,8 @@
 \Magento\FunctionalTestingFramework\Config\MftfApplicationConfig::create(
     true,
     \Magento\FunctionalTestingFramework\Config\MftfApplicationConfig::GENERATION_PHASE,
-    true
+    true,
+    false
 );
 
 // Load needed framework env params

dev/tests/util/MftfTestCase.php

@@ -5,13 +5,20 @@
  */
 namespace tests\util;
 
+use Magento\FunctionalTestingFramework\ObjectManager;
 use Magento\FunctionalTestingFramework\Test\Handlers\TestObjectHandler;
 use Magento\FunctionalTestingFramework\Util\TestGenerator;
 use PHPUnit\Framework\TestCase;
 
 abstract class MftfTestCase extends TestCase
 {
-    const RESOURCES_PATH = __DIR__ . '/../verification/Resources';
+    const RESOURCES_PATH = __DIR__ .
+    DIRECTORY_SEPARATOR .
+    '..' .
+    DIRECTORY_SEPARATOR .
+    'verification' .
+    DIRECTORY_SEPARATOR .
+    'Resources';
 
     /**
      * Private function which takes a test name, generates the test and compares with a correspondingly named txt file
@@ -37,4 +44,57 @@ public function generateAndCompareTest($testName)
             $cestFile
         );
     }
-}
+
+    /**
+     * Private function which attempts to generate tests given an invalid shcema of a various type
+     *
+     * @param string[] $fileContents
+     * @param string $objectType
+     * @param string $expectedError
+     * @throws \Exception
+     */
+    public function validateSchemaErrorWithTest($fileContents, $objectType ,$expectedError)
+    {
+        $this->clearHandler();
+        $fullTestModulePath = TESTS_MODULE_PATH .
+            DIRECTORY_SEPARATOR .
+            'TestModule' .
+            DIRECTORY_SEPARATOR .
+            $objectType .
+            DIRECTORY_SEPARATOR;
+
+        foreach ($fileContents as $fileName => $fileContent) {
+            $tempFile = $fullTestModulePath . $fileName;
+            $handle = fopen($tempFile, 'w') or die('Cannot open file:  ' . $tempFile);
+            fwrite($handle, $fileContent);
+            fclose($handle);
+        }
+        try {
+            $this->expectExceptionMessage($expectedError);
+            TestObjectHandler::getInstance()->getObject("someTest");
+        } finally {
+            foreach (array_keys($fileContents) as $fileName) {
+                unlink($fullTestModulePath . $fileName);
+            }
+            $this->clearHandler();
+        }
+    }
+
+    /**
+     * Clears test handler and object manager to force recollection of test data
+     *
+     * @throws \Exception
+     */
+    private function clearHandler()
+    {
+        // clear test object handler to force recollection of test data
+        $property = new \ReflectionProperty(TestObjectHandler::class, 'testObjectHandler');
+        $property->setAccessible(true);
+        $property->setValue(null);
+
+        // clear test object handler to force recollection of test data
+        $property = new \ReflectionProperty(ObjectManager::class, 'instance');
+        $property->setAccessible(true);
+        $property->setValue(null);
+    }
+}

dev/tests/verification/TestModule/Test/PageReplacementTest.xml

@@ -22,7 +22,4 @@
         <amOnPage stepKey="oneParamAdminPageString" url="{{AdminOneParamPage.url('StringLiteral')}}"/>
         <amOnUrl stepKey="onExternalPage" url="{{ExternalPage.url}}"/>
     </test>
-    <test name="ExternalPageTestBadReference">
-        <amOnPage stepKey="onExternalPage" url="{{ExternalPage.url}}"/>
-    </test>
 </tests>

dev/tests/verification/Tests/SchemaValidationTest.php

@@ -0,0 +1,42 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace tests\verification\Tests;
+
+use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
+use tests\util\MftfTestCase;
+use AspectMock\Test as AspectMock;
+
+class SchemaValidationTest extends MftfTestCase
+{
+    /**
+     * Test generation of a test referencing an action group with no arguments
+     *
+     * @throws \Exception
+     * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException
+     */
+    public function testInvalidTestSchema()
+    {
+        AspectMock::double(MftfApplicationConfig::class, ['debugEnabled' => true]);
+        $testFile = ['testFile.xml' => "<tests><test name='testName'><annotations>a</annotations></test></tests>"];
+        $expectedError = TESTS_MODULE_PATH .
+            DIRECTORY_SEPARATOR .
+            "TestModule" .
+            DIRECTORY_SEPARATOR .
+            "Test" .
+            DIRECTORY_SEPARATOR .
+            "testFile.xml";
+        $this->validateSchemaErrorWithTest($testFile, 'Test', $expectedError);
+    }
+
+    /**
+     * After method functionality
+     * @return void
+     */
+    protected function tearDown()
+    {
+        AspectMock::clean();
+    }
+}

src/Magento/FunctionalTestingFramework/Config/MftfApplicationConfig.php

@@ -34,6 +34,13 @@ class MftfApplicationConfig
      */
     private $verboseEnabled;
 
+    /**
+     * Determines whether the user would like to execute mftf in a verbose run.
+     *
+     * @var bool
+     */
+    private $debugEnabled;
+
     /**
      * MftfApplicationConfig Singelton Instance
      *
@@ -47,10 +54,15 @@ class MftfApplicationConfig
      * @param bool $forceGenerate
      * @param string $phase
      * @param bool $verboseEnabled
+     * @param bool $debugEnabled
      * @throws TestFrameworkException
      */
-    private function __construct($forceGenerate = false, $phase = self::EXECUTION_PHASE, $verboseEnabled = null)
-    {
+    private function __construct(
+        $forceGenerate = false,
+        $phase = self::EXECUTION_PHASE,
+        $verboseEnabled = null,
+        $debugEnabled = null
+    ) {
         $this->forceGenerate = $forceGenerate;
 
         if (!in_array($phase, self::MFTF_PHASES)) {
@@ -59,6 +71,7 @@ private function __construct($forceGenerate = false, $phase = self::EXECUTION_PH
 
         $this->phase = $phase;
         $this->verboseEnabled = $verboseEnabled;
+        $this->debugEnabled = $debugEnabled;
     }
 
     /**
@@ -68,12 +81,14 @@ private function __construct($forceGenerate = false, $phase = self::EXECUTION_PH
      * @param bool $forceGenerate
      * @param string $phase
      * @param bool $verboseEnabled
+     * @param bool $debugEnabled
      * @return void
      */
-    public static function create($forceGenerate, $phase, $verboseEnabled)
+    public static function create($forceGenerate, $phase, $verboseEnabled, $debugEnabled)
     {
         if (self::$MFTF_APPLICATION_CONTEXT == null) {
-            self::$MFTF_APPLICATION_CONTEXT = new MftfApplicationConfig($forceGenerate, $phase, $verboseEnabled);
+            self::$MFTF_APPLICATION_CONTEXT =
+                new MftfApplicationConfig($forceGenerate, $phase, $verboseEnabled, $debugEnabled);
         }
     }
 
@@ -115,6 +130,17 @@ public function verboseEnabled()
         return $this->verboseEnabled ?? getenv('MFTF_DEBUG');
     }
 
+    /**
+     * Returns a boolean indicating whether the user has indicated a debug run, which will lengthy validation
+     * with some extra error messaging to be run
+     *
+     * @return bool
+     */
+    public function debugEnabled()
+    {
+        return $this->debugEnabled ?? getenv('MFTF_DEBUG');
+    }
+
     /**
      * Returns a string which indicates the phase of mftf execution.
      *

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

@@ -157,17 +157,14 @@ protected function readFiles($fileList)
                 } else {
                     $configMerger->merge($content);
                 }
+                if (MftfApplicationConfig::getConfig()->debugEnabled()) {
+                    $this->validateSchema($configMerger, $fileList->getFilename());
+                }
             } catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) {
                 throw new \Exception("Invalid XML in file " . $fileList->getFilename() . ":\n" . $e->getMessage());
             }
         }
-        if ($this->validationState->isValidationRequired()) {
-            $errors = [];
-            if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) {
-                $message = "Invalid Document \n";
-                throw new \Exception($message . implode("\n", $errors));
-            }
-        }
+        $this->validateSchema($configMerger);
 
         $output = [];
         if ($configMerger) {
@@ -220,4 +217,23 @@ protected function verifyFileEmpty($content, $fileName)
         }
         return true;
     }
+
+    /**
+     * Validate read xml against expected schema
+     *
+     * @param string $configMerger
+     * @param string $filename
+     * @throws \Exception
+     * @return void
+     */
+    protected function validateSchema($configMerger, $filename = null)
+    {
+        if ($this->validationState->isValidationRequired()) {
+            $errors = [];
+            if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) {
+                $message = $filename ? $filename . PHP_EOL . "Invalid Document \n" : PHP_EOL . "Invalid Document \n";
+                throw new \Exception($message . implode("\n", $errors));
+            }
+        }
+    }
 }

src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php

@@ -6,6 +6,7 @@
 
 namespace Magento\FunctionalTestingFramework\Config\Reader;
 
+use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig;
 use Magento\FunctionalTestingFramework\Exceptions\Collector\ExceptionCollector;
 use Magento\FunctionalTestingFramework\Util\Iterator\File;
 
@@ -21,7 +22,6 @@ class MftfFilesystem extends \Magento\FunctionalTestingFramework\Config\Reader\F
     public function readFiles($fileList)
     {
         $exceptionCollector = new ExceptionCollector();
-        $errors = [];
         /** @var \Magento\FunctionalTestingFramework\Test\Config\Dom $configMerger */
         $configMerger = null;
         foreach ($fileList as $key => $content) {
@@ -40,17 +40,15 @@ public function readFiles($fileList)
                 } else {
                     $configMerger->merge($content, $fileList->getFilename(), $exceptionCollector);
                 }
+                if (MftfApplicationConfig::getConfig()->debugEnabled()) {
+                    $this->validateSchema($configMerger, $fileList->getFilename());
+                }
             } catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) {
                 throw new \Exception("Invalid XML in file " . $key . ":\n" . $e->getMessage());
             }
         }
         $exceptionCollector->throwException();
-        if ($this->validationState->isValidationRequired()) {
-            if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) {
-                $message = "Invalid Document \n";
-                throw new \Exception($message . implode("\n", $errors));
-            }
-        }
+        $this->validateSchema($configMerger, $fileList->getFilename());
 
         $output = [];
         if ($configMerger) {

src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php

@@ -36,7 +36,8 @@ protected function configure()
             ->addOption("config", 'c', InputOption::VALUE_REQUIRED, 'default, singleRun, or parallel', 'default')
             ->addOption("force", 'f',InputOption::VALUE_NONE, 'force generation of tests regardless of Magento Instance Configuration')
             ->addOption('time', 'i', InputOption::VALUE_REQUIRED, 'Used in combination with a parallel configuration, determines desired group size (in minutes)', 10)
-            ->addOption('tests', 't', InputOption::VALUE_REQUIRED, 'A parameter accepting a JSON string used to determine the test configuration');
+            ->addOption('tests', 't', InputOption::VALUE_REQUIRED, 'A parameter accepting a JSON string used to determine the test configuration')
+            ->addOption('debug', 'd', InputOption::VALUE_NONE, 'run extra validation when generating tests');
     }
 
     /**
@@ -54,6 +55,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $json = $input->getOption('tests');
         $force = $input->getOption('force');
         $time = $input->getOption('time') * 60 * 1000; // convert from minutes to milliseconds
+        $debug = $input->getOption('debug');
         $verbose = $output->isVerbose();
 
         if ($json !== null && !json_decode($json)) {
@@ -66,7 +68,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
             throw new TestFrameworkException("time option cannot be less than or equal to 0");
         }
 
-        $testConfiguration = $this->createTestConfiguration($json, $tests, $force, $verbose);
+        $testConfiguration = $this->createTestConfiguration($json, $tests, $force, $debug, $verbose);
 
         // create our manifest file here
         $testManifest = TestManifestFactory::makeManifest($config, $testConfiguration['suites']);
@@ -92,16 +94,18 @@ protected function execute(InputInterface $input, OutputInterface $output)
      * @param string $json
      * @param array $tests
      * @param bool $force
+     * @param bool $debug
      * @param bool $verbose
      * @return array
      */
-    private function createTestConfiguration($json, array $tests, bool $force, bool $verbose)
+    private function createTestConfiguration($json, array $tests, bool $force, bool $debug, bool $verbose)
     {
         // set our application configuration so we can references the user options in our framework
         MftfApplicationConfig::create(
             $force,
             MftfApplicationConfig::GENERATION_PHASE,
-            $verbose
+            $verbose,
+            $debug
         );
 
         $testConfiguration = [];