Skip to content

Commit 760573b

Browse files
KevinBKozanKevinBKozan
authored
MQE-456: persisted.data references break double quotes unnecessarily
- Changed resolveTestVariable to process each arg individually, as opposed to the whole string. - fixed quote breaking issue. - consolidated and refactored code around persisted data replacement.
1 parent ea1c134 commit 760573b

File tree

7 files changed

+306
-41
lines changed

7 files changed

+306
-41
lines changed

‎dev/tests/verification/Resources/PersistedReplacementCest.txt

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
namespace Magento\AcceptanceTest\_generated\Backend;
3+
4+
use Magento\FunctionalTestingFramework\AcceptanceTester;
5+
use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler;
6+
use Magento\FunctionalTestingFramework\DataGenerator\Persist\DataPersistenceHandler;
7+
use Magento\FunctionalTestingFramework\DataGenerator\Objects\EntityDataObject;
8+
use Yandex\Allure\Adapter\Annotation\Features;
9+
use Yandex\Allure\Adapter\Annotation\Stories;
10+
use Yandex\Allure\Adapter\Annotation\Title;
11+
use Yandex\Allure\Adapter\Annotation\Description;
12+
use Yandex\Allure\Adapter\Annotation\Parameter;
13+
use Yandex\Allure\Adapter\Annotation\Severity;
14+
use Yandex\Allure\Adapter\Model\SeverityLevel;
15+
use Yandex\Allure\Adapter\Annotation\TestCaseId;
16+
17+
/**
18+
*/
19+
class PersistedReplacementCest
20+
{
21+
/**
22+
* @var DataPersistenceHandler $createData1;
23+
*/
24+
protected $createData1;
25+
26+
public function _before(AcceptanceTester $I)
27+
{
28+
$I->amGoingTo("create entity that has the mergeKey: createData1");
29+
$replacementPerson = DataObjectHandler::getInstance()->getObject("replacementPerson");
30+
$this->createData1 = new DataPersistenceHandler($replacementPerson);
31+
$this->createData1->createEntity();
32+
}
33+
34+
/**
35+
* @Parameter(name = "AcceptanceTester", value="$I")
36+
* @param AcceptanceTester $I
37+
* @return void
38+
*/
39+
public function PersistedReplacementTest(AcceptanceTester $I)
40+
{
41+
$I->amGoingTo("create entity that has the mergeKey: testScopeData");
42+
$replacementPerson = DataObjectHandler::getInstance()->getObject("replacementPerson");
43+
$testScopeData = new DataPersistenceHandler($replacementPerson);
44+
$testScopeData->createEntity();
45+
$I->amGoingTo("create entity that has the mergeKey: uniqueData");
46+
$uniquePerson = DataObjectHandler::getInstance()->getObject("uniquePerson");
47+
$uniqueData = new DataPersistenceHandler($uniquePerson);
48+
$uniqueData->createEntity();
49+
$I->amOnPage("/success/success2.html");
50+
$I->amOnPage($testScopeData->getCreatedDataByName('firstname') . ".html");
51+
$I->amOnPage($this->createData1->getCreatedDataByName('firstname') . ".html");
52+
$I->amOnPage("/" . $testScopeData->getCreatedDataByName('firstname') . "/" . $testScopeData->getCreatedDataByName('lastname') . ".html");
53+
$I->amOnPage("/" . $this->createData1->getCreatedDataByName('firstname') . "/" . $this->createData1->getCreatedDataByName('lastname') . ".html");
54+
$I->click("#element ." . $testScopeData->getCreatedDataByName('firstname'));
55+
$I->click("#" . $testScopeData->getCreatedDataByName('firstname') . " .success");
56+
$I->click("#John-Doe ." . $testScopeData->getCreatedDataByName('lastname'));
57+
$I->click("#" . $testScopeData->getCreatedDataByName('firstname') . " ." . $testScopeData->getCreatedDataByName('lastname'));
58+
$I->click("#" . $this->createData1->getCreatedDataByName('firstname') . " ." . $this->createData1->getCreatedDataByName('lastname'));
59+
$I->fillField("#sample", "Hello " . $testScopeData->getCreatedDataByName('firstname') . " " . $testScopeData->getCreatedDataByName('lastname'));
60+
$I->fillField("#sample", "Hello " . $this->createData1->getCreatedDataByName('firstname') . " " . $this->createData1->getCreatedDataByName('lastname'));
61+
$I->searchAndMultiSelectOption("#selector", [$testScopeData->getCreatedDataByName('lastname')]);
62+
$I->searchAndMultiSelectOption("#selector", [$this->createData1->getCreatedDataByName('lastname')]);
63+
$I->amOnPage($uniqueData->getCreatedDataByName('firstname') . ".html");
64+
$I->amOnPage("/" . $uniqueData->getCreatedDataByName('firstname') . "/" . $uniqueData->getCreatedDataByName('lastname') . ".html");
65+
$I->click("#element ." . $uniqueData->getCreatedDataByName('firstname'));
66+
$I->click("#" . $uniqueData->getCreatedDataByName('firstname') . " .success");
67+
$I->click("#" . $uniqueData->getCreatedDataByName('firstname'));
68+
}
69+
}

