-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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\Downloadable\Model\Sample\DeleteHandler #26835
Unit test for Magento\Downloadable\Model\Sample\DeleteHandler #26835
Conversation
Hi @karyna-tsymbal-atwix. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
*/ | ||
class DeleteHandlerTest extends TestCase | ||
{ | ||
const STUB_PRODUCT_TYPE = 'simple'; |
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.
Just a recommendation for the future to declare constant scope as private
. It will reduce the chance of "temptation" to make another dependency on the current test in another test.
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 the note, will keep it in mind!
/** | ||
* Test case when provided Product has type Downloadable. | ||
*/ | ||
public function testExecuteWithDownloadableProduct() |
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.
Another advice is to use return type hinting if we declare strict types
in the future. So we have
public function testExecuteWithDownloadableProduct() | |
public function testExecuteWithDownloadableProduct(): void |
Hi @rogyar, thank you for the review. |
Hi @karyna-tsymbal-atwix, thank you for your contribution! |
Description (*)
Unit test for Magento\Downloadable\Model\Sample\DeleteHandler
Contribution checklist (*)