3
\$\begingroup\$

I am writing a unit test for a Dispatcher that will route a HTTP request to a specific Controller and invoke a specific method on that Controller. It will also trigger various events at different steps of the process. The majority of what is going on is carried out by dependencies injected at construction time; the Dispatcher is just wiring everything together in the proper order.

I have gotten to where I am testing the events being triggered appropriately during processing. There are three "primary" steps of the process:

  • Routing
  • Controller Invocation
  • Response being sent to user

Before each primary step is carried out a "before" event is triggered and after the step is finished an "after" event is triggered. The problem I am encountering is with unit testing that the appropriate after event is not triggered before the before event. I have attempted to mock this dependency with PHPUnit for these tests and use the Mock object's $this->at($methodInvokeIndex) but this is, to be blunt, a royal PITA. Every time you invoke a new method on the object you have to go readjust indexes all over the place. For the sake of this review this method of mocking objects is not feasible and will not be implemented.

Below are the test helpers and unit test I have for testing the before routing event is triggered. The reason I ask for the review is that the test is, well, a bit hacky. I'm getting this huge red warnings about the test and typically I see that as a failure in the design but I'm not seeing how.

Anyway, to the code. Everything in this question is related to a project available at cspray/SprayFire:restful-dispatcher.

MediatorHelper

<?php

/**
 * An abstract \SprayFire\Mediator\Mediator that will cache all triggered events
 * for detailed inspection during unit testing.
 * 
 * @author Charles Sprayberry
 * @license See LICENSE in source root
 */

namespace SprayFireTest\Helpers\DispatcherMediator;

use \SprayFire\Mediator;

abstract class MediatorHelper implements Mediator\Mediator {

    /**
     * A collection of info related to triggered events; each element in the numerically
     * indexed array is the info for a triggered event, each one of these arrays
     * will have the following keys:
     *
     * 'name' => event name,
     * 'target' => event Target,
     * 'args' => event args
     *
     * @property array
     */
    private $events = [];

    /**
     * @property array
     *      @type \SprayFire\Mediator\Callback
     */
    protected $callbacks = [];

    /**
     * Will cache each triggered event for later inspection.
     *
     * @param string $eventName
     * @param object $Target
     * @param array $args
     * @return bool|void
     */
    public function triggerEvent($eventName, $Target, array $args = []) {
        $this->events[] = [
            'name' => $eventName,
            'target' => $Target,
            'args' => $args
        ];
    }

    /**
     * Will return an array representing the info for each triggered event before
     * getTriggeredEvents() was called.
     *
     * @return array
     */
    public function getTriggeredEvents() {
        return $this->events;
    }

    /**
     * Will return an array of \SprayFire\Mediator\Callback objects or an empty
     * array if no callbacks are added for the given event.
     *
     * @param string $eventName
     * @return array
     */
    public function getCallbacks($eventName) {
        return $this->callbacks;
    }

    /**
     * Should add a callback to a collection of callbacks for a specific event
     * based on the event name returned from the passed $Callback.
     *
     * @param \SprayFire\Mediator\Callback $Callback
     * @return boolean
     */
    public function addCallback(Mediator\Callback $Callback) {
        $this->callbacks[] = $Callback;
    }

    /**
     * Should remove a callback from the collection of callbacks for the event
     * name.
     *
     * @param \SprayFire\Mediator\Callback $Callback
     */
    public function removeCallback(Mediator\Callback $Callback) {
        return false;
    }

}

TriggerBeforeRoutingEvent

<?php

/**
 * A test helper that ensures the appropriate \SprayFire\Events::BEFORE_ROUTING
 * event is triggered one time and that \SprayFire\Events::AFTER_ROUTING is not
 * triggered when the event is triggered.
 * 
 * @author Charles Sprayberry
 * @license See LICENSE in source root
 */

namespace SprayFireTest\Helpers\DispatcherMediator;

use \SprayFire\Mediator;
use \SprayFire\Events;
use \PHPUnit_Framework_AssertionFailedError as TestFailedException;

class TriggerBeforeRoutingEvent extends MediatorHelper {

