Skip to content

Commit

Permalink
- Removed create_function() function usage
Browse files Browse the repository at this point in the history
  • Loading branch information
YevSent committed Feb 1, 2018
1 parent bc111f5 commit 7c79007
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2528,4 +2528,5 @@
['_isAttributeValueEmpty', 'Magento\Catalog\Model\ResourceModel\AbstractResource'],
['var_dump', ''],
['each', ''],
['create_function', ''],
];
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\Validator\Test\Unit\Constraint\Option;

/**
* Test case for \Magento\Framework\Validator\Constraint\Option\Callback
*/
use Magento\Framework\Validator\Constraint\Option\Callback;
use Magento\Framework\Validator\Test\Unit\Test\Callback as TestCallback;

class CallbackTest extends \PHPUnit\Framework\TestCase
{
/**
Expand All @@ -28,41 +27,46 @@ class CallbackTest extends \PHPUnit\Framework\TestCase
*/
public function testGetValue($callback, $expectedResult, $arguments = null, $createInstance = false)
{
$option = new \Magento\Framework\Validator\Constraint\Option\Callback($callback, $arguments, $createInstance);
$this->assertEquals($expectedResult, $option->getValue());
$option = new Callback($callback, $arguments, $createInstance);
self::assertEquals($expectedResult, $option->getValue());

This comment has been minimized.

Copy link
@orlangur

orlangur Feb 8, 2018

Contributor

@joni-jones please avoid such kind of changes, they are incorrect.

This comment has been minimized.

Copy link
@YevSent

YevSent Feb 9, 2018

Author Contributor

What kind of changes? self usage instead $this? Why is it incorrect?

This comment has been minimized.

Copy link
@orlangur

orlangur Feb 9, 2018

Contributor

Because they are not intended to be used this way: sebastianbergmann/phpunit#1914 (comment) We do not encourage static calls generally in Magento and calling static method in object context is perfectly valid from PHP perspective (note that opposite is not true and raising E_DEPRECATED in PHP7).

Until there is no sniff to enforce $this feel free to implement new tests with syntax of your choice but please don't spoil changesets with such unrelated changes.

This comment has been minimized.

Copy link
@YevSent

YevSent Feb 9, 2018

Author Contributor

The community reaction is not so unambiguous
screenshot from 2018-02-09 12-57-14

but please don't spoil changesets with such unrelated changes.

Ok, I agree with that.

}

/**
* Data provider for testGetValue
*/
public function getConfigDataProvider()
{
$functionName = create_function('', 'return "Value from function";');
$closure = function () {
return 'Value from closure';
};

$mock = $this->getMockBuilder('Foo')->setMethods(['getValue'])->getMock();
$mock->expects(
$this->once()
)->method(
'getValue'
)->with(
'arg1',
'arg2'
)->will(
$this->returnValue('Value from mock')
);
$mock = $this->getMockBuilder('Foo')
->setMethods(['getValue'])
->getMock();
$mock->method('getValue')
->with('arg1', 'arg2')
->willReturn('Value from mock');

return [
[$functionName, 'Value from function'],
[$closure, 'Value from closure'],
[[$this, 'getTestValue'], self::TEST_VALUE],
[[__CLASS__, 'getTestValueStatically'], self::TEST_VALUE],
[[$mock, 'getValue'], 'Value from mock', ['arg1', 'arg2']],
[
[\Magento\Framework\Validator\Test\Unit\Test\Callback::class, 'getId'],
\Magento\Framework\Validator\Test\Unit\Test\Callback::ID,
$closure,
'Value from closure'
],
[
[$this, 'getTestValue'],
self::TEST_VALUE
],
[
[__CLASS__, 'getTestValueStatically'],
self::TEST_VALUE
],
[
[$mock, 'getValue'],
'Value from mock', ['arg1', 'arg2']
],
[
[TestCallback::class, 'getId'],
TestCallback::ID,
null,
true
]
Expand Down Expand Up @@ -90,25 +94,29 @@ public function getTestValue()
*
* @dataProvider setArgumentsDataProvider
*
* @param mixed $value
* @param mixed $expectedValue
* @param string|array $value
* @param string|array $expectedValue
*/
public function testSetArguments($value, $expectedValue)
{
$option = new \Magento\Framework\Validator\Constraint\Option\Callback(
function () {
}
);
$option = new Callback(function () {
});
$option->setArguments($value);
$this->assertAttributeEquals($expectedValue, '_arguments', $option);
self::assertAttributeEquals($expectedValue, '_arguments', $option);
}

/**
* Data provider for testGetValue
*/
public function setArgumentsDataProvider()
{
return [['baz', ['baz']], [['foo', 'bar'], ['foo', 'bar']]];
return [
['baz', ['baz']],
[
['foo', 'bar'],
['foo', 'bar']
]
];
}

/**
Expand All @@ -119,11 +127,12 @@ public function setArgumentsDataProvider()
* @param mixed $callback
* @param string $expectedMessage
* @param bool $createInstance
* @expectedException \InvalidArgumentException
*/
public function testGetValueException($callback, $expectedMessage, $createInstance = false)
{
$option = new \Magento\Framework\Validator\Constraint\Option\Callback($callback, null, $createInstance);
$this->expectException('InvalidArgumentException', $expectedMessage);
$option = new Callback($callback, null, $createInstance);
self::expectExceptionMessage($expectedMessage);
$option->getValue();
}

Expand All @@ -139,10 +148,22 @@ public function getValueExceptionDataProvider()
['Not_Existing_Callback_Class', 'someMethod'],
'Class "Not_Existing_Callback_Class" was not found',
],
[[$this, 'notExistingMethod'], 'Callback does not callable'],
[['object' => $this, 'method' => 'getTestValue'], 'Callback does not callable'],
['unknown_function', 'Callback does not callable'],
[new \stdClass(), 'Callback does not callable'],
[
[$this, 'notExistingMethod'],
'Callback does not callable'
],
[
['object' => $this, 'method' => 'getTestValue'],
'Callback does not callable'
],
[
'unknown_function',
'Callback does not callable'
],
[
new \stdClass(),
'Callback does not callable'
],
[
[$this, 'getTestValue'],
'Callable expected to be an array with class name as first element',
Expand Down

0 comments on commit 7c79007

Please sign in to comment.