[Suggestion] provide a way to check valid EventSubscriberInterface implementations

[stof]

The EventSubscriberInterface asks you to return an array of events for which the class wants to listen. Validating the proper implementation of this interface is a bit complex. Here is an example doing part of it:

<?php

namespace spec\Acme\DemoBundle\Activity;

use PhpSpec\Exception\Example\FailureException;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class MyListenerSpec extends ObjectBehavior
{
    function it_is_an_event_subscriber()
    {
        $this->shouldHaveType('Symfony\Component\EventDispatcher\EventSubscriberInterface');

        $this->getSubscribedEvents()->shouldBeArray();
        foreach ($this->getSubscribedEvents()->getWrappedObject() as $event => $listeners) {
            if (is_string($listeners)) {
                $this->shouldHaveListenerMethod($listeners);
                continue;
            }

            if (!is_array($listeners)) {
                throw new FailureException(sprintf('The listener registered for %s must be a string or an array of listeners', $event));
            }

            if (is_string($listeners[0])) {
                $this->shouldHaveListenerMethod($listeners[0]);
                continue;
            }

            foreach ($listeners as $i => $listener) {
                if (!is_array($listener)) {
                    throw new FailureException(sprintf('The listener %s registered for %s must be an array (method, priority)', $i, $event));
                }

                $this->shouldHaveListenerMethod($listener[0]);
            }
        }
    }

    public function getMatchers()
    {
        return array(
            'haveListenerMethod' => function ($subject, $method) {
                if (!method_exists($subject, $method)) {
                    return false;
                }

                $refl = new \ReflectionMethod($subject, $method);

                return $refl->isPublic() && !$refl->isAbstract();
            }
        );
    }
}

This implementation is incomplete:

• it does not verify that the priority is returned properly in cases where it is expected (both cases where I access the index 0 for the method should check the index 1 for an integer)
• it misses some checks for missing indexes in arrays
• it is not implemented as a matcher on its own

This implementation most checks that the registered method actually exist (which avoids mistakes like doing a typo in the method name)

Having to repeat this complex validation in each subscriber spec would be a huge pain, which is why I would prefer to register it as a matcher and write something like $this->shouldBeAnEventSubscriber() or something like that.

Do you think such matcher could fit in the Symfony2Extension ? Or is the extension aimed at integrating with the fullstack and component-specific matchers should better go in a separate extension easier to use separately ? I haven't tried to use the Symfony2Extension yet in my project, so I don't know how intrusive it is (i.e. is it possible to register it in a project not using the fullstack to benefit from such a matcher ?)

[stof]

The other solution could be to have this matcher in the core as it would benefit PhpSpec itself as well for instance. However, this could look too framework-specific to be in core (which is why I suggested it here first to avoid requests to include similar matchers in core for any library in the future).
What do you think @phpspec/team-phpspec @everzet ?

[ciaranmcnulty]

I think it might be an overstretch of this method's responsibility to verify that these methods are callable / public - surely that is going to be covered by other tests?

[stof]

@ciaranmcnulty if you don't write a spec yet for other methods of the class, you would have something passing by defining a private or an abstract method, while getting a fatal error when dispatching the event in your code. The expectation for an event subscriber is that the dispatcher is able to call the method, not only that the method exists. Otherwise, you would not be catching errors where you used the name of your private method instead of the name of the proper listener when both exist in the class.

Note that I'm not trying to call the listener method itself. The fact that the listener method works should indeed be covered by other tests (I cannot call it anyway, as most listeners will not accept the base Event class but typehint the expected subclass instead, and so I'm not able to guess the needed argument)

[ciaranmcnulty]

I see your point, I guess it's more that I think you're imposing false ignorance of what the rest of the class is going to do...

For me I'd be happy to do:

$this-> getSubscribedEvents()->shouldReturn(['foo'=>'handleFoo']);

In the example you've provided getSubscribedEvents could return an empty array and the test would not catch it. Or it could return an array handling events foo and bar but not baz which is critical, and the test would not detect it.

[stof]

@ciaranmcnulty Asserting on which event you subscribe is a different expectation, which should appear on its own in the spec. It is part of your expectation of the class, not of the expectation of it being a valid implementation of the EventSubscriberInterface
And as far as the EventDispatcher is concerned, returning an empty array is a valid implementation of the interface. Registering a non-existent method leads to a fatal error whe calling it, and it might be hard to catch if it is an event triggered only in some specific cases (and it could be catched only by an integration or functional test).

$this->getSubscribedEvents()->shouldReturn(['foo'=>'handleFoo']);

will not test at all that your handleFoo method is expected to exist and be callable. And it does not validate that your returned array really matches what is supported by the EventDispatcher (it is easy for this simple case, but checking the proper syntax is important for more complex cases where you want to set the priority or to register several methods on the same event).
Tus, if you make a typo in the registered method name when writing your spec for getSubscribedEvents(), your spec will force you to do the same typo in the code (which will then pass) instead of warning you that you made something wrong