Codeception/module-laravel

Terminate method not being called on terminable middleware in test

AndrewFeeney opened this issue · 8 comments

After upgrading a Laravel 6 application to Laravel 7 I noticed that a test which was making assertions against behaviour which is executed in the terminate() method of a custom middleware was failing.

After further inspection, I found that even though the application behaviour had not changed, the terminate() method was never being called in the test. The laravel module calls terminate() on the HTTP Kernel class, but passes in the original request object to the terminate() method.

$this->app->make(Kernel::class)->terminate($request, $response);

In order for the terminate() method to be called on the route middleware, the applicable middlewares are fetched from route instance returned from $request->route() if any. However, the route does not appear to be resolved on the request instance that gets passed into the terminate method.

It would appear to me that in the normal execution of the Laravel HTTP Kernel, the instance of the request that is being passed into the terminate method is being resolved out of the service container rather than being mutated by reference such as this test module code might expect. If you dump the request instance returned by request() after the call to handle() on the kernel you can see that the route has been resolved and bound to the request instance.

I haven't yet managed to isolate the specific change which is causing my test, which previously passed the original "laravel5" module and Laravel 6, and now fails with both Laravel 7, and 8 using this new "laravel" module. It might be a change in the Laravel framework, or it might be related to changes in the Codeception module.

Swapping the $request variable with the request() global helper function (which resolves the request out of the service container under the hood) fixed the issue. I'm not sure that this is the way you would want to fix it in a patch, since you might not want to rely on the request() global function or use implicit service location like that, but it did confirm that this appears to be the problem.

        $this->app->make(Kernel::class)->terminate(request(), $response);

I'd be happy to look at submitting a PR with a fix, but since the tests are in a different repo, I'll have to take a look at that repo and get my head around how the tests workflow goes. I wanted to leave this issue here in case anybody else hits this, and / or has a better idea of the cause or a solution.

@AndrewFeeney I went through all the changes I made in the migration from module-laravel5 to module-laravel and didn't find any significant changes that could've broken something with this flow.

Most of the changes were related to fixing the database related code that changed in Laravel 8 and adding strict typing.

So, i'm wondering if to get the request object like this...:

$this->app->make(Kernel::class)
-    ->terminate($request, $response); 
+    ->terminate($this->app['request'], $response);

... would help fix your issue too.

I have just released version 2.1.0 of this module, so it is a good time to submit a PR for this if you have time.

If you have any questions regarding the test that you should write in the test project, feel free to ping me.