Testing @Vercoutere's middleware

1-summary.md

Trying to help test this: https://gist.github.com/Vercoutere/c002cfb0040ddb04bf3d

I can't actually run this, but this is generally how I would test what you have without changing any of your code. It doesn't look quite as bad when you remove all the comments in the test :)

I've only provided an example of one specific code path, but the rest would follow a similar approach.

The test definitely has some complexity, which you might use to inform your design decisions. Most of the complexity comes from global access to the Eloquent queries, and through the use of service location through the Route and App facades to reach out and grab dependencies instead of passing them in.

All in all, it's pretty neat that you can still test this with confidence despite that, because Laravel does a pretty good job of making it easy to swap service located dependencies.

Changes you could make to simplify the test...

1. Register a TenantMiddleware binding in the IOC container so you can control how it is built. This would let you pass in an instance of Tenant as a dependency, which you can use to make the static queries against. You could pass in a stub in your tests and avoid having to integrate against the database this way. PHP would happily let you do $tenants->getByDomain($domain) even though getByDomain is a static method, pretty convenient for this sort of thing honestly.

2. Pass in the Context as a dependency to the middleware as well instead of resolving it inline. Easier to pass the spy in this way.

3. Use $request->route('domain') instead of Route::input('domain') (for 'tld' and 'subdomain' as well) so you can pass in a stub for the $request instead of relying on stubbing through the facade.

These are just ideas and not things you definitely need to do, but worth considering. I don't subscribe to the idea that your goal when designing code should be to make testing as simple as possible, and have definitely seen it introduce unnecessary complexity. In this case though, these are probably pretty good changes to consider.

Hope that helps!

2-no-code-changes.php

<?php

// Since you are relying on Eloquent calls, we need to integrate
// against some sort of DB, like an in-memory SQLite DB.
class IntegrationTestCase extends TestCase
{
    public function setUp()
    {
        parent::setUp();
        Artisan::call('migrate');
    }
}

class TenantMiddlewareTest extends IntegrationTestCase
{
    function test_it_sets_the_context_based_on_the_subdomain()
    {
        // Shortcut for creating a database-valid Eloquent model that your
        // queries will return, using a package like Faktory, TestDummy,
        // or FactoryMuffin
        $tenant = Factory::create('tenant', ['subdomain' => 'some-tenant']);

        // Facades can be conveniently stubbed this way
        Route::shouldReceive('input')->with('domain')->andReturn(null);
        Route::shouldReceive('input')->with('tld')->andReturn(null);
        Route::shouldReceive('input')->with('subdomain')->andReturn($tenant);

        // Request isn't actually used, so we can just pass a dummy
        $request = Mockery::mock();

        // We can use a hand-rolled Closure double like this to ensure
        // it gets called.
        $next = function () { return 'next-middleware'; };

        // We'll spy the Context and bind it to the container to make
        // sure it gets set with the correct tenant.
        $context = Mockery::spy('Context');

        App::singleton('Context', function ($app) use ($context) {
            return $context;
        });

        $middleware = new TenantMiddleware;

        $result = $middleware->handle($request, $next);

        // Make sure $next was called instead of redirect
        $this->assertEquals($result, 'next-middleware');

        // Make sure the context was set with the right tenant
        $context->shouldHaveReceived('set')->with($tenant);
    }
}

3-with-suggested-changes.php

<?php

// Here's what it might look like with the code changes I suggested.
// The biggest win here is no need to integrate against a migrated
// database.

class TenantMiddlewareTest extends TestCase
{
    function test_it_sets_the_context_based_on_the_subdomain()
    {
        $matchingTenant = Mockery::mock('Tenant');
        $context = Mockery::spy('Context');
        $tenants = Mockery::mock('Tenant');
        $request = Mockery::mock('Request');

        $tenants->shouldReceive('getBySubdomain')
            ->andReturn($matchingTenant);

        $request->shouldReceive('route')->with('domain')->andReturn(null);
        $request->shouldReceive('route')->with('tld')->andReturn(null);
        $request->shouldReceive('route')->with('subdomain')->andReturn($matchingTenant);

        $next = function () { return 'next-middleware'; };

        $middleware = new TenantMiddleware($tenants, $context);

        $result = $middleware->handle($request, $next);

        $this->assertEquals($result, 'next-middleware');
        $context->shouldHaveReceived('set')->with($matchingTenant);
    }
}