laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.42k stars 10.99k forks source link

Http client not applying middleware when using Http::fake() #34505

Closed badams closed 4 years ago

badams commented 4 years ago

Description:

If you provide middleware on your Http client it will never be invoked when using Http::fake()

Steps To Reproduce:

class HttpClientMiddlewareTest extends TestCase
{
    public function test_it_applies_middleware_when_no_fake()
    {
        $middleware = Middleware::mapResponse(function (ResponseInterface $response) {
            return $response->withHeader('X-Foo', 'bar');
        });

        $response = Http::asForm()->withMiddleware($middleware)->get('http://google.com');
        $this->assertEquals('bar', $response->header('X-Foo'));
    }

    public function test_it_doesn_not_apply_middleware_when_fake()
    {
        $middleware = Middleware::mapResponse(function (ResponseInterface $response) {
            return $response->withHeader('X-Foo', 'bar');
        });

        Http::fake();

        $response = Http::asForm()->withMiddleware($middleware)->get('http://google.com');
        $this->assertEmpty($response->header('X-Foo'));
    }
}

Screen Shot 2020-09-24 at 2 45 56 PM

As you can see the middleware is not applied in the second test

driesvints commented 4 years ago

I'm actually not sure about this one myself. Will need input from @taylorotwell here.

taylorotwell commented 4 years ago

I'm not sure I would expect any middleware to run at all, so to me personally this is expected behavior. If you're faking the HTTP client you are going to specify exactly what responses should returned.

badams commented 4 years ago

@taylorotwell @driesvints I'd really appreciate if you can reconsider this, without this working its completely impossible to test that middleware is configured and working correctly.

Perhaps my example of mutating the response was not great - this was for simplicity sake and was lifted directly from the guzzle docs.

I have a number of middleware that perform a range of actions such as:

Without the ability to test my middleware, the withMiddleware(...) method is not useful to me and I've had to opt for a more procedural approach or use Guzzle directly, but honestly I much prefer the interface for Http and Http:fake()

I'm happy to work on a PR for this if needed.

driesvints commented 4 years ago

We're probably not going to reconsider this, sorry.

eduarguz commented 4 years ago

So, I also spent some time in this... Then found this issue.

I think there is another good use case for this. We, very often, need to log what is sent to an api and what is received.

When testing, using Mail::fake(), Testing this logger will not be possible

$response = Http::withMiddleware(Middleware::log(
            app(LogManager::class)->channel(),
            new MessageFormatter()
        ))->{$method}($url, $payload);

just in case +1 on this

Thanks