‎dev/tests/verification/TestModule/Cest/persistedReplacementCest.xml

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
9+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:noNamespaceSchemaLocation="../../../../../src/Magento/FunctionalTestingFramework/Test/etc/testSchema.xsd">
11+
<cest name="PersistedReplacementCest">
12+
<before>
13+
<createData entity="replacementPerson" mergeKey="createData1"/>
14+
</before>
15+
<test name="PersistedReplacementTest">
16+
<createData entity="replacementPerson" mergeKey="testScopeData"/>
17+
<createData entity="uniquePerson" mergeKey="uniqueData"/>
18+
19+
<!-- parameterized url that uses literal params -->
20+
<amOnPage url="{{SamplePage.url('success','success2')}}" mergeKey="a0"/>
21+
22+
<!-- url referencing data that was created in this <test> -->
23+
<amOnPage url="$testScopeData.firstname$.html" mergeKey="a1"/>
24+
25+
<!-- url referencing data that was created in a <before> -->
26+
<amOnPage url="$$createData1.firstname$$.html" mergeKey="a2"/>
27+
28+
<!--parameterized url that uses created data params-->
29+
<amOnPage url="{{SamplePage.url($testScopeData.firstname$,$testScopeData.lastname$)}}" mergeKey="a3"/>
30+
<amOnPage url="{{SamplePage.url($$createData1.firstname$$,$$createData1.lastname$$)}}" mergeKey="a4"/>
31+
32+
<!-- parameterized selector that uses literal params -->
33+
<click selector="{{SampleSection.oneParamElement($testScopeData.firstname$)}}" mergeKey="c1"/>
34+
<click selector="{{SampleSection.twoParamElement($testScopeData.firstname$,'success')}}" mergeKey="c2"/>
35+
36+
<!-- parameterized selector with literal, static data, and created data -->
37+
<click selector="{{SampleSection.threeParamElement('John', replacementPerson.lastname, $testScopeData.lastname$)}}"
38+
mergeKey="c3"/>
39+
40+
<!-- selector that uses created data -->
41+
<click selector="#$testScopeData.firstname$ .$testScopeData.lastname$" mergeKey="c4"/>
42+
<click selector="#$$createData1.firstname$$ .$$createData1.lastname$$" mergeKey="c5"/>
43+
44+
<!-- userInput that uses created data -->
45+
<fillField selector="#sample" userInput="Hello $testScopeData.firstname$ $testScopeData.lastname$"
46+
mergeKey="f1"/>
47+
<fillField selector="#sample" userInput="Hello $$createData1.firstname$$ $$createData1.lastname$$"
48+
mergeKey="f2"/>
49+
50+
<!-- parameterArray replacement-->
51+
<searchAndMultiSelectOption mergeKey="g1" selector="#selector" parameterArray="[$testScopeData.lastname$]"/>
52+
<searchAndMultiSelectOption mergeKey="g2" selector="#selector" parameterArray="[$$createData1.lastname$$]"/>
53+
54+
<!-- uniqueData replacement -->
55+
<amOnPage url="$uniqueData.firstname$.html" mergeKey="h1"/>
56+
<amOnPage url="{{SamplePage.url($uniqueData.firstname$,$uniqueData.lastname$)}}" mergeKey="h2"/>
57+
<click selector="{{SampleSection.oneParamElement($uniqueData.firstname$)}}" mergeKey="h3"/>
58+
<click selector="{{SampleSection.twoParamElement($uniqueData.firstname$,'success')}}" mergeKey="h4"/>
59+
<click selector="#$uniqueData.firstname$" mergeKey="h5"/>
60+
</test>
61+
</cest>
62+
</config>

