Add ability to use array entities as arguments. #89

nathanjosiah · 2018-04-06T17:24:13Z

Added support for passing array entities as arguments.

Description

As a Test Engineer I want to pass data entity arrays as arguments so that don't have to use hard-coded flat arrays.

For example:

The Entity

<entities>
    <entity name="DefaultCustomerGroup" type="customerGroup">
        <array key="group_names">
            <item>Default</item>
        </array>
    </entity>
</entities>

The ActionGroup

<actionGroups>
    <actionGroup name="searchAndMultiSelectActionGroup">
        <arguments>
            <argument name="options" />
        </arguments>
        <selectMultipleOptions filterSelector="foo" optionSelector="bar" stepKey="selectSpecifiedOptions">
            <array>{{options}}</array>
        </selectMultipleOptions>
    </actionGroup>
</actionGroups>

The Test

<tests>
    <test name="AdminCreateCustomerTest">
        <actionGroup ref="searchAndMultiSelectActionGroup" stepKey="searchAndSelectGroup">
            <argument name="options" value="DefaultCustomerGroup.group_names"/>
        </actionGroup>
    </test>
</tests>

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/verification tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)
Changes to Framework doesn't have backward incompatible changes for tests or have related Pull Request with fixes to tests

magento-cicd2 · 2018-04-06T17:24:16Z

All committers have signed the CLA.

coveralls · 2018-04-06T17:43:16Z

Coverage increased (+9.7%) to 61.012% when pulling 00b21e3 on magento-borg:resolve-array-data into 2fe0d2a on magento:develop.

coveralls · 2018-04-06T17:43:16Z

Coverage increased (+9.7%) to 61.012% when pulling 00b21e3 on magento-borg:resolve-array-data into 2fe0d2a on magento:develop.

coveralls · 2018-04-06T17:43:16Z

Coverage increased (+0.02%) to 50.316% when pulling 9bce276 on magento-borg:resolve-array-data into 8d6eadf on magento:develop.

nathanjosiah · 2018-04-06T18:11:11Z

src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php

@@ -496,6 +496,10 @@ private function findAndReplaceReferences($objectHandler, $inputString)

            $replacement = $this->resolveParameterization($parameterized, $replacement, $match, $obj);