    /**
     * @param string $eventName
     * @param object $Target
     * @param array $args
     * @return bool|void
     * @throws \PHPUnit_Framework_AssertionFailedError
     */
    public function triggerEvent($eventName, $Target, array $args = []) {
        if ($eventName === Events::BEFORE_ROUTING) {
            $alreadyTriggered = $this->getTriggeredEvents();
            foreach($alreadyTriggered as $eventInfo) {
                if ($eventInfo['name'] === Events::BEFORE_ROUTING) {
                    throw new TestFailedException('The ' . Events::BEFORE_ROUTING . ' event was triggered multiple times');
                }

                if ($eventInfo['name'] === Events::AFTER_ROUTING) {
                    throw new TestFailedException('The ' . Events::AFTER_ROUTING . ' event was triggered before the ' . Events::BEFORE_ROUTING . ' event');
                }
            }
        }


        // we store the event after we check already triggered events
        // otherwise the first call to BEFORE_ROUTING will cause test to fail on first check
        parent::triggerEvent($eventName, $Target, $args);
    }

}

DispatcherTest

This is just the one test related to above code. The references to $MockObject properties are mock objects that will satisfy the dependency injection contract.

<?php

/**
 * @author  Charles Sprayberry
 * @license Subject to the terms of the LICENSE file in the project root
 * @version 0.3
 * @since   0.1
 */

namespace SprayFireTest\Dispatcher\FireDispatcher;

use \SprayFire\Events;
use \SprayFire\Dispatcher\FireDispatcher;
use \SprayFireTest\Helpers;
use \PHPUnit_Framework_TestCase as PHPUnitTestCase;

class DispatcherTest extends PHPUnitTestCase {

    public function testDispatcherInvokesBeforeRoutingOneTimeAndBeforeAfterRouting() {
        $this->MockRouter->expects($this->once())
                         ->method('getRoute')
                         ->will($this->returnValue($this->getMock('\\SprayFire\\Router\\Route')));

        $this->MockFactory->expects($this->once())
                          ->method('makeObject')
                          ->will($this->returnValue($this->getMock('\\SprayFire\\Controller\\Controller')));

        $Mediator = new Helpers\DispatcherMediator\TriggerBeforeRoutingEvent();
        $Dispatcher = new FireDispatcher\Dispatcher($this->MockRouter, $this->MockFactory, $Mediator);
        $Dispatcher->dispatch($this->MockRequest, $this->MockResponse);

        $testPassed = false;
        $triggeredEvents = $Mediator->getTriggeredEvents();
        foreach($triggeredEvents as $eventInfo) {
            if ($eventInfo['name'] === Events::BEFORE_ROUTING) {
                $testPassed = true;
            }
        }

        if (!$testPassed) {
            $this->fail('The ' . Events::BEFORE_ROUTING . ' event was not triggered');
        }

    }

}

So, the hackiness is pretty apparent. To go over the problems I see:

  • Some of the testing is actually taking place in the Helpers. If the Helper throws a PHPUnit_Framework_AssertionFailedError it is ultimately doing something the actual unit testing test case should be doing.
  • The actual unit test requires a foreach loop and I've always found that this "seems" wrong.

The pros are that I don't have to work with trying to mock out something that is extremely difficult to do so.

Is there something horribly wrong here? Are my fears justified and this is more indicative of a problem with the design? I've come to the conclusion that I don't believe the design is necessarily a problem; I can't come up with any way to perform these actions in a way that makes this test easier to implement.

\$\endgroup\$
  • \$\begingroup\$ Yes you’re right about your fears. Your user land code should not have test exceptions, and your code shouldn’t need any mock really. You should be able to set it up easily by creating an empty dispatcher, adding a route and events to it, and then do assertions on those events. As far as I know nothing lets you do this right now and that’s your design problem. Make this test using TDD and imagine how you’d like to use and test it without thinking about the real implementation, you’ll find a much better design since you don’t have a choice to test it properly. \$\endgroup\$ – Steve Chamaillard Jul 17 '18 at 9:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.