diff --git a/dev/tests/_bootstrap.php b/dev/tests/_bootstrap.php index 0baf41c56..0adfc2e74 100644 --- a/dev/tests/_bootstrap.php +++ b/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 diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionGroupObjectExtractorTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionGroupObjectExtractorTest.php new file mode 100644 index 000000000..47b797c1c --- /dev/null +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionGroupObjectExtractorTest.php @@ -0,0 +1,70 @@ +testActionGroupObjectExtractor = new ActionGroupObjectExtractor(); + TestLoggingUtil::getInstance()->setMockLoggingUtil(); + } + + /** + * Tests basic action object extraction with an empty stepKey + */ + public function testEmptyStepKey() + { + $this->expectExceptionMessage( + "StepKeys cannot be empty. Action='sampleAction' in Action Group filename.xml" + ); + $this->testActionGroupObjectExtractor->extractActionGroup($this->createBasicActionObjectArray("")); + } + + /** + * Utility function to return mock parser output for testing extraction into ActionObjects. + * + * @param string $stepKey + * @param string $actionGroup + * @param string $filename + * @return array + */ + private function createBasicActionObjectArray( + $stepKey = 'testAction1', + $actionGroup = "actionGroup", + $filename = "filename.xml" + ) { + $baseArray = [ + 'nodeName' => 'actionGroup', + 'name' => $actionGroup, + 'filename' => $filename, + $stepKey => [ + "nodeName" => "sampleAction", + "stepKey" => $stepKey, + "someAttribute" => "someAttributeValue" + ] + ]; + return $baseArray; + } + + /** + * clean up function runs after all tests + */ + public static function tearDownAfterClass() + { + TestLoggingUtil::getInstance()->clearMockLoggingUtil(); + } +} diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionObjectExtractorTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionObjectExtractorTest.php index 7872b3a02..1d645ab62 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionObjectExtractorTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/ActionObjectExtractorTest.php @@ -50,7 +50,7 @@ public function testInvalidMergeOrderReference() } catch (\Exception $e) { TestLoggingUtil::getInstance()->validateMockLogStatement( 'error', - 'Line 103: Invalid ordering configuration in test', + 'Line 108: Invalid ordering configuration in test', [ 'test' => 'TestWithSelfReferencingStepKey', 'stepKey' => ['invalidTestAction1'] @@ -89,6 +89,15 @@ public function testAmbiguousMergeOrderReference() ); } + /** + * Tests basic action object extraction with an empty stepKey + */ + public function testEmptyStepKey() + { + $this->expectExceptionMessage("StepKeys cannot be empty. Action='sampleAction'"); + $this->testActionObjectExtractor->extractActions($this->createBasicActionObjectArray("")); + } + /** * Utility function to return mock parser output for testing extraction into ActionObjects. * 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..86828e9a5 --- /dev/null +++ b/dev/tests/verification/Tests/SchemaValidationTest.php @@ -0,0 +1,42 @@ + true]); + $testFile = ['testFile.xml' => "a"]; + $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(); + } +} 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 e287a84d9..c241e9c62 100644 --- a/src/Magento/FunctionalTestingFramework/Config/Reader/Filesystem.php +++ b/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)); + } + } + } } diff --git a/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php b/src/Magento/FunctionalTestingFramework/Config/Reader/MftfFilesystem.php index 1106ec9b7..e09e40290 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; @@ -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) { diff --git a/src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php b/src/Magento/FunctionalTestingFramework/Console/GenerateTestsCommand.php index d9ef5dea5..bec3b0b19 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('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 = []; diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/ActionGroupObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Test/Util/ActionGroupObjectExtractor.php index fd8093358..04f739af6 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/ActionGroupObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Test/Util/ActionGroupObjectExtractor.php @@ -62,7 +62,11 @@ public function extractActionGroup($actionGroupData) ); // TODO filename is now available to the ActionGroupObject, integrate this into debug and error statements - $actions = $this->actionObjectExtractor->extractActions($actionData); + try { + $actions = $this->actionObjectExtractor->extractActions($actionData); + } catch (\Exception $error) { + throw new XmlException($error->getMessage() . " in Action Group " . $actionGroupData[self::FILENAME]); + } if (array_key_exists(self::ACTION_GROUP_ARGUMENTS, $actionGroupData)) { $arguments = $this->extractArguments($actionGroupData[self::ACTION_GROUP_ARGUMENTS]); diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php index 4ffd70ecf..d157ca5d6 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php +++ b/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php @@ -27,6 +27,7 @@ class ActionObjectExtractor extends BaseObjectExtractor const ACTION_GROUP_ARG_VALUE = 'value'; const BEFORE_AFTER_ERROR_MSG = "Merge Error - Steps cannot have both before and after attributes.\tStepKey='%s'"; const STEP_KEY_BLACKLIST_ERROR_MSG = "StepKeys cannot contain non alphanumeric characters.\tStepKey='%s'"; + const STEP_KEY_EMPTY_ERROR_MSG = "StepKeys cannot be empty.\tAction='%s'"; const DATA_PERSISTENCE_CUSTOM_FIELD = 'field'; const DATA_PERSISTENCE_CUSTOM_FIELD_KEY = 'key'; const ACTION_OBJECT_PERSISTENCE_FIELDS = 'customFields'; @@ -59,6 +60,10 @@ public function extractActions($testActions, $testName = null) foreach ($testActions as $actionName => $actionData) { $stepKey = $actionData[self::TEST_STEP_MERGE_KEY]; + if (empty($stepKey)) { + throw new XmlException(sprintf(self::STEP_KEY_EMPTY_ERROR_MSG, $actionData['nodeName'])); + } + if (preg_match('/[^a-zA-Z0-9_]/', $stepKey)) { throw new XmlException(sprintf(self::STEP_KEY_BLACKLIST_ERROR_MSG, $actionName)); }