saloonphp / saloon

🤠 Build beautiful API integrations and SDKs with Saloon
https://docs.saloon.dev
MIT License
2.03k stars 105 forks source link

feat(testing): accept type-hinted requests as `assertSent` parameters #424

Open innocenzi opened 3 months ago

innocenzi commented 3 months ago

Closes https://github.com/saloonphp/saloon/issues/417

This pull request adds support for type-hinting a request class as the first parameter of the closure passed to MockClient#assertSent. This allows running this kind of test:

MockClient::getGlobal()->assertSent(function (CreateDealRequest $request) {
    expect($request->body()->all())->toMatchArray([
        'status' => DealStatus::OPEN,
    ]);
});

Before this pull request, the test above could only be written using a condition:

MockClient::getGlobal()->assertSent(function (Request $request) {
    if ($request instanceof CreateDealRequest) {
        expect($request->body()->all())->toMatchArray([
            'status' => DealStatus::OPEN,
        ]);

        return true;
    }
});
innocenzi commented 2 months ago

I ignored the phpstan errors as they seem to be false-positives.

juse-less commented 1 month ago

For some reason, I can't get GitHub to let me comment on the changes on my tablet, so I'll make a regular comment for now.

The PHPStan errors around the Reflection APIs might need an extra look. F.ex., the ReflectionFunction only accepts a Closure, but we're passing a callable. I'm not sure how common it is to pass an invokable object, though. That should be relatively easy to fix.

new ReflectionFunction($callable(...));

I'm not 100 % sure on the ReflectionType for the parameters. ReflectionType itself doesn't have a getName(), nor does it's children except for the ReflectionNamedType. This may or may not be an issue. If only one type is defined, it'll work. If no parameters are defined, then index 0 will also give an error. But realistically, I feel like these are edge cases.