Framework: Can't test routes that use model binding (when using WithoutMiddleware trait)

Created on 4 Nov 2016  Â·  10Comments  Â·  Source: laravel/framework

  • Laravel Version: 5.3.22
  • PHP Version: 7

Description:

It's not possible (as far as I can tell) to test a route that uses Route-Model binding, if you also need to use the WithoutMiddleware trait (or method).

This seems to clearly be because model bindings are substituted in a middleware.

Steps To Reproduce:

  1. Create a test
  2. use WithoutMiddleware
  3. Try to call/test a route that uses model binding ($this->get('route/{id}')
  4. Model will not be bound, nor can it be explicitly bound within the test, as the router has already booted.

Most helpful comment

I understand what you're saying, and you're not _wrong_. I just disagree that this is "expected" behavior. Two important, helpful features of the framework (model binding and easy integration testing) are in conflict, and making the developer implement some sort of complex workaround, without any documentation even, to make them play nice is not a very Laravel way to handle it.

I don't think that this case is or will be uncommon — model binding is great, and simple integration testing using the middleware helpers provided is also great. They should play nice, and I (again, respectfully) posit that them failing to play nice constitutes a bug.

With more features of the framework being implemented as middlewares, I would even suggest that the WithoutMiddleware trait by default handle the exclusion of certain essential feature-middlewares from being disabled. There is no benefit whatsoever to accidentally disabling model binding in an integration test, and I'll bet there are more features that will have similar consequences in future.

All 10 comments

This is the expected behaviour now I'm, afraid. You need middleware turned on if you want model binding.

If you want to disable specific middleware in testing, you could follow the approach the csrf middleware uses in the core. Take a look at the source and it'll all make sense - better than me explaining it. :)

@GrahamCampbell With respect, I disagree that this is an issue that can just be closed. It represents a direct conflict between two important framework features, which is totally undocumented, took me two days to figure out and has no simple workaround (your suggestion above notwithstanding).

The 'WithoutMiddleware` trait is meant specifically for use with doing integration tests with controllers. It seems to me that it breaking model bindings at all, let alone without a single mention in the docs isn't something that should just be closed.

Thoughts?

Since bindings are implemented with middleware, turning off middleware should break them. For that reason, I'd recommend not globally turning them off, but instead implementing a more sophisticated strategy such as the one used in the core for CSRF. You can not turn off middlewares and the CSRF middleware will still not run during unit testing.

I understand what you're saying, and you're not _wrong_. I just disagree that this is "expected" behavior. Two important, helpful features of the framework (model binding and easy integration testing) are in conflict, and making the developer implement some sort of complex workaround, without any documentation even, to make them play nice is not a very Laravel way to handle it.

I don't think that this case is or will be uncommon — model binding is great, and simple integration testing using the middleware helpers provided is also great. They should play nice, and I (again, respectfully) posit that them failing to play nice constitutes a bug.

With more features of the framework being implemented as middlewares, I would even suggest that the WithoutMiddleware trait by default handle the exclusion of certain essential feature-middlewares from being disabled. There is no benefit whatsoever to accidentally disabling model binding in an integration test, and I'll bet there are more features that will have similar consequences in future.

@GrahamCampbell Also, assuming we're not going to agree on this, could you please point me at specifically where you're referring to with the CSRF middleware?

I realize this is old but here's what I did.
Create class app/Http/Middleware/Authenticate.php which has

namespace App\Http\Middleware;

class Authenticate extends \Illuminate\Auth\Middleware\Authenticate
{
    protected function authenticate(array $guards)
    {
        if (!app()->runningUnitTests()) {
            return parent::authenticate($guards);
        }
    }
}

Modify app/Http/Kernel.php

    protected $routeMiddleware = [
//        'auth' => \Illuminate\Auth\Middleware\Authenticate::class,
        'auth' => Authenticate::class,

remove use WithoutMiddleware; from my test.

In my case WithoutMiddleware was used to bypass authentication and CSRF solely, so I made a custom trait which only disable those and leaves the rest, including model binding.

My custom trait (inspired by the original WithoutMiddleware:

trait WithTestMiddlewareOnly
{
    /**
     * Prevent all middleware from being executed for this test class.
     *
     * Taken from Illuminate\Foundation\Testing\WithoutMiddleware::disableMiddlewareForAllTests
     *
     * @throws \Exception
     */
    public function onlyEnableTestMiddlewareForAllTests()
    {
        $middlewaresToDisable = [
            \Illuminate\Auth\Middleware\Authenticate::class,
            \Illuminate\Foundation\Http\Middleware\VerifyCsrfToken::class,
        ];

        if ( method_exists( $this, 'withoutMiddleware' ) ) {
            $this->withoutMiddleware( $middlewaresToDisable );
        }
        else {
            throw new Exception( 'Unable to disable middleware. MakesHttpRequests trait not used.' );
        }
    }
}

Plugging the trait into my tests, I have a custom Tests\TestCase that extends the default TestCase and all my tests extend. In this file, extend the setUpTraits function:

protected function setUpTraits()
{
    $uses = parent::setUpTraits();

    if (isset($uses[WithTestMiddlewareOnly::class])) {
        $this->onlyEnableTestMiddlewareForAllTests();
    }

    return $uses;
}

I can now just include WithTestMiddlewareOnly on top of all relevant tests, just like I was doing with WithoutMiddleware.

@robinmoisson Thanks for this!

Do you add this in place of WithoutMiddleware, or alongside it?

That is not expected. No.

Let's talk like real adults.

No one meant to break the route model binding functionality when creating/using the WithoutMiddleware facade. No one meant to disable TrimStrings either.

It is not expected because; In any controlled experiment:

  • Control group must be controlled under normal conditions.
  • The experimental group must be maintained under normal conditions in all aspects except for one variable being tested.

Since our control group is the Action being tested, we don't expect any changes which affect the behavior of it.

That bug is just a well-understood consequence of an overused feature.

Possible Solution:
Use withoutMiddleware method and cherry-pick the middlewares you don't want.

public function test(): void {
    $this->withoutMiddleware(Authenticate::class);
}
Was this page helpful?
0 / 5 - 0 ratings

Related issues

GrahamCampbell picture GrahamCampbell  Â·  139Comments

thewinterwind picture thewinterwind  Â·  63Comments

JosephSilber picture JosephSilber  Â·  176Comments

robjbrain picture robjbrain  Â·  64Comments

ThomHurks picture ThomHurks  Â·  75Comments