-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Unit test for Magento\Reports\Observer\CheckoutCartAddProductObserver #26579
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.
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
Unit test for Magento\Reports\Observer\CheckoutCartAddProductObserver #26579
Conversation
Hi @karyna-tsymbal-atwix. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
Hi @aleron75, thank you for the review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karyna-tsymbal-atwix I love the progress that you've made with Tests!
Just a few hints to your tests and then this will be another example of your perfect work! 👍
use PHPUnit\Framework\TestCase; | ||
|
||
/** | ||
* Unit Test for @see CheckoutCartAddProductObserver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of adding @see there? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, it's my old habit :) I've replaced it with a name of the class. Is that ok?
/** | ||
* The case when event has to be successfully saved | ||
*/ | ||
public function testExecute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give more meaningful name to test.
testExecute
is bad practice, as it should explain what we expect to be tested.
For example: testExecuteExpectsSaveCalledOnceForNewProduct()
*/ | ||
private function checkOriginalMethodIsNeverExecuted() | ||
{ | ||
$this->eventSaverMock->expects($this->never()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That actually does not exceeds 80 characters in a row, so you can keep that single-line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution
Hi @lbajsarowicz, thank you for the review. |
✔️ QA Passed |
This is actually addressed. Waiting for fixes to be handled. |
Hi @karyna-tsymbal-atwix, thank you for your contribution! |
Description (*)
Covers Magento\Reports\Observer\CheckoutCartAddProductObserver with a unit test.
Contribution checklist (*)