Skip to content

Conversation

Nazar65
Copy link
Member

@Nazar65 Nazar65 commented Oct 18, 2019

Description (*)

Cover \Magento\AdobeStockImageAdminUi\Ui\Component\Listing\Filter\Color with a unit test

Fixed Issues (if relevant)

  1. [Unit tests] Cover \Magento\AdobeStockImageAdminUi\Ui\Component\Listing\Filter\Color with a unit test #491: Cover \Magento\AdobeStockImageAdminUi\Ui\Component\Listing\Filter\Color with a unit test

Manual testing scenarios (*)

Run unit tests

Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Nazar65 thanks for the pull requests, please see review comments

['name' => 'name_one', 'config/colorFormat' => 'format_one']
);

$reflection = new \ReflectionClass($this->color);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pass filterData by mocking Context->getFiltersParams method instead of modifying the test class with reflection.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

*/
public function testPrepare()
{
$componentInterface = $this->createMock(\Magento\Framework\View\Element\UiComponentInterface::class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reorganize the test to make it easier to read: use data provider and extract a couple of private methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@Nazar65
Copy link
Member Author

Nazar65 commented Oct 18, 2019

Hi @sivaschenko done ✔️

@sivaschenko sivaschenko merged commit 59170a8 into magento:develop Oct 20, 2019
@ghost
Copy link

ghost commented Oct 20, 2019

Hi @Nazar65, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants