Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

33306: Eliminated AspectMock from the ParallelGroupSorterTest class #857

Merged
merged 4 commits into from
Aug 9, 2021
Merged

33306: Eliminated AspectMock from the ParallelGroupSorterTest class #857

merged 4 commits into from
Aug 9, 2021

Conversation

bohdan-harniuk
Copy link
Contributor

Description

Eliminated usage of AspectMock from the \tests\unit\Magento\FunctionalTestFramework\Util\Sorter\ParallelGroupSorterTest.

Fixed Issues (if relevant)

  1. [MFTF] Replace AspectMock with PHPUnit for ParallelGroupSorterTest magento2#33306: [MFTF] Replace AspectMock with PHPUnit for ParallelGroupSorterTest

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 backwards-incompatible changes for tests or have related Pull Request with fixes to tests

@magento-engcom-team magento-engcom-team added Partner: Atwix partners-contribution Pull Request is created by Magento Partner labels Jul 22, 2021
@bohdan-harniuk
Copy link
Contributor Author

Hello, @jilu1

This task was finished without creating new singletons, so it is not a blocker. You can proceed with the code review.

Thanks, Bohdan

@andrewbess andrewbess self-assigned this Jul 22, 2021
andrewbess
andrewbess previously approved these changes Jul 22, 2021
View reviewed changes
Copy link

@andrewbess andrewbess left a comment

Choose a reason for hiding this comment

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

Hello @bohdan-harniuk
Thank you for your contribution.
Great job.

@bohdan-harniuk bohdan-harniuk mentioned this pull request Jul 26, 2021
Merged
5 tasks
@anzin anzin mentioned this pull request Jul 26, 2021
Merged
5 tasks
@okolesnyk okolesnyk self-assigned this Jul 30, 2021
okolesnyk
okolesnyk previously approved these changes Aug 3, 2021
View reviewed changes
@okolesnyk
Copy link
Member

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

@magento-engcom-team
Copy link

@okolesnyk the pull request successfully imported.

jilu1
jilu1 suggested changes Aug 5, 2021
View reviewed changes
Copy link
Contributor

@jilu1 jilu1 left a comment

Choose a reason for hiding this comment

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

Please fix the following unit test error.

1) tests\unit\Magento\FunctionalTestFramework\Util\TestGeneratorTest::testEntityException
Error: Call to a member function readTestData() on null

/home/runner/work/magento2-functional-testing-framework/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Test/Handlers/TestObjectHandler.php:220
/home/runner/work/magento2-functional-testing-framework/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Test/Handlers/TestObjectHandler.php:65
/home/runner/work/magento2-functional-testing-framework/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php:391
/home/runner/work/magento2-functional-testing-framework/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Util/TestGenerator.php:244
/home/runner/work/magento2-functional-testing-framework/magento2-functional-testing-framework/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php:60

ERRORS!
Tests: 292, Assertions: 569, Errors: 1, Skipped: 1, Incomplete: 4.

@jilu1
Copy link
Contributor

jilu1 commented Aug 5, 2021

Please update with the latest develop branch and re-run unit tests. Let us know if you can reproduce the error.

@bohdan-harniuk
Copy link
Contributor Author

Hello, @jilu1!

I'll look at it, thank you!

@bohdan-harniuk
Merge branch 'develop' of github.com:magento/magento2-functional-test…
27f0205 
…ing-framework into improvement/mftf-33306-eliminate-aspect-mock-from-paraller-group-sorter-test
@bohdan-harniuk
MFTF-33306: Fixed error after branch actualization
d98d930
@bohdan-harniuk bohdan-harniuk dismissed stale reviews from okolesnyk and andrewbess via d98d930 August 9, 2021 12:03
@bohdan-harniuk
Copy link
Contributor Author

bohdan-harniuk commented Aug 9, 2021 •
edited
Loading

Hello, @jilu1!

I've found and fixed that error after the branch actualisation.
Please, proceed with the review.

Thanks, Bohdan

@jilu1 jilu1 removed the needs update label Aug 9, 2021
jilu1
jilu1 reviewed Aug 9, 2021
View reviewed changes
dev/tests/unit/Magento/FunctionalTestFramework/Util/Sorter/ParallelGroupSorterTest.php Outdated
)->make();

AspectMock::double(TestObjectHandler::class, ['getInstance' => $mockHandler])->make();
$this->createMockForTest(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

$actionCount ([300, 275]) for this test is different from what's in createMockForTest() ([300, 275, 300, 275]). Please check if affects the purpose of this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, @jilu1, for the great point!
I guess that I missed this one. Now it is fixed. Please, check!

jilu1
jilu1 suggested changes Aug 9, 2021
View reviewed changes
Copy link
Contributor

@jilu1 jilu1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@bohdan-harniuk
Changes look good. Please check one comment inlined.

@bohdan-harniuk bohdan-harniuk requested a review from jilu1 August 9, 2021 18:12
@jilu1 jilu1 removed the needs update label Aug 9, 2021
@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 3fb5eef into magento:develop Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jilu1 jilu1 jilu1 approved these changes

@okolesnyk okolesnyk okolesnyk left review comments

@andrewbess andrewbess Awaiting requested review from andrewbess

Assignees

@okolesnyk okolesnyk

@andrewbess andrewbess

Labels
in progress Partner: Atwix partners-contribution Pull Request is created by Magento Partner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bohdan-harniuk @okolesnyk @magento-engcom-team @jilu1 @andrewbess @magento-devops-reposync-svc