-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add default callback functionality #42
base: master
Are you sure you want to change the base?
Changes from 4 commits
55ffe59
5d35a4c
139e6d5
8feeeb6
84e6436
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
namespace LaraCrafts\GeoRoutes\Tests\Unit; | ||
|
||
use Closure; | ||
use InvalidArgumentException; | ||
use LaraCrafts\GeoRoutes\CallbackRegistrar; | ||
use LaraCrafts\GeoRoutes\Tests\TestCase; | ||
use Mockery; | ||
|
@@ -22,9 +23,14 @@ class CallbackRegistrarTest extends TestCase | |
/** @var \Illuminate\Routing\Router */ | ||
protected $router; | ||
|
||
/** @var \LaraCrafts\GeoRoutes\CallbackRegistrar|\Mockery\MockInterface */ | ||
protected $registrar; | ||
|
||
public function setUp(): void | ||
{ | ||
parent::setUp(); | ||
|
||
$this->registrar = new CallbackRegistrar; | ||
} | ||
|
||
public function tearDown(): void | ||
|
@@ -39,10 +45,9 @@ public function testIfCallbacksFetchesTheProxiesList() | |
'bar' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::bar', | ||
]; | ||
|
||
$registrar = new CallbackRegistrar; | ||
$registrar->loadCallbacks($callbacks); | ||
$this->registrar->loadCallbacks($callbacks); | ||
|
||
$proxies = $registrar->callbacks(); | ||
$proxies = $this->registrar->callbacks(); | ||
|
||
foreach ($callbacks as $key => $callback) { | ||
$this->assertArrayHasKey('or' . ucfirst($key), $proxies); | ||
|
@@ -57,11 +62,11 @@ public function testIfCallbacksInvokesLoadCallbacksWhenArrayArgIsPresent() | |
'bar' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::bar', | ||
]; | ||
|
||
$registrar = Mockery::mock(CallbackRegistrar::class)->makePartial(); | ||
$this->registrar = Mockery::mock(CallbackRegistrar::class)->makePartial(); | ||
|
||
$registrar->shouldReceive('loadCallbacks')->with($callbacks)->once(); | ||
$this->registrar->shouldReceive('loadCallbacks')->with($callbacks)->once(); | ||
|
||
$registrar->callbacks($callbacks); | ||
$this->registrar->callbacks($callbacks); | ||
} | ||
|
||
public function testIfParseCallbacksLoadsCallbacksFromClass() | ||
|
@@ -71,11 +76,9 @@ public function testIfParseCallbacksLoadsCallbacksFromClass() | |
'orBar' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::bar', | ||
]; | ||
|
||
$registrar = new CallbackRegistrar; | ||
|
||
$registrar->parseCallbacks(\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::class); | ||
$this->registrar->parseCallbacks(\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::class); | ||
|
||
$proxies = $registrar->callbacks(); | ||
$proxies = $this->registrar->callbacks(); | ||
|
||
foreach ($expected as $proxy => $callable) { | ||
$this->assertArrayHasKey($proxy, $proxies); | ||
|
@@ -85,41 +88,78 @@ public function testIfParseCallbacksLoadsCallbacksFromClass() | |
|
||
public function testIfCallbackReturnsCallable() | ||
{ | ||
$registrar = new CallbackRegistrar; | ||
$registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
$this->registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
|
||
$this->assertEquals('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', $registrar->callback('foo')); | ||
$this->assertEquals('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', $registrar->callback('orFoo')); | ||
$this->assertEquals('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', $this->registrar->callback('foo')); | ||
$this->assertEquals('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', $this->registrar->callback('orFoo')); | ||
} | ||
|
||
public function testIfHasCallbackReturnsTrueIfCallbackExists() | ||
{ | ||
$registrar = new CallbackRegistrar; | ||
$registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
$this->registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
|
||
$this->assertTrue($registrar->hasCallback('foo')); | ||
$this->assertTrue($this->registrar->hasCallback('foo')); | ||
} | ||
|
||
public function testIfHasCallbackReturnsFalseIfCallbackDoesNotExist() | ||
{ | ||
$registrar = new CallbackRegistrar; | ||
|
||
$this->assertFalse($registrar->hasCallback('foo')); | ||
$this->assertFalse($this->registrar->hasCallback('foo')); | ||
} | ||
|
||
public function testIfHasProxyReturnsTrueIfProxyExists() | ||
{ | ||
$registrar = new CallbackRegistrar; | ||
$registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
$this->registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
|
||
$this->assertTrue($registrar->hasProxy('orFoo')); | ||
$this->assertTrue($this->registrar->hasProxy('orFoo')); | ||
} | ||
|
||
public function testIfHasProxyReturnsFalseIfProxyDoesNotExist() | ||
{ | ||
$registrar = new CallbackRegistrar; | ||
$this->assertFalse($this->registrar->hasProxy('orFoo')); | ||
} | ||
|
||
public function testIfSetDefaultInvokesCallbackIfArgIsString() | ||
{ | ||
$this->registrar = Mockery::mock(CallbackRegistrar::class)->makePartial(); | ||
$this->registrar->loadCallbacks(['foo' => '\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo']); | ||
|
||
$this->registrar->shouldReceive('callback')->with('foo')->once(); | ||
|
||
$this->registrar->setDefault('foo'); | ||
} | ||
Comment on lines
+121
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion this test is unnecessary. You should test the functionality, not the implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Omitting this test means that we'll have non-test covered code, also this test assures that the method does not interpret callables (e.g. FooClass::bar) as strings which would result in an exception. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion you should test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have thought of that during the testing phase, but that would make the tests tightly coupled and would cause both to fail if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be exact my problem is at line 126. With that you are testing the implementation. However, as far as I know you made |
||
|
||
public function testIfSetDefaultSetsDefaultPropertyValueIfArgIsCallable() | ||
{ | ||
$this->registrar->setDefault('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', 'arg1'); | ||
|
||
$default = $this->getProperty($this->registrar, 'default')->getValue($this->registrar); | ||
|
||
$this->assertEquals(['\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', ['arg1']], $default); | ||
} | ||
|
||
public function testIfSetDefaultThrowsExceptionIfArgIsInvalid() | ||
{ | ||
$this->expectException(InvalidArgumentException::class); | ||
$this->expectExceptionMessage('setDefault expects parameter 1 to be string or callable integer given'); | ||
|
||
$this->registrar->setDefault(321314); | ||
} | ||
|
||
public function testIfGetDefaultReturnsDefaultCallbackArray() | ||
{ | ||
$this->registrar->setDefault('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', 'arg1'); | ||
|
||
$this->assertEquals( | ||
['\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo', ['arg1']], | ||
$this->registrar->getDefault() | ||
); | ||
} | ||
|
||
public function testIfInvokeDefaultCallbackExecutesDefault() | ||
{ | ||
$this->registrar->setDefault('\LaraCrafts\GeoRoutes\Tests\Mocks\Callbacks::foo'); | ||
|
||
$this->assertFalse($registrar->hasProxy('orFoo')); | ||
$this->assertEquals('foo', $this->registrar->invokeDefaultCallback()); | ||
} | ||
|
||
/** | ||
|
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.
According to the doc block
@return void
but this clearly returns the default.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.
Oh that is a typo, forgot to change the auto generated return type, i will also add the @throws exception