‎dev/tests/verification/TestModule/Data/persistedReplacementData.xml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
/**
5+
* Copyright © Magento, Inc. All rights reserved.
6+
* See COPYING.txt for license details.
7+
*/
8+
-->
9+
10+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
11+
xsi:noNamespaceSchemaLocation="../../../../../src/Magento/FunctionalTestingFramework/DataGenerator/etc/dataProfileSchema.xsd">
12+
<entity name="replacementPerson" type="samplePerson">
13+
<data key="firstname">John</data>
14+
<data key="lastName">Doe</data>
15+
</entity>
16+
<entity name="uniquePerson" type="samplePerson">
17+
<data key="firstname" unique="suffix">John</data>
18+
<data key="lastName">Doe</data>
19+
</entity>
20+
</config>

‎dev/tests/verification/TestModule/Page/SamplePage.xml

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
9+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:noNamespaceSchemaLocation="../../../../../../vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Page/etc/PageObject.xsd">
11+
<page name="SamplePage" urlPath="/{{var1}}/{{var2}}.html" module="SampleTests" parameterized="true">
12+
<section name="SampleSection"/>
13+
</page>
14+
</config>

‎dev/tests/verification/TestModule/Section/SampleSection.xml

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
9+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:noNamespaceSchemaLocation="../../../../../../vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Page/etc/SectionObject.xsd">
11+
<section name="SampleSection">
12+
<element name="oneParamElement" type="button" selector="#element .{{var1}}" parameterized="true"/>
13+
<element name="twoParamElement" type="button" selector="#{{var1}} .{{var2}}" parameterized="true"/>
14+
<element name="threeParamElement" type="button" selector="#{{var1}}-{{var2}} .{{var3}}" parameterized="true"/>
15+
<element name="timeoutElement" type="button" selector="#foo" timeout="30"/>
16+
</section>
17+
</config>

‎dev/tests/verification/Tests/PersistedReplacementGenerationTest.php

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace tests\verification\Tests;
7+
8+
use Magento\FunctionalTestingFramework\Test\Handlers\CestObjectHandler;
9+
use Magento\FunctionalTestingFramework\Util\TestGenerator;
10+
use PHPUnit\Framework\TestCase;
11+
use tests\verification\Util\FileDiffUtil;
12+
13+
class PersistedReplacementGenerationTest extends TestCase
14+
{
15+
const PERSISTED_REPLACEMENT_CEST = 'PersistedReplacementCest';
16+
const RESOURCES_PATH = __DIR__ . '/../Resources';
17+
18+
/**
19+
* Tests flat generation of a hardcoded cest file with no external references.
20+
*/
21+
public function testPersistedReplacementGeneration()
22+
{
23+
$cest = CestObjectHandler::getInstance()->getObject(self::PERSISTED_REPLACEMENT_CEST);
24+
$test = TestGenerator::getInstance(null, [$cest]);
25+
$test->createAllCestFiles();
26+
27+
$cestFile = $test->getExportDir() .
28+
DIRECTORY_SEPARATOR .
29+
self::PERSISTED_REPLACEMENT_CEST .
30+
".php";
31+
32+
$this->assertTrue(file_exists($cestFile));
33+
34+
$fileDiffUtil = new FileDiffUtil(
35+
self::RESOURCES_PATH . DIRECTORY_SEPARATOR . self::PERSISTED_REPLACEMENT_CEST . ".txt",
36+
$cestFile
37+
);
38+
39+
$diffResult = $fileDiffUtil->diffContents();
40+
$this->assertNull($diffResult, $diffResult);
41+
}
42+
}

