Unit Test to cover class \Magento\Captcha\CustomerData\Captcha #25686

edenduong · 2019-11-21T14:31:58Z

Description (*)

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha

Test getSectionData() when user is login and require captcha

Fixed Issues (if relevant)

Manual testing scenarios (*)

Covered by Unit Test

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 · 2019-11-21T14:32:01Z

Hi @edenduong. 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.3-develop instance - deploy vanilla Magento instance

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

lbajsarowicz · 2019-11-22T04:01:14Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+class CaptchaTest extends \PHPUnit\Framework\TestCase
+{
+    /**
+     * @var CaptchaHelper | \PHPUnit_Framework_MockObject_MockObject


Unnecessary spaces

lbajsarowicz · 2019-11-22T04:03:01Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+    /**
+     * Test getSectionData() when user is login and require captcha
+     */
+    public function testGetSectionData()


The test name should be more explanatory, current is too generic.

edenduong · 2019-11-22T06:02:22Z

Hi @lbajsarowicz: I have changed it. Please check it again.

lbajsarowicz

Several changes, in my opinion, are still necessary. Not crucial ones, but still something that needs to be done.

lbajsarowicz · 2019-11-22T10:59:58Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+        $this->customerSession->expects($this->any())->method('isLoggedIn')
+            ->willReturn(true);
+
+        $this->customerData = $this->createMock(CustomerData::class);


If you are instantiating mock there and using only for this test, use local variable instead of class property.

lbajsarowicz · 2019-11-22T12:58:42Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+ *
+ * Class \Magento\Captcha\Test\Unit\CustomerData\CaptchaTest
+ */
+class CaptchaTest extends \PHPUnit\Framework\TestCase


Import the class.

lbajsarowicz · 2019-11-22T12:58:59Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+/**
+ * Test class to cover \Magento\Captcha\CustomerData\Captcha
+ *
+ * Class \Magento\Captcha\Test\Unit\CustomerData\CaptchaTest


This line is completely redundant.

lbajsarowicz · 2019-11-22T12:59:37Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+    /**
+     * @var CaptchaHelper|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $helper;


It's highly recommended to mark that it's still mock, not the real implementation by renaming the property to helperMock

lbajsarowicz · 2019-11-22T12:59:56Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+    /**
+     * @var CustomerSession|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $customerSession;


Append Mock to name to mark that's not real implementation.

lbajsarowicz · 2019-11-22T13:00:02Z

app/code/Magento/Captcha/Test/Unit/CustomerData/CaptchaTest.php

+    /**
+     * @var CustomerData|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $customerData;


Append Mock to name to mark that's not real implementation.

lbajsarowicz

Now it's fine. Thanks for your contribution!

magento-engcom-team · 2019-11-23T18:32:44Z

Hi @lbajsarowicz, thank you for the review.
ENGCOM-6322 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

engcom-Bravo · 2019-11-24T21:59:20Z

✔️ QA Passed

…aptcha #25686

m2-assistant · 2019-11-26T03:21:57Z

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

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha

129771e

edenduong requested a review from kokoc as a code owner November 21, 2019 14:31

magento-engcom-team added Component: Captcha Release Line: 2.3 tech-partners-contribution Technology Partner: Magestore labels Nov 21, 2019

lbajsarowicz suggested changes Nov 22, 2019

View reviewed changes

ghost assigned lbajsarowicz Nov 22, 2019

ghost added the Progress: needs update label Nov 22, 2019

Refactor to pass review

7534f33

lbajsarowicz suggested changes Nov 22, 2019

View reviewed changes

refactor code to pass review

9992c0e

lbajsarowicz approved these changes Nov 23, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Nov 23, 2019

engcom-Bravo self-assigned this Nov 24, 2019

engcom-Bravo added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Nov 24, 2019

ghost added Progress: testing in progress Progress: extended testing and removed Progress: ready for testing Progress: testing in progress labels Nov 24, 2019

engcom-Bravo added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests and removed Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests labels Nov 24, 2019

engcom-Foxtrot self-assigned this Nov 25, 2019

ghost added Progress: accept and removed Progress: extended testing labels Nov 25, 2019

magento-engcom-team added the Award: test coverage label Nov 25, 2019

magento-engcom-team pushed a commit that referenced this pull request Nov 26, 2019

ENGCOM-6322: Unit Test to cover class \Magento\Captcha\CustomerData\C…

75a3b7d

…aptcha #25686

magento-engcom-team merged commit 9992c0e into magento:2.3-develop Nov 26, 2019

magento-engcom-team added this to the Release: 2.3.5 milestone Nov 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha #25686

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha #25686

edenduong commented Nov 21, 2019

m2-assistant bot commented Nov 21, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

edenduong commented Nov 22, 2019

lbajsarowicz left a comment

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz Nov 22, 2019

lbajsarowicz left a comment

magento-engcom-team commented Nov 23, 2019

engcom-Bravo commented Nov 24, 2019

m2-assistant bot commented Nov 26, 2019

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha #25686

Unit Test to cover class \Magento\Captcha\CustomerData\Captcha #25686

Conversation

edenduong commented Nov 21, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Nov 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edenduong commented Nov 22, 2019

lbajsarowicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbajsarowicz left a comment

Choose a reason for hiding this comment

magento-engcom-team commented Nov 23, 2019

engcom-Bravo commented Nov 24, 2019

m2-assistant bot commented Nov 26, 2019