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.

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

Remove zend json from the test suite #10320

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ abstract class AbstractController extends \PHPUnit_Framework_TestCase
protected $_runOptions = [];

/**
* @var \Magento\TestFramework\Request
* @var \Magento\Framework\App\RequestInterface
*/
protected $_request;

/**
* @var \Magento\TestFramework\Response
* @var \Magento\Framework\App\ResponseInterface
*/
protected $_response;

Expand Down Expand Up @@ -102,7 +102,7 @@ public function dispatch($uri)
/**
* Request getter
*
* @return \Magento\TestFramework\Request
* @return \Magento\Framework\App\RequestInterface
*/
public function getRequest()
{
Expand All @@ -115,7 +115,7 @@ public function getRequest()
/**
* Response getter
*
* @return \Magento\TestFramework\Response
* @return \Magento\Framework\App\ResponseInterface
*/
public function getResponse()
{
Expand Down Expand Up @@ -268,14 +268,21 @@ protected function getCookieMessages($messageType = null)
{
/** @var $cookieManager CookieManagerInterface */
$cookieManager = $this->_objectManager->get(CookieManagerInterface::class);

/** @var $jsonSerializer \Magento\Framework\Serialize\Serializer\Json */
$jsonSerializer = $this->_objectManager->get(\Magento\Framework\Serialize\Serializer\Json::class);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I was not sure if I should use the class \Magento\Framework\Serialize\Serializer\Json here or just json_* but I thought since this was using the cookie interface it should also use the serializer. Happy to make a change if it is thought best to do something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

As we suppress exception anyway here I would simplify implementation and got rid of try/catch at all. But as it is just testing code it does not really matter.

try {
$messages = \Zend_Json::decode(
$cookieManager->getCookie(MessagePlugin::MESSAGES_COOKIES_NAME, \Zend_Json::encode([]))
$messages = $jsonSerializer->unserialize(
$cookieManager->getCookie(
MessagePlugin::MESSAGES_COOKIES_NAME,
$jsonSerializer->serialize([])
)
);

if (!is_array($messages)) {
$messages = [];
}
} catch (\Zend_Json_Exception $e) {
} catch (\InvalidArgumentException $e) {
$messages = [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,25 @@ class ControllerAbstractTest extends \Magento\TestFramework\TestCase\AbstractCon
/** @var \PHPUnit_Framework_MockObject_MockObject | CookieManagerInterface */
private $cookieManagerMock;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Serialize\Serializer\Json
*/
private $serializerMock;

protected function setUp()
{
$testObjectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);

$this->messageManager = $this->getMock(\Magento\Framework\Message\Manager::class, [], [], '', false);
$this->cookieManagerMock = $this->getMock(CookieManagerInterface::class, [], [], '', false);
$this->serializerMock = $this->getMockBuilder(\Magento\Framework\Serialize\Serializer\Json::class)
->disableOriginalConstructor()
->getMock();
$this->serializerMock->expects($this->any())->method('unserialize')->willReturnCallback(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I love this approach for mocking methods

Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to think that using real dependency in this case would be a better idea, however we usually stick to mocking all the stuff in the unit tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah when it comes to mocking or not mocking I am not so sure what is "best" but the callback here is a nice idea specifically for this mock.

Copy link
Contributor

Choose a reason for hiding this comment

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

Such "mock" is practically equivalent to a real class 😉 I prefer tiny tests with small fixtures having mocks in form of returning plain values. Copy-pasting such callbacks to dozens of tests somewhat increases maintenance efforts.

function ($serializedData) {
return json_decode($serializedData, true);
}
);
$this->interpretationStrategyMock = $this->getMock(InterpretationStrategyInterface::class, [], [], '', false);
$this->interpretationStrategyMock->expects($this->any())
->method('interpret')
Expand Down Expand Up @@ -58,6 +71,7 @@ function (MessageInterface $message) {
[\Magento\Framework\App\ResponseInterface::class, $response],
[\Magento\Framework\Message\Manager::class, $this->messageManager],
[CookieManagerInterface::class, $this->cookieManagerMock],
[\Magento\Framework\Serialize\Serializer\Json::class, $this->serializerMock],
[InterpretationStrategyInterface::class, $this->interpretationStrategyMock],
]
)
Expand Down Expand Up @@ -244,6 +258,6 @@ private function addSessionMessages()

$this->cookieManagerMock->expects($this->any())
->method('getCookie')
->willReturn(\Zend_Json::encode($cookieMessages));
->willReturn(json_encode($cookieMessages));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function testLoadDefault()
$this->assertNotEmpty($this->model->getTemplateText());
$this->assertNotEmpty($this->model->getTemplateSubject());
$this->assertNotEmpty($this->model->getOrigTemplateVariables());
$this->assertInternalType('array', \Zend_Json::decode($this->model->getOrigTemplateVariables()));
$this->assertInternalType('array', json_decode($this->model->getOrigTemplateVariables(), true));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected function setUp()
public function testGetEntityBehaviors()
{
$actualEntities = $this->_model->getEntityBehaviors();
$expectedEntities = \Zend_Json::encode($this->_expectedEntities);
$expectedEntities = json_encode($this->_expectedEntities);
$this->assertEquals($expectedEntities, $actualEntities);
}

Expand All @@ -105,7 +105,7 @@ public function testGetEntityBehaviors()
public function testGetUniqueBehaviors()
{
$actualBehaviors = $this->_model->getUniqueBehaviors();
$expectedBehaviors = \Zend_Json::encode($this->_expectedBehaviors);
$expectedBehaviors = json_encode($this->_expectedBehaviors);
$this->assertEquals($expectedBehaviors, $actualBehaviors);
}
}