Fixes #246 : Forward plugin should detach problem listeners #247
Conversation
src/Controller/Plugin/Forward.php
Outdated
|
|
||
| foreach ($classArray as $class) { | ||
| if ($currentCallback instanceof $class) { | ||
| // Pass $currentEvent; when using zend-eventmanager v2, |
There was a problem hiding this comment.
Please remove this comment, it was only relevant for v2.
| @@ -237,6 +238,35 @@ function ($e) {} | |||
| $this->assertEquals(['content' => 'ZendTest\Mvc\Controller\TestAsset\ForwardController::testAction'], $result); | |||
| } | |||
|
|
|||
| public function testProblemListenersAreDetachedAndReattachedWhenPluginDispatchsRequestedController() | |||
There was a problem hiding this comment.
No assertion or expected exception?
There was a problem hiding this comment.
@froschdesign I'm not sure what the ZF policy or practice is for this type of assertion but in this case, the assertion is implied in the mock method invocation expectation:
$sharedEvents->expects($this->once())->method('detach')If the method isn't invoked with the expected arguments, the test will fail with an exception. What is the preferred way of testing something like this? Maybe a test spy that monitors called methods and then the assertion can be the method was called?
There was a problem hiding this comment.
@froschdesign Just bumping this, it's been 3 months and I'd love to start using ZF3. This issue is the only thing holding us back.
I'm happy to make whatever change to the test. As my comment above says, my assertion is implied in the mock once invocation expectation. Would you prefer I change this to an explicit test spy? I'm not sure what the ZF test expectation would be for this use case.
|
Test failure is unrelated and fixed in #260 |
Fixes #246