-
Notifications
You must be signed in to change notification settings - Fork 11.6k
Open
Labels
Description
Laravel Version
9.x, 10.x, 11.x, 12.x
PHP Version
All supported
Database Driver & Version
None
Description
As far as I can tell this is an issue with all versions of Laravel back to at least v9.
The documentation at https://laravel.com/docs/events#manually-registering-events says that you should manually associate events and listeners in your service provider like this:
public function boot()
{
Event::listen(
PodcastProcessed::class,
[SendPodcastNotification::class, 'handle']
);
}
however the deprecated EventFaker
documentation says:
Event::fake();
Event::assertListening(PodcastProcessed::class, SendPodcastNotification::class);
The code in v9.x framework/src/Illuminate/Support/Testing/Fakes/EventFake.php
clearly suggests that you can specify listeners as either "class@method"
or ["class", "method"]
.
It turns out that there are (IMO) three algorithmic bugs in the assertListening
method:
- The expected and actual listeners are not independently normalised when they should be - when the actual listener is NOT specified as "class@method", the expected listener is not normalised.
- The documentation suggests that the expected listener should be specified just as a class name, and this is incorrect - currently it needs to be specified as either "class@method" or "[class, method]'.
- IMO (and this is a personal opinion) if the expected listener is specified as only "class" and not one of the two methods in point 2., then it should match based only on classes and not on method.
Solution
- See proposed code fix below.
- Restore
EventFaker
documentation from 8.x to later versions. - Improve the Event Listener documentation to document the use of
"class@method"
.
Steps To Reproduce
public function assertListening($expectedEvent, $expectedListener)
{
foreach ($this->dispatcher->getListeners($expectedEvent) as $listenerClosure) {
$actualListener = (new ReflectionFunction($listenerClosure))
->getStaticVariables()['listener'];
$normalizedListener = $expectedListener;
if (is_string($actualListener) && Str::contains($actualListener, '@')) {
$actualListener = Str::parseCallback($actualListener);
if (is_string($expectedListener)) {
if (Str::contains($expectedListener, '@')) {
$normalizedListener = Str::parseCallback($expectedListener);
} else {
$normalizedListener = [
$expectedListener,
method_exists($expectedListener, 'handle') ? 'handle' : '__invoke',
];
}
}
}
if ($actualListener === $normalizedListener ||
($actualListener instanceof Closure &&
$normalizedListener === Closure::class)) {
PHPUnit::assertTrue(true);
return;
}
}
PHPUnit::assertTrue(
false,
sprintf(
'Event [%s] does not have the [%s] listener attached to it',
$expectedEvent,
print_r($expectedListener, true)
)
);
}
and IMO should be:
public function assertListening($expectedEvent, $expectedListener)
{
$normalizedListener = $expectedListener;
if (is_string($normalizedListener) && Str::contains($normalizedListener, '@')) {
$normalizedListener = Str::parseCallback($normalizedListener);
}
foreach ($this->dispatcher->getListeners($expectedEvent) as $listenerClosure) {
$actualListener = (new ReflectionFunction($listenerClosure))
->getStaticVariables()['listener'];
if (is_string($actualListener) && Str::contains($actualListener, '@')) {
$actualListener = Str::parseCallback($actualListener);
}
if (is_string($normalizedListener) && is_array($actualListener)) {
$actualListener = $actualListener[0];
}
if ($actualListener === $normalizedListener ||
($actualListener instanceof Closure &&
$normalizedListener === Closure::class)) {
PHPUnit::assertTrue(true);
return;
}
}
PHPUnit::assertTrue(
false,
sprintf(
'Event [%s] does not have the [%s] listener attached to it',
$expectedEvent,
print_r($expectedListener, true)
)
);
}