‎src/Magento/FunctionalTestingFramework/Util/TestGenerator.php

+82-41
Original file line numberDiff line numberDiff line change
@@ -876,57 +876,102 @@ private function generateStepsPhp($stepsObject, $stepsData, $hookObject = false)
876876
}
877877

878878
/**
879-
* Resolves replacement of $input$ and $$input$$ in given string.
880-
* Can be given a boolean to surround replacement with quote breaking.
879+
* Resolves replacement of $input$ and $$input$$ in given function, recursing and replacing individual arguments
880+
* Also determines if each argument requires any quote replacement.
881881
* @param string $inputString
882-
* @param bool $quoteBreak
882+
* @param array $args
883883
* @return string
884-
* @throws \Exception
885884
*/
886-
private function resolveTestVariable($inputString, $quoteBreak = false)
885+
private function resolveTestVariable($inputString, $args)
887886
{
888887
$outputString = $inputString;
889-
$replaced = false;
890888

891-
// Check for Cest-scope variables first, stricter regex match.
892-
preg_match_all("/\\$\\$[\w.\[\]]+\\$\\$/", $outputString, $matches);
893-
foreach ($matches[0] as $match) {
894-
$replacement = null;
895-
$variable = $this->stripAndSplitReference($match, '$$');
896-
if (count($variable) != 2) {
897-
throw new \Exception(
898-
"Invalid Persisted Entity Reference: " . $match .
899-
". Hook persisted entity references must follow \$\$entityMergeKey.field\$\$ format."
900-
);
901-
}
902-
$replacement = sprintf("\$this->%s->getCreatedDataByName('%s')", $variable[0], $variable[1]);
903-
if ($quoteBreak) {
904-
$replacement = '" . ' . $replacement . ' . "';
905-
}
906-
$outputString = str_replace($match, $replacement, $outputString);
907-
$replaced = true;
889+
//Loop through each argument, replace and then replace
890+
foreach ($args as $arg) {
891+
$outputArg = $arg;
892+
// Match on any $$data.key$$ found inside arg, matches[0] will be array of $$data.key$$
893+
preg_match_all("/\\$\\$[\w.\[\]]+\\$\\$/", $outputArg, $matches);
894+
$this->replaceMatchesIntoArg($matches[0], $outputArg, "$$");
895+
896+
// Match on any $data.key$ found inside arg, matches[0] will be array of $data.key$
897+
preg_match_all("/\\$[\w.\[\]]+\\$/", $outputArg, $matches);
898+
$this->replaceMatchesIntoArg($matches[0], $outputArg, "$");
899+
900+
$outputString = str_replace($arg, $outputArg, $outputString);
908901
}
909902

910-
// Check Test-scope variables
911-
preg_match_all("/\\$[\w.\[\]]+\\$/", $outputString, $matches);
912-
foreach ($matches[0] as $match) {
903+
return $outputString;
904+
}
905+
906+
/**
907+
* Replaces all matches into given outputArg with. Variable scope determined by delimiter given
908+
* @param array $matches
909+
* @param string &$outputArg
910+
* @param string $delimiter
911+
* @return void
912+
* @throws \Exception
913+
*/
914+
private function replaceMatchesIntoArg($matches, &$outputArg, $delimiter)
915+
{
916+
foreach ($matches as $match) {
913917
$replacement = null;
914-
$variable = $this->stripAndSplitReference($match, '$');
918+
$variable = $this->stripAndSplitReference($match, $delimiter);
915919
if (count($variable) != 2) {
916920
throw new \Exception(
917-
"Invalid Persisted Entity Reference: " . $match .
918-
". Test persisted entity references must follow \$entityMergeKey.field\$ format."
921+
"Invalid Persisted Entity Reference: {$match}.
922+
Test persisted entity references must follow {$delimiter}entityMergeKey.field{$delimiter} format."
919923
);
920924
}
921-
$replacement = sprintf("$%s->getCreatedDataByName('%s')", $variable[0], $variable[1]);
922-
if ($quoteBreak) {
923-
$replacement = '" . ' . $replacement . ' . "';
925+
if ($delimiter == "$") {
926+
$replacement = sprintf("$%s->getCreatedDataByName('%s')", $variable[0], $variable[1]);
927+
} elseif ($delimiter == "$$") {
928+
$replacement = sprintf("\$this->%s->getCreatedDataByName('%s')", $variable[0], $variable[1]);
929+
}
930+
931+
//Determine if quoteBreak check is necessary. Assume replacement is surrounded in quotes, then override
932+
if (strpos($outputArg, "\"") !== false) {
933+
$outputArg = $this->processQuoteBreaks($match, $outputArg, $replacement);
934+
} else {
935+
$outputArg = str_replace($match, $replacement, $outputArg);
924936
}
925-
$outputString = str_replace($match, $replacement, $outputString);
926-
$replaced = true;
927937
}
938+
}
928939

929-
return $outputString;
940+
/**
941+
* Processes an argument for $data.key$ and determines if it needs quote breaks on either ends.
942+
* Returns an output with quote breaks and replacement already done.
943+
* @param string $match
944+
* @param string $argument
945+
* @param string $replacement
946+
* @return string
947+
*/
948+
private function processQuoteBreaks($match, $argument, $replacement)
949+
{
950+
$outputArg = $argument;
951+
$beforeIndex = strpos($outputArg, $match) - 1;
952+
$afterIndex = $beforeIndex + strlen($match) + 1;
953+
$quoteBefore = true;
954+
$quoteAfter = true;
955+
956+
// Prepare replacement with quote breaks if needed
957+
if ($argument[$beforeIndex] != "\"") {
958+
$replacement = '" . ' . $replacement;
959+
$quoteBefore = false;
960+
}
961+
if ($argument[$afterIndex] != "\"") {
962+
$replacement = $replacement . ' . "';
963+
$quoteAfter = false;
964+
}
965+
//Remove quotes at either end of argument if they aren't necessary.
966+
if ($quoteBefore) {
967+
$outputArg = substr($outputArg, 0, $beforeIndex) . substr($outputArg, $beforeIndex+1);
968+
$afterIndex--;
969+
}
970+
if ($quoteAfter) {
971+
$outputArg = substr($outputArg, 0, $afterIndex) . substr($outputArg, $afterIndex+1);
972+
}
973+
$outputArg = str_replace($match, $replacement, $outputArg);
974+
return $outputArg;
930975
}
931976

932977
/**
@@ -1226,9 +1271,7 @@ private function wrapFunctionCall($actor, $action, ...$args)
12261271
}
12271272
$output .= ");\n";
12281273

1229-
// TODO put in condiional to prevent unncessary quote break (i.e. there are no strings to be appended to
1230-
// variable call.
1231-
return $this->resolveTestVariable($output, true);
1274+
return $this->resolveTestVariable($output, $args);
12321275
}
12331276

12341277
/**
@@ -1256,9 +1299,7 @@ private function wrapFunctionCallWithReturnValue($returnVariable, $actor, $actio
12561299
}
12571300
$output .= ");\n";
12581301

1259-
// TODO put in condiional to prevent unncessary quote break (i.e. there are no strings to be appended to
1260-
// variable call.
1261-
return $output = $this->resolveTestVariable($output, true);
1302+
return $this->resolveTestVariable($output, $args);
12621303
}
12631304
// @codingStandardsIgnoreEnd
12641305
}

0 commit comments

Comments
 (0)