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

Replace deprecated constructs from Magento_Newsletter PHPUnit. #27613

Merged
merged 7 commits into from
Apr 22, 2020
Merged

Replace deprecated constructs from Magento_Newsletter PHPUnit. #27613

merged 7 commits into from
Apr 22, 2020

Conversation

AndreyChorniy
Copy link
Contributor

Description (*)

Added compatibility PHPUnit8 for Magento Newsletter module

Related Pull Requests

Fixed Issues (if relevant)

  1. Unit Tests incompatible with PHPUnit 8 #27500

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Apr 5, 2020

Hi @AndreyChorniy. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

dmytro-ch
dmytro-ch requested changes Apr 5, 2020
View reviewed changes
Copy link
Contributor

@dmytro-ch dmytro-ch left a comment

Choose a reason for hiding this comment

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

Hello @AndreyChorniy, could you please fix the same issues as already mentioned in your adjacent PR #27615.
Thank you! :)

@ghost ghost assigned dmytro-ch Apr 5, 2020
lbajsarowicz
lbajsarowicz previously approved these changes Apr 5, 2020
View reviewed changes
Copy link
Contributor

@lbajsarowicz lbajsarowicz left a comment

Choose a reason for hiding this comment

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

✔️ Huge amount of work, congratulations!

@ghost ghost assigned lbajsarowicz Apr 5, 2020
@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz, thank you for the review.
ENGCOM-7269 has been created to process this Pull Request
✳️ @lbajsarowicz, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

dmytro-ch
dmytro-ch requested changes Apr 5, 2020
View reviewed changes
Copy link
Contributor

@dmytro-ch dmytro-ch left a comment

Choose a reason for hiding this comment

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

The tests still fail.

PHP Fatal error:  Declaration of Magento\Newsletter\Test\Unit\Block\Adminhtml\Queue\PreviewTest::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /home/dev/sites/bravo/app/code/Magento/Newsletter/Test/Unit/Block/Adminhtml/Queue/PreviewTest.php on line 174

@ghost ghost dismissed lbajsarowicz’s stale review April 5, 2020 13:46

Pull Request state was updated. Re-review required.

@AndreyChorniy
Copy link
Contributor Author

Hello @dmytro-ch,
I am in progress with this, it will be fixed in a couple of hrs.

@dmytro-ch
Copy link
Contributor

@magento run all tests

@slavvka slavvka added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Apr 15, 2020
@slavvka slavvka changed the base branch from 2.4-develop to phpunit8 April 16, 2020 14:47
slavvka
slavvka requested changes Apr 17, 2020
View reviewed changes
Copy link
Member

@slavvka slavvka left a comment

Choose a reason for hiding this comment

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

@AndreyChorniy please fix the static test:

PHP Code Mess has found error(s):
/var/www/html/app/code/Magento/Newsletter/Test/Unit/Block/Adminhtml/Template/PreviewTest.php:28	The class PreviewTest has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13.

@ghost ghost assigned slavvka Apr 17, 2020
@slavvka
Copy link
Member

slavvka commented Apr 17, 2020

And please resolve the conflicts

Andrew Chorniy added 3 commits April 22, 2020 10:44
Merge remote-tracking branch 'upstream/phpunit8' into phpunit8/mpdule…
fb20e8f 
…-newsletter

# Conflicts:
#	app/code/Magento/Newsletter/Test/Unit/Block/Adminhtml/Queue/PreviewTest.php
#	app/code/Magento/Newsletter/Test/Unit/Block/Adminhtml/Template/PreviewTest.php
#	app/code/Magento/Newsletter/Test/Unit/Controller/Manage/SaveTest.php
#	app/code/Magento/Newsletter/Test/Unit/Model/ProblemTest.php
#	app/code/Magento/Newsletter/Test/Unit/Model/Queue/TransportBuilderTest.php
#	app/code/Magento/Newsletter/Test/Unit/Model/QueueTest.php
#	app/code/Magento/Newsletter/Test/Unit/Model/Template/FilterTest.php
#	app/code/Magento/Newsletter/Test/Unit/Model/TemplateTest.php
Fix PHPMD issue
12a4cb4
Fix test issue
7208448
@ghost ghost removed the Progress: needs update label Apr 22, 2020
@slavvka slavvka merged commit b2f063a into magento:phpunit8 Apr 22, 2020
@m2-assistant
Copy link

m2-assistant bot commented Apr 22, 2020

Hi @AndreyChorniy, 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.

@magento-engcom-team magento-engcom-team mentioned this pull request May 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytro-ch dmytro-ch dmytro-ch requested changes

@slavvka slavvka slavvka approved these changes

@lbajsarowicz lbajsarowicz lbajsarowicz left review comments

Assignees

@lbajsarowicz lbajsarowicz

@slavvka slavvka

@dmytro-ch dmytro-ch

Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Newsletter Event: Global-Contribution-Day Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AndreyChorniy @magento-engcom-team @dmytro-ch @slavvka @lbajsarowicz