+            if (is_array($replacement)) {
+                $replacement = '["' . implode('","', $replacement) . '"]';


I'm aware this would be insufficient escapement but I wasn't sure how far this needed to go. I figured if nothing else this PR serves as a proof of concept and opens the conversation to this feature.

@nathanjosiah, by insufficient escapement do you mean how it assumes that the array members are strings?

@KevinBKozan Unless multidimensional arrays have a use case then I'm not thinking as much about that. I was thinking more about if the items contain quotes or $. The resulting Codception unit test would potentially be malformed or inadvertently contain PHP variable interpolation.

Gotcha. Let me leave some review comments on some small stuff then!

KevinBKozan

Two small changes requested, but otherwise good work and thank you for diving into it!

KevinBKozan · 2018-04-18T15:01:49Z

src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php

@@ -496,6 +496,10 @@ private function findAndReplaceReferences($objectHandler, $inputString)

            $replacement = $this->resolveParameterization($parameterized, $replacement, $match, $obj);

+            if (is_array($replacement)) {
+                $replacement = '["' . implode('","', $replacement) . '"]';


We should array_walk/array_map with addSlashes to break quotes at the very least. Inadvertend variable interpolation in MFTF is not yet handled, so this is not introducing anything new.

KevinBKozan · 2018-04-18T15:03:37Z

src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php

@@ -496,6 +496,10 @@ private function findAndReplaceReferences($objectHandler, $inputString)

            $replacement = $this->resolveParameterization($parameterized, $replacement, $match, $obj);

+            if (is_array($replacement)) {


This check should either live in the elseif block in 497, or in the resolveEntityDataObjectReference() function. This conditional only ever applies to EntityDataObjects, so checking it every time is unecessary.

nathanjosiah · 2018-04-18T16:07:59Z

@KevinBKozan I updated my branch with the requested changed and merged mainline and ran tests to verify current compatibility.

KevinBKozan

Wasn't very clear in my first feedback, my apologies. Still need to move the new conditional.

KevinBKozan · 2018-04-18T18:41:18Z

src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php

@@ -504,6 +504,9 @@ private function findAndReplaceReferences($objectHandler, $inputString)
                    throw new TestReferenceException("Could not resolve entity reference " . $inputString);
                }
            }
+            elseif (is_array($replacement)) {
+                $replacement = '["' . implode('","', array_map('addSlashes',$replacement)) . '"]';


@nathanjosiah my bad, I don't think I was very clear about this portion. I meant that your addition should be entirely inside the elseif on line 496; that elseif is the portion that deals with the object passed in being an EntityDataObject. Since the only time $replacement could only ever be an array is if it's an array inside an EntityDataObject, we should only every have to check it inside that conditional.

Does that make sense?

@KevinBKozan Ah, yes I see what you mean. I have pushed an updated.

nathanjosiah · 2018-04-18T18:54:26Z

@KevinBKozan I have updated the code and the Travis build is now passing.

KevinBKozan · 2018-04-18T19:29:58Z

@nathanjosiah Awesome. Code additions look good and all the builds are passing, thank you very much for the contribution!

…2-functional-testing-framework-843 [Imported] MFTF-33305: Eliminate AspectMock from ObjectExtensionUtilTest

Added ability to use array entities as arguments.

00b21e3

nathanjosiah commented Apr 6, 2018

View reviewed changes

KevinBKozan suggested changes Apr 18, 2018

View reviewed changes

nathanjosiah added 2 commits April 18, 2018 11:02

Added slash escapement and moved condition for array entity arguments

d3ad955

Merge branch 'develop' into resolve-array-data

2c38cc6

KevinBKozan suggested changes Apr 18, 2018

View reviewed changes

Moved array entity data replacement conditional

9bce276

KevinBKozan approved these changes Apr 18, 2018

View reviewed changes

KevinBKozan merged commit 2bdc905 into magento:develop Apr 19, 2018

nathanjosiah deleted the resolve-array-data branch April 20, 2018 16:40

magento-devops-reposync-svc pushed a commit that referenced this pull request Aug 3, 2021

Merge pull request #89 from magento-commerce/imported-magento-magento…

66a652b

…2-functional-testing-framework-843 [Imported] MFTF-33305: Eliminate AspectMock from ObjectExtensionUtilTest

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to use array entities as arguments. #89

Add ability to use array entities as arguments. #89

nathanjosiah commented Apr 6, 2018 •

edited

Loading

magento-cicd2 commented Apr 6, 2018 •

edited

Loading

coveralls commented Apr 6, 2018

coveralls commented Apr 6, 2018

coveralls commented Apr 6, 2018 •

edited

Loading

nathanjosiah Apr 6, 2018

KevinBKozan Apr 18, 2018

nathanjosiah Apr 18, 2018

KevinBKozan Apr 18, 2018

KevinBKozan left a comment

KevinBKozan Apr 18, 2018

KevinBKozan Apr 18, 2018

nathanjosiah commented Apr 18, 2018

KevinBKozan left a comment

KevinBKozan Apr 18, 2018

nathanjosiah Apr 18, 2018 •

edited

Loading

nathanjosiah commented Apr 18, 2018

KevinBKozan commented Apr 18, 2018

		@@ -496,6 +496,10 @@ private function findAndReplaceReferences($objectHandler, $inputString)

		$replacement = $this->resolveParameterization($parameterized, $replacement, $match, $obj);

		if (is_array($replacement)) {

Add ability to use array entities as arguments. #89

Add ability to use array entities as arguments. #89

Conversation

nathanjosiah commented Apr 6, 2018 • edited Loading

Description

Contribution checklist

magento-cicd2 commented Apr 6, 2018 • edited Loading

coveralls commented Apr 6, 2018

coveralls commented Apr 6, 2018

coveralls commented Apr 6, 2018 • edited Loading

nathanjosiah Apr 6, 2018

Choose a reason for hiding this comment

KevinBKozan Apr 18, 2018

Choose a reason for hiding this comment

nathanjosiah Apr 18, 2018

Choose a reason for hiding this comment

KevinBKozan Apr 18, 2018

Choose a reason for hiding this comment

KevinBKozan left a comment

Choose a reason for hiding this comment

KevinBKozan Apr 18, 2018

Choose a reason for hiding this comment

KevinBKozan Apr 18, 2018

Choose a reason for hiding this comment

nathanjosiah commented Apr 18, 2018

KevinBKozan left a comment

Choose a reason for hiding this comment

KevinBKozan Apr 18, 2018

Choose a reason for hiding this comment

nathanjosiah Apr 18, 2018 • edited Loading

Choose a reason for hiding this comment

nathanjosiah commented Apr 18, 2018

KevinBKozan commented Apr 18, 2018

nathanjosiah commented Apr 6, 2018 •

edited

Loading

magento-cicd2 commented Apr 6, 2018 •

edited

Loading

coveralls commented Apr 6, 2018 •

edited

Loading

nathanjosiah Apr 18, 2018 •

edited

Loading