From cce89dd62fe40287b1fa2cb256d09cb05e58f830 Mon Sep 17 00:00:00 2001 From: Alex Calandra Date: Mon, 23 Jul 2018 14:35:32 -0500 Subject: [PATCH 1/5] MQE-1151: Extending a skipped test fails to generate the parent before and after actions - Added parameter to getHooks to return the hooks if the value is true - Added a verification test for the topic --- .../Resources/ExtendingSkippedTest.txt | 66 +++++++++++++++++++ .../Test/ExtendedFunctionalTest.xml | 31 ++++++++- .../Tests/ExtendedGenerationTest.php | 11 ++++ .../Test/Objects/TestObject.php | 5 +- .../Test/Util/ObjectExtensionUtil.php | 3 +- 5 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 dev/tests/verification/Resources/ExtendingSkippedTest.txt diff --git a/dev/tests/verification/Resources/ExtendingSkippedTest.txt b/dev/tests/verification/Resources/ExtendingSkippedTest.txt new file mode 100644 index 000000000..35cd3a825 --- /dev/null +++ b/dev/tests/verification/Resources/ExtendingSkippedTest.txt @@ -0,0 +1,66 @@ +amOnPage("/beforeUrl"); + } + + /** + * @param AcceptanceTester $I + * @throws \Exception + */ + public function _after(AcceptanceTester $I) + { + $I->amOnPage("/afterUrl"); + } + + /** + * @param AcceptanceTester $I + * @throws \Exception + */ + public function _failed(AcceptanceTester $I) + { + $I->saveScreenshot(); + } + + /** + * @Severity(level = SeverityLevel::CRITICAL) + * @Features({"TestModule"}) + * @Stories({"Child"}) + * @Parameter(name = "AcceptanceTester", value="$I") + * @param AcceptanceTester $I + * @return void + * @throws \Exception + */ + public function ExtendingSkippedTest(AcceptanceTester $I) + { + $I->comment("text"); + $I->comment("child"); + } +} diff --git a/dev/tests/verification/TestModule/Test/ExtendedFunctionalTest.xml b/dev/tests/verification/TestModule/Test/ExtendedFunctionalTest.xml index 602b9a02c..9d50f7197 100644 --- a/dev/tests/verification/TestModule/Test/ExtendedFunctionalTest.xml +++ b/dev/tests/verification/TestModule/Test/ExtendedFunctionalTest.xml @@ -116,7 +116,6 @@ - @@ -129,4 +128,34 @@ + + + + + <group value="Parent"/> + <features value="Parent"/> + <stories value="Parent"/> + <skip> + <issueId value="NONE"/> + </skip> + </annotations> + <before> + <amOnPage url="/beforeUrl" stepKey="beforeAmOnPageKey"/> + </before> + <after> + <amOnPage url="/afterUrl" stepKey="afterAmOnPageKey"/> + </after> + <comment userInput="text" stepKey="keepMe"/> + <comment userInput="text" stepKey="replaceMe"/> + </test> + <test name="ExtendingSkippedTest" extends="SkippedParent"> + <annotations> + <severity value="CRITICAL"/> + <title value="ChildExtendedTestSkippedParent"/> + <group value="Child"/> + <features value="Child"/> + <stories value="Child"/> + </annotations> + <comment userInput="child" stepKey="replaceMe"/> + </test> </tests> \ No newline at end of file diff --git a/dev/tests/verification/Tests/ExtendedGenerationTest.php b/dev/tests/verification/Tests/ExtendedGenerationTest.php index d0c9926d0..46246c45b 100644 --- a/dev/tests/verification/Tests/ExtendedGenerationTest.php +++ b/dev/tests/verification/Tests/ExtendedGenerationTest.php @@ -96,4 +96,15 @@ public function testExtendedTestGenerationNoParent() { $this->generateAndCompareTest('ChildExtendedTestNoParent'); } + + /** + * Tests extending a skipped test generation. + * + * @throws \Exception + * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + */ + public function testExtendingSkippedGeneration() + { + $this->generateAndCompareTest('ExtendingSkippedTest'); + } } diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php index 71f5ec36b..1d766c0db 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php @@ -164,12 +164,13 @@ public function getAnnotations() /** * Returns hooks. * + * @param boolean $isExtended * @return TestHookObject[] */ - public function getHooks() + public function getHooks($isExtended = false) { // if this test is skipped we do not want any before/after actions to generate as the tests will not run - if ($this->isSkipped()) { + if ($this->isSkipped() && !$isExtended) { return []; } return $this->hooks; diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php b/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php index 97f67004a..f1efe0b27 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php +++ b/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php @@ -146,7 +146,8 @@ public function extendActionGroup($actionGroupObject) private function resolveExtendedHooks($testObject, $parentTestObject) { $testHooks = $testObject->getHooks(); - $parentHooks = $parentTestObject->getHooks(); + // Send true to getHooks to return even if test is skipped + $parentHooks = $parentTestObject->getHooks(true); // Get the hooks for each Test merge changes from the child hooks to the parent hooks into the child hooks foreach ($testHooks as $key => $hook) { From 55c9ee4b414751dd691384ed2ecd298cd3fc892c Mon Sep 17 00:00:00 2001 From: Alex Calandra <calandra.aj@gmail.com> Date: Tue, 24 Jul 2018 10:47:30 -0500 Subject: [PATCH 2/5] MQE-1153: ExecuteJS javascript escaping matches persisted data - Updated Regular expression for executeJS to be stricter for escaping characters --- dev/tests/verification/Resources/ExecuteJsEscapingTest.txt | 2 ++ dev/tests/verification/TestModule/Test/ExecuteJsTest.xml | 2 ++ src/Magento/FunctionalTestingFramework/Util/TestGenerator.php | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt b/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt index 94dc2cbdb..d1fe0fd2f 100644 --- a/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt +++ b/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt @@ -31,5 +31,7 @@ class ExecuteJsEscapingTestCest { $javaVariableEscape = $I->executeJS("return \$javascriptVariable"); $mftfVariableNotEscaped = $I->executeJS("return {$doNotEscape}"); + $persistedDataNotEscaped = $I->executeJS("return " . $persisted->getCreatedDataByName('data')); + $hookPersistedDataNotEscaped = $I->executeJS("return " . $this->persisted->getCreatedDataByName('data')); } } diff --git a/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml b/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml index 2429fa484..2fdafd792 100644 --- a/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml +++ b/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml @@ -11,5 +11,7 @@ <test name="ExecuteJsEscapingTest"> <executeJS function="return $javascriptVariable" stepKey="javaVariableEscape"/> <executeJS function="return {$doNotEscape}" stepKey="mftfVariableNotEscaped"/> + <executeJS function="return $persisted.data$" stepKey="persistedDataNotEscaped"/> + <executeJS function="return $$persisted.data$$" stepKey="hookPersistedDataNotEscaped"/> </test> </tests> diff --git a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php index af39f6ad1..281603322 100644 --- a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php +++ b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php @@ -620,7 +620,7 @@ public function generateStepsPhp($actionObjects, $hookObject = false, $actor = " } // turn $javaVariable => \$javaVariable but not {$mftfVariable} if ($actionObject->getType() == "executeJS") { - $function = preg_replace('/(?<!{)(\$[\w\d_]+)/', '\\\\$1', $function); + $function = preg_replace('/(?<!{)(\$[A-z._]+)(?![A-z.]*+\$)/', '\\\\$1', $function); } } From e7268e4c3081f9280e1d6f82dfe2811cbcb57f35 Mon Sep 17 00:00:00 2001 From: Alex Calandra <calandra.aj@gmail.com> Date: Tue, 31 Jul 2018 09:40:37 -0500 Subject: [PATCH 3/5] MQE-1151: Extending a skipped test fails to generate the parent before and after actions - Removing paramater from getHooks - Adding test for hooks generation on skipped test - Moving skipped logic to testGenerator --- .../Resources/SkippedTestWithHooks.txt | 38 +++++++++++++++++++ .../TestModule/Test/SkippedTest.xml | 17 +++++++++ .../Tests/SkippedGenerationTest.php | 11 ++++++ .../Test/Objects/TestObject.php | 7 +--- .../Test/Util/ObjectExtensionUtil.php | 3 +- .../Util/TestGenerator.php | 6 ++- 6 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 dev/tests/verification/Resources/SkippedTestWithHooks.txt diff --git a/dev/tests/verification/Resources/SkippedTestWithHooks.txt b/dev/tests/verification/Resources/SkippedTestWithHooks.txt new file mode 100644 index 000000000..d28c61486 --- /dev/null +++ b/dev/tests/verification/Resources/SkippedTestWithHooks.txt @@ -0,0 +1,38 @@ +<?php +namespace Magento\AcceptanceTest\_default\Backend; + +use Magento\FunctionalTestingFramework\AcceptanceTester; +use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler; +use Magento\FunctionalTestingFramework\DataGenerator\Persist\DataPersistenceHandler; +use Magento\FunctionalTestingFramework\DataGenerator\Objects\EntityDataObject; +use Magento\FunctionalTestingFramework\DataGenerator\Handlers\CredentialStore; +use \Codeception\Util\Locator; +use Yandex\Allure\Adapter\Annotation\Features; +use Yandex\Allure\Adapter\Annotation\Stories; +use Yandex\Allure\Adapter\Annotation\Title; +use Yandex\Allure\Adapter\Annotation\Description; +use Yandex\Allure\Adapter\Annotation\Parameter; +use Yandex\Allure\Adapter\Annotation\Severity; +use Yandex\Allure\Adapter\Model\SeverityLevel; +use Yandex\Allure\Adapter\Annotation\TestCaseId; + +/** + * @Title("skippedTestWithHooks") + * @Description("") + */ +class SkippedTestWithHooksCest +{ + /** + * @Stories({"skippedWithHooks"}) + * @Severity(level = SeverityLevel::MINOR) + * @Features({"TestModule"}) + * @Parameter(name = "AcceptanceTester", value="$I") + * @param AcceptanceTester $I + * @return void + * @throws \Exception + */ + public function SkippedTestWithHooks(AcceptanceTester $I, \Codeception\Scenario $scenario) + { + $scenario->skip("This test is skipped due to the following issues:\nSkippedValue"); + } +} diff --git a/dev/tests/verification/TestModule/Test/SkippedTest.xml b/dev/tests/verification/TestModule/Test/SkippedTest.xml index bceb619f8..7641e270e 100644 --- a/dev/tests/verification/TestModule/Test/SkippedTest.xml +++ b/dev/tests/verification/TestModule/Test/SkippedTest.xml @@ -19,6 +19,23 @@ </skip> </annotations> </test> + <test name="SkippedTestWithHooks"> + <annotations> + <stories value="skippedWithHooks"/> + <title value="skippedTestWithHooks"/> + <description value=""/> + <severity value="AVERAGE"/> + <skip> + <issueId value="SkippedValue"/> + </skip> + </annotations> + <before> + <comment userInput="skippedComment" stepKey="beforeComment"/> + </before> + <after> + <comment userInput="skippedComment" stepKey="afterComment"/> + </after> + </test> <test name="SkippedTestTwoIssues"> <annotations> <stories value="skippedMultiple"/> diff --git a/dev/tests/verification/Tests/SkippedGenerationTest.php b/dev/tests/verification/Tests/SkippedGenerationTest.php index 6a65df8e3..2d259e02c 100644 --- a/dev/tests/verification/Tests/SkippedGenerationTest.php +++ b/dev/tests/verification/Tests/SkippedGenerationTest.php @@ -20,6 +20,17 @@ public function testSkippedGeneration() $this->generateAndCompareTest('SkippedTest'); } + /** + * Tests skipped test generation does not generate hooks. + * + * @throws \Exception + * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + */ + public function testSkippedWithHooksGeneration() + { + $this->generateAndCompareTest('SkippedTestWithHooks'); + } + /** * Tests skipped test with multiple issues generation. * diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php index 1d766c0db..30c3a9004 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/TestObject.php @@ -164,15 +164,10 @@ public function getAnnotations() /** * Returns hooks. * - * @param boolean $isExtended * @return TestHookObject[] */ - public function getHooks($isExtended = false) + public function getHooks() { - // if this test is skipped we do not want any before/after actions to generate as the tests will not run - if ($this->isSkipped() && !$isExtended) { - return []; - } return $this->hooks; } diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php b/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php index f1efe0b27..97f67004a 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php +++ b/src/Magento/FunctionalTestingFramework/Test/Util/ObjectExtensionUtil.php @@ -146,8 +146,7 @@ public function extendActionGroup($actionGroupObject) private function resolveExtendedHooks($testObject, $parentTestObject) { $testHooks = $testObject->getHooks(); - // Send true to getHooks to return even if test is skipped - $parentHooks = $parentTestObject->getHooks(true); + $parentHooks = $parentTestObject->getHooks(); // Get the hooks for each Test merge changes from the child hooks to the parent hooks into the child hooks foreach ($testHooks as $key => $hook) { diff --git a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php index af39f6ad1..bcae12c9b 100644 --- a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php +++ b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php @@ -207,7 +207,11 @@ private function assembleTestPhp($testObject) $className = $testObject->getCodeceptionName(); try { - $hookPhp = $this->generateHooksPhp($testObject->getHooks()); + if (!$testObject->isSkipped()) { + $hookPhp = $this->generateHooksPhp($testObject->getHooks()); + } else { + $hookPhp = null; + } $testsPhp = $this->generateTestPhp($testObject); } catch (TestReferenceException $e) { throw new TestReferenceException($e->getMessage() . "\n" . $testObject->getFilename()); From 59862c9458188081f7ca89c1443754168120521a Mon Sep 17 00:00:00 2001 From: Alex Calandra <calandra.aj@gmail.com> Date: Tue, 31 Jul 2018 15:02:41 -0500 Subject: [PATCH 4/5] MQE-1153: ExecuteJS javascript escaping matches persisted data - Updated regex to support an additional use case - Added test for additional use case --- dev/tests/verification/Resources/ExecuteJsEscapingTest.txt | 1 + dev/tests/verification/TestModule/Test/ExecuteJsTest.xml | 1 + src/Magento/FunctionalTestingFramework/Util/TestGenerator.php | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt b/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt index d1fe0fd2f..371eb3d0d 100644 --- a/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt +++ b/dev/tests/verification/Resources/ExecuteJsEscapingTest.txt @@ -33,5 +33,6 @@ class ExecuteJsEscapingTestCest $mftfVariableNotEscaped = $I->executeJS("return {$doNotEscape}"); $persistedDataNotEscaped = $I->executeJS("return " . $persisted->getCreatedDataByName('data')); $hookPersistedDataNotEscaped = $I->executeJS("return " . $this->persisted->getCreatedDataByName('data')); + $addNewAttributeForRule = $I->executeJS("document.querySelector('entity option[value=" . $this->productAttribute->getCreatedDataByName('attribute_code') . "]').setAttribute('selected', 'selected')"); } } diff --git a/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml b/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml index 2fdafd792..b361cdd28 100644 --- a/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml +++ b/dev/tests/verification/TestModule/Test/ExecuteJsTest.xml @@ -13,5 +13,6 @@ <executeJS function="return {$doNotEscape}" stepKey="mftfVariableNotEscaped"/> <executeJS function="return $persisted.data$" stepKey="persistedDataNotEscaped"/> <executeJS function="return $$persisted.data$$" stepKey="hookPersistedDataNotEscaped"/> + <executeJS function="document.querySelector('entity option[value=$$productAttribute.attribute_code$$]').setAttribute('selected', 'selected')" stepKey="addNewAttributeForRule"/> </test> </tests> diff --git a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php index 281603322..13cba9675 100644 --- a/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php +++ b/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php @@ -620,7 +620,7 @@ public function generateStepsPhp($actionObjects, $hookObject = false, $actor = " } // turn $javaVariable => \$javaVariable but not {$mftfVariable} if ($actionObject->getType() == "executeJS") { - $function = preg_replace('/(?<!{)(\$[A-z._]+)(?![A-z.]*+\$)/', '\\\\$1', $function); + $function = preg_replace('/(?<!{)(\$[A-Za-z._]+)(?![A-z.]*+\$)/', '\\\\$1', $function); } } From 5b8dbf86b26bd47d1341f0d7867af5de6f8d9195 Mon Sep 17 00:00:00 2001 From: Kevin Kozan <kkozan@magento.com> Date: Fri, 3 Aug 2018 12:34:10 -0500 Subject: [PATCH 5/5] [2.3.2 - Release] Changelog - Added entry to changelog. --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61a27806b..19849c241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ Magento Functional Testing Framework Changelog ================================================ +2.3.2 +----- +### Fixes +* The `executeJs` `function` no longer escapes persisted variables referenced via `$$persisted.key$$`. +* Extending a test no longer fails to generate the parent `test`'s `before`/`after` blocks if the parent was skipped. + 2.3.1 ----- ### Enhancements