From 596efbe770357fa764a47b33ebd695e9c01bda61 Mon Sep 17 00:00:00 2001 From: Alex Calandra Date: Wed, 6 Jun 2018 16:14:57 -0500 Subject: [PATCH 1/4] MQE-1017: Better Error Messaging When Non-Whitespace Characters Are Outside XML Elements - Adding debug flag to generate tests - Adding individual file validation for schema when debug is on --- dev/tests/_bootstrap.php | 1 + .../Config/MftfApplicationConfig.php | 34 ++++++++++++++++--- .../Config/Reader/Filesystem.php | 7 ++++ .../Config/Reader/MftfFilesystem.php | 11 +++++- .../Console/GenerateTestsCommand.php | 12 ++++--- 5 files changed, 56 insertions(+), 9 deletions(-) diff --git a/dev/tests/_bootstrap.php b/dev/tests/_bootstrap.php index 0baf41c56..84cf32264 100644 --- a/dev/tests/_bootstrap.php +++ b/dev/tests/_bootstrap.php @@ -31,6 +31,7 @@ \Magento\FunctionalTestingFramework\Config\MftfApplicationConfig::create( true, \Magento\FunctionalTestingFramework\Config\MftfApplicationConfig::GENERATION_PHASE, + true, true ); diff --git a/src/Magento/FunctionalTestingFramework/Config/MftfApplicationConfig.php b/src/Magento/FunctionalTestingFramework/Config/MftfApplicationConfig.php index 4d4500b1e..81505067f 100644 --- a/src/Magento/FunctionalTestingFramework/Config/MftfApplicationConfig.php +++ b/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. * diff --git a/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php b/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php index adcb330ba..9ed63798f 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php +++ b/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php @@ -150,6 +150,13 @@ protected function readFiles($fileList) } else { $configMerger->merge($content); } + if ($this->validationState->isValidationRequired()) { + $errors = []; + if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) { + $message = $fileList->getFilename() . PHP_EOL . "Invalid Document \n"; + throw new \Exception($message . implode("\n", $errors)); + } + } } catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) { throw new \Exception("Invalid XML in file " . $fileList->getFilename() . ":\n" . $e->getMessage()); } diff --git a/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php b/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php index e39e39199..b65b7886c 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php +++ b/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; @@ -36,6 +37,14 @@ public function readFiles($fileList) } else { $configMerger->merge($content, $fileList->getFilename(), $exceptionCollector); } + if (MftfApplicationConfig::getConfig()->debugEnabled()) { + if ($this->validationState->isValidationRequired()) { + if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) { + $message = "Invalid Document: " . PHP_EOL . $fileList->getFilename() . PHP_EOL; + throw new \Exception($message . implode("\n", $errors)); + } + } + } } catch (\Magento\FunctionalTestingFramework\Config\Dom\ValidationException $e) { throw new \Exception("Invalid XML in file " . $key . ":\n" . $e->getMessage()); } @@ -43,7 +52,7 @@ public function readFiles($fileList) $exceptionCollector->throwException(); if ($this->validationState->isValidationRequired()) { if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) { - $message = "Invalid Document \n"; + $message = "Invalid Document: " . PHP_EOL; throw new \Exception($message . implode("\n", $errors)); } } diff --git a/src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php b/src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php index a8d34f1c9..912dca4a8 100644 --- a/src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php +++ b/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('lines', 'l', InputOption::VALUE_REQUIRED, 'Used in combination with a parallel configuration, determines desired group size', 500) - ->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'); $lines = $input->getOption('lines'); + $debug = $input->getOption('debug'); $verbose = $output->isVerbose(); if ($json !== null && !json_decode($json)) { @@ -61,7 +63,7 @@ protected function execute(InputInterface $input, OutputInterface $output) throw new TestFrameworkException("JSON could not be parsed: " . json_last_error_msg()); } - $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']); @@ -84,16 +86,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 = []; From 5afa47ef4e7b9f9dadae9414552958d047976c32 Mon Sep 17 00:00:00 2001 From: Alex Calandra Date: Thu, 7 Jun 2018 11:19:47 -0500 Subject: [PATCH 2/4] MQE-1017: Better Error Messaging When Non-Whitespace Characters Are Outside XML Elements - Abstracted validation code in Filesystem --- .../Config/Reader/Filesystem.php | 37 ++++++++++++------- .../Config/Reader/MftfFilesystem.php | 15 +------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php b/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php index 9ed63798f..224b3a2d5 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php +++ b/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php @@ -5,6 +5,8 @@ */ namespace Magento\FunctionalTestingFramework\Config\Reader; +use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; + /** * Filesystem configuration loader. Loads configuration from XML files, split by scopes. */ @@ -150,24 +152,14 @@ protected function readFiles($fileList) } else { $configMerger->merge($content); } - if ($this->validationState->isValidationRequired()) { - $errors = []; - if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) { - $message = $fileList->getFilename() . PHP_EOL . "Invalid Document \n"; - throw new \Exception($message . implode("\n", $errors)); - } + 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) { @@ -199,4 +191,23 @@ protected function createConfigMerger($mergerClass, $initialContents) } return $result; } + + /** + * 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)); + } + } + } } diff --git a/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php b/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php index b65b7886c..8d0fcd373 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php +++ b/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php @@ -22,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) { @@ -38,24 +37,14 @@ public function readFiles($fileList) $configMerger->merge($content, $fileList->getFilename(), $exceptionCollector); } if (MftfApplicationConfig::getConfig()->debugEnabled()) { - if ($this->validationState->isValidationRequired()) { - if ($configMerger && !$configMerger->validate($this->schemaFile, $errors)) { - $message = "Invalid Document: " . PHP_EOL . $fileList->getFilename() . PHP_EOL; - throw new \Exception($message . implode("\n", $errors)); - } - } + $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: " . PHP_EOL; - throw new \Exception($message . implode("\n", $errors)); - } - } + $this->validateSchema($configMerger, $fileList->getFilename()); $output = []; if ($configMerger) { From eac8f86f426ac39f7a9df3fb34e5f55d9711ee82 Mon Sep 17 00:00:00 2001 From: Alex Calandra Date: Fri, 8 Jun 2018 13:56:33 -0500 Subject: [PATCH 3/4] MQE-1017: Better Error Messaging When Non-Whitespace Characters Are Outside XML Elements - Adding verification test for filesystem to check error --- dev/tests/util/MftfTestCase.php | 64 ++++++++++++++++++- .../TestModule/Test/PageReplacementTest.xml | 3 - .../Tests/SchemaValidationTest.php | 30 +++++++++ 3 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 dev/tests/verification/Tests/SchemaValidationTest.php diff --git a/dev/tests/util/MftfTestCase.php b/dev/tests/util/MftfTestCase.php index e96c75fde..8bfa1009f 100644 --- a/dev/tests/util/MftfTestCase.php +++ b/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 ); } -} \ No newline at end of file + + /** + * 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); + } +} diff --git a/dev/tests/verification/TestModule/Test/PageReplacementTest.xml b/dev/tests/verification/TestModule/Test/PageReplacementTest.xml index acdbbb798..b6adee5d7 100644 --- a/dev/tests/verification/TestModule/Test/PageReplacementTest.xml +++ b/dev/tests/verification/TestModule/Test/PageReplacementTest.xml @@ -22,7 +22,4 @@ - - - \ No newline at end of file diff --git a/dev/tests/verification/Tests/SchemaValidationTest.php b/dev/tests/verification/Tests/SchemaValidationTest.php new file mode 100644 index 000000000..7d247693e --- /dev/null +++ b/dev/tests/verification/Tests/SchemaValidationTest.php @@ -0,0 +1,30 @@ + "a"]; + $expectedError = TESTS_MODULE_PATH . + DIRECTORY_SEPARATOR . + "TestModule" . + DIRECTORY_SEPARATOR . + "Test" . + DIRECTORY_SEPARATOR . + "testFile.xml"; + $this->validateSchemaErrorWithTest($testFile, 'Test', $expectedError); + } +} From 320e5d38481d476608204f4e1e6165fb0894b288 Mon Sep 17 00:00:00 2001 From: Alex Calandra Date: Wed, 13 Jun 2018 09:53:19 -0500 Subject: [PATCH 4/4] MQE-1017: Better Error Messaging When Non-Whitespace Characters Are Outside XML Elements - Changed default verification tests to use debug turned off --- dev/tests/_bootstrap.php | 2 +- .../verification/Tests/SchemaValidationTest.php | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/dev/tests/_bootstrap.php b/dev/tests/_bootstrap.php index 84cf32264..0adfc2e74 100644 --- a/dev/tests/_bootstrap.php +++ b/dev/tests/_bootstrap.php @@ -32,7 +32,7 @@ true, \Magento\FunctionalTestingFramework\Config\MftfApplicationConfig::GENERATION_PHASE, true, - true + false ); // Load needed framework env params diff --git a/dev/tests/verification/Tests/SchemaValidationTest.php b/dev/tests/verification/Tests/SchemaValidationTest.php index 7d247693e..86828e9a5 100644 --- a/dev/tests/verification/Tests/SchemaValidationTest.php +++ b/dev/tests/verification/Tests/SchemaValidationTest.php @@ -5,7 +5,9 @@ */ namespace tests\verification\Tests; +use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; use tests\util\MftfTestCase; +use AspectMock\Test as AspectMock; class SchemaValidationTest extends MftfTestCase { @@ -17,6 +19,7 @@ class SchemaValidationTest extends MftfTestCase */ public function testInvalidTestSchema() { + AspectMock::double(MftfApplicationConfig::class, ['debugEnabled' => true]); $testFile = ['testFile.xml' => "a"]; $expectedError = TESTS_MODULE_PATH . DIRECTORY_SEPARATOR . @@ -27,4 +30,13 @@ public function testInvalidTestSchema() "testFile.xml"; $this->validateSchemaErrorWithTest($testFile, 'Test', $expectedError); } + + /** + * After method functionality + * @return void + */ + protected function tearDown() + { + AspectMock::clean(); + } }