From d210dcd60e42bf81c0e9105400b44b48e0e37075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sat, 11 Jan 2020 14:59:47 +0100 Subject: [PATCH 1/5] #55 Throw Exception when cannot resolve URL attribute --- .../Test/Objects/ActionObject.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php index 53743330e..caa5b2113 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php @@ -404,6 +404,14 @@ private function resolveUrlReference() $url = $this->actionAttributes[ActionObject::ACTION_ATTRIBUTE_URL]; $replacement = $this->findAndReplaceReferences(PageObjectHandler::getInstance(), $url); + + $missingReferences = $this->getMissingReferences($replacement); + if (!empty($missingReferences)) { + throw new TestReferenceException( + sprintf('Can not resolve replacements: "%s"', implode('", "', $missingReferences)) + ); + } + if ($replacement) { $this->resolvedCustomAttributes[ActionObject::ACTION_ATTRIBUTE_URL] = $replacement; $allPages = PageObjectHandler::getInstance()->getAllObjects(); @@ -417,6 +425,20 @@ private function resolveUrlReference() } } + /** + * Returns array of missing references + * + * @param $replacement + * @return array + */ + private function getMissingReferences($replacement): array + { + $mustachePattern = '/({{[\w]+}})|({{[\w]+\.[\w\[\]]+}})|({{[\w]+\.[\w]+\((?(?!}}).)+\)}})/'; + preg_match_all($mustachePattern, $replacement, $matches); + + return array_filter($matches[1], function($match) { return false === strpos($match, '_ENV.'); }); + } + /** * Look up the value for EntityDataObjectName.Key and set it as the corresponding attribute in the resolved custom * attributes. From e06c1883d6f12097383ade28c08773b54690f043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sat, 11 Jan 2020 15:13:53 +0100 Subject: [PATCH 2/5] Amend PHPUnit tests and Static tests --- .../Test/Objects/ActionObjectTest.php | 15 +++------------ .../Test/Objects/ActionObject.php | 6 ++++-- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php index 511c9748f..4954cea19 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php @@ -227,7 +227,7 @@ public function testResolveUrl() } /** - * {{PageObject}} should not be replaced and should elicit a warning in console + * {{PageObject}} should not be replaced and should throw an exception! * * @throws /Exception */ @@ -248,18 +248,9 @@ public function testResolveUrlWithNoAttribute() // Call the method under test $actionObject->resolveReferences(); - // Expect this warning to get generated - TestLoggingUtil::getInstance()->validateMockLogStatement( - "warning", - "page url attribute not found and is required", - ['action' => $actionObject->getType(), 'url' => '{{PageObject}}', 'stepKey' => $actionObject->getStepKey()] - ); + $this->expectExceptionMessage('Can not resolve replacements: "{{PageObject}}"'); - // Verify - $expected = [ - 'url' => '{{PageObject}}' - ]; - $this->assertEquals($expected, $actionObject->getCustomActionAttributes()); + $actionObject->getCustomActionAttributes(); } /** diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php index caa5b2113..67a57031e 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php @@ -428,7 +428,7 @@ private function resolveUrlReference() /** * Returns array of missing references * - * @param $replacement + * @param string $replacement * @return array */ private function getMissingReferences($replacement): array @@ -436,7 +436,9 @@ private function getMissingReferences($replacement): array $mustachePattern = '/({{[\w]+}})|({{[\w]+\.[\w\[\]]+}})|({{[\w]+\.[\w]+\((?(?!}}).)+\)}})/'; preg_match_all($mustachePattern, $replacement, $matches); - return array_filter($matches[1], function($match) { return false === strpos($match, '_ENV.'); }); + return array_filter($matches[1], function ($match) { + return false === strpos($match, '_ENV.'); + }); } /** From 15d61bbd36812a780c9fafc214571d0bfd50a756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sat, 11 Jan 2020 17:29:36 +0100 Subject: [PATCH 3/5] Change Unit Tests to cover the change (including Exception thrown) --- .../Test/Objects/ActionObjectTest.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php index 4954cea19..890083d51 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Test/Objects/ActionObjectTest.php @@ -24,6 +24,8 @@ */ class ActionObjectTest extends MagentoTestCase { + const STUB_PAGE_URL_WITH_NO_ATTRIBUTE = '{{PageObject}}/some/path'; + /** * Before test functionality * @return void @@ -233,10 +235,11 @@ public function testResolveUrl() */ public function testResolveUrlWithNoAttribute() { - // Set up mocks + // Given $actionObject = new ActionObject('merge123', 'amOnPage', [ - 'url' => '{{PageObject}}' + 'url' => self::STUB_PAGE_URL_WITH_NO_ATTRIBUTE ]); + $pageObject = new PageObject('PageObject', '/replacement/url.html', 'Test', [], false, "test"); $pageObjectList = ["PageObject" => $pageObject]; $instance = AspectMock::double( @@ -245,12 +248,15 @@ public function testResolveUrlWithNoAttribute() )->make(); // bypass the private constructor AspectMock::double(PageObjectHandler::class, ['getInstance' => $instance]); - // Call the method under test - $actionObject->resolveReferences(); - + // Expect $this->expectExceptionMessage('Can not resolve replacements: "{{PageObject}}"'); + $expected = [ + 'url' => self::STUB_PAGE_URL_WITH_NO_ATTRIBUTE + ]; - $actionObject->getCustomActionAttributes(); + // When + $actionObject->resolveReferences(); + $this->assertEquals($expected, $actionObject->getCustomActionAttributes()); } /** From fd19fb574c1f4d90a0e6cb7b4232d45e6268022c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sat, 11 Jan 2020 18:03:37 +0100 Subject: [PATCH 4/5] Amend PHPUnit tests --- .../FunctionalTestingFramework/Test/Objects/ActionObject.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php index 67a57031e..cf78fe522 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php @@ -437,7 +437,7 @@ private function getMissingReferences($replacement): array preg_match_all($mustachePattern, $replacement, $matches); return array_filter($matches[1], function ($match) { - return false === strpos($match, '_ENV.'); + return !empty($match) && false === strpos($match, '_ENV.'); }); } From c8a8ba915d977dbe51f1cc6ce49bb1ec06ca3268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Tue, 14 Jan 2020 18:40:43 +0100 Subject: [PATCH 5/5] Improve readability of mustache patterns --- .../Test/Objects/ActionObject.php | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php index cf78fe522..b9e3a3465 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php +++ b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php @@ -76,6 +76,9 @@ class ActionObject const ACTION_ATTRIBUTE_USERINPUT = 'userInput'; const ACTION_TYPE_COMMENT = 'comment'; const INVISIBLE_STEP_ACTIONS = ['retrieveEntityField', 'getSecret']; + const REGEX_SINGLE_GROUP = '[\w]+'; + const REGEX_WITH_INDEX = '[\w]+\.[\w\[\]]+'; + const REGEX_WITH_PARAM = '[\w]+\.[\w]+\((?(?!}}).)+\)'; /** * The unique identifier for the action @@ -433,8 +436,13 @@ private function resolveUrlReference() */ private function getMissingReferences($replacement): array { - $mustachePattern = '/({{[\w]+}})|({{[\w]+\.[\w\[\]]+}})|({{[\w]+\.[\w]+\((?(?!}}).)+\)}})/'; - preg_match_all($mustachePattern, $replacement, $matches); + $matchPatterns = [ + self::REGEX_SINGLE_GROUP, + self::REGEX_WITH_INDEX, + self::REGEX_WITH_PARAM + ]; + + preg_match_all($this->getMustachePattern($matchPatterns), $replacement, $matches); return array_filter($matches[1], function ($match) { return !empty($match) && false === strpos($match, '_ENV.'); @@ -534,10 +542,12 @@ private function stripAndReturnParameters($reference) */ private function findAndReplaceReferences($objectHandler, $inputString) { - //look for parameter area, if so use different regex - $regex = ActionObject::ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN; + $matchPatterns = [ + self::REGEX_WITH_INDEX, + self::REGEX_WITH_PARAM + ]; - preg_match_all($regex, $inputString, $matches); + preg_match_all($this->getMustachePattern($matchPatterns), $inputString, $matches); $outputString = $inputString; @@ -720,7 +730,11 @@ private function resolveParameterization($isParameterized, $replacement, $match, */ private function matchParameterReferences($reference, $parameters) { - preg_match_all('/{{[\w.]+}}/', $reference, $varMatches); + $matchPatterns = [ + self::REGEX_SINGLE_GROUP + ]; + + preg_match_all($this->getMustachePattern($matchPatterns), $reference, $varMatches); $varMatches[0] = array_unique($varMatches[0]); $this->checkParameterCount($varMatches[0], $parameters, $reference); @@ -790,4 +804,15 @@ private function checkParameterCount($matches, $parameters, $reference) ); } } + + /** + * Returns Mustache regex pattern + * + * @param array|null $patterns + * @return string + */ + private function getMustachePattern(array $patterns = []): string + { + return '/({{' .implode('}})|({{', $patterns).'}})/'; + } }