Make testing of requests a joy

[bobmulder]

Hi Sam!

Working with Saloon makes it a joy to develop my API integrations. However, when testing, my test classes become a mess. I often use assertSent, with a callable and a bunch of checks which looks like:

$mockClient->assertSent(function (Request $request, Response $response) {
    return $request instanceof CreateWebhookRequest
        && Arr::get($request->defaultBody(), 'action') === ActionType::Create
        && Arr::get($request->defaultBody(), 'objectType') === ObjectType::Customer
        && Arr::get($request->defaultBody(), 'url') === 'http://local.mbb/webhooks/integration'
        && $response->status() === 200;
});

Today, I started experimenting in how I'd like to design my tests, and came up with my own RequestValidator. Using this helper, results in:

$integration->assertSent(value: function (Request $request) {
    return RequestValidator::for($request)
        ->instanceOf(CreateProfileRequest::class)
        ->bodyEquals('fields.Email', 'info@email.nl')
        ->bodyEquals('fields.Voornaam', 'Firstname')
        ->bodyEquals('fields.Tussenvoegsel', '')
        ->bodyEquals('fields.Achternaam', 'Lastname')
        ->bodyEquals('fields.Klantnummer', 22446688)
        ->bodyEquals('fields.Relatienummer', "88664422")
        ->bodyEquals('fields.CreatedOn', '2021-01-01 12:00:00')
        ->bodyEquals('fields.UpdatedOn', '2021-01-01 12:00:00')
        ->validate();
});

Presonally I like this kind of syntax. Maybe I'd make some changes later, but I want to check up with you what you think about it. Would this be a nice addition to Saloon to make testing API's a joy?

For anyone else interested, this is what RequestValidator looks like right now:

<?php

namespace Tests\Support;

use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Saloon\Contracts\Request;

class RequestValidator
{

    protected Collection $results;

    public function __construct(protected Request $request)
    {
        $this->results = collect();
    }

    public static function for(Request $request): self
    {
        return new static($request);
    }

    public function instanceOf(string $class): self
    {
        $check = $this->request instanceof $class);

        $this->results->push($check);

        return $this;
    }

    public function bodyEquals(string $path, mixed $value): self
    {
        $expected = Arr::get($this->request->body()->all(), $path);

        if ($expected instanceof Carbon) {
            $expected = (string) $expected;
        }

        $this->results->push($expected === $value);

        return $this;
    }

    public function validate(): bool
    {
        return $this->results->every(fn ($item) => $item === true);
    }
}

[Sammyjo20]

Hey @bobmulder first off - very sorry for a slow reply to your issue here - it's been a crazy couple of weeks so I'm just getting back into Saloon development. I hope you are well.

I have to say I really love this 🔥 Do you have any plans to make a PR for this? Otherwise I can add it to my v3/v4 roadmap

[bobmulder]

Hi @Sammyjo20! I can't blame you, since I have a hard time myself to keep organised here, so no worries 😜.

I love your enthusiasm about the idea. It wouldn't be fair to make you do the work for v3/v4 yourself. I'd love to collaborate on this. I would be able to do the biggest implementation/coding part, and would need a little help with testing, some decisions, and making it all ready to release.

But first we have to make some decisions on the DX. Would you prefer to continue discussing on here or schedule a videocall?

[bobmulder]

Some thoughts that crossed my mind:

Providing a RequestValidator within assertSee

The first example provided above would be cleaner when having a Request object which contains its own validation methods. The dx would be a little bit cleaner:

$integration->assertSent(fn (RequestValidator $request) => $request
        ->instanceOf(CreateProfileRequest::class)
        ->bodyEquals('fields.Email', 'info@email.nl')
        ->bodyEquals('fields.Voornaam', 'Firstname')
        ->bodyEquals('fields.Tussenvoegsel', '')
        ->bodyEquals('fields.Achternaam', 'Lastname')
        ->bodyEquals('fields.Klantnummer', 22446688)
        ->bodyEquals('fields.Relatienummer', "88664422")
        ->bodyEquals('fields.CreatedOn', '2021-01-01 12:00:00')
        ->bodyEquals('fields.UpdatedOn', '2021-01-01 12:00:00')
        ->validate(); // could be optional if the above methods are assertion methods
);

Another variant on this suggestion is to return a validator, only if the user asks for it. So assertSent(fn (RequestValidator $request) would be possible, but assertSent(fn (Request $request) as well, which wouldn't break the current usage of assertSent.

Combining faking and assertions

I'm not quite sure if this would be a good idea, but it might be interesting. Currently, I first 'arrange' my test, where I set up my response mocks. Next, I execute some stuff, and at last I make my assertions, which have a lot to do with my mocks.

What if you could do something like:

$mockClient = new MockClient([

    TestResponse::fake($data, 200)
        ->expect(once())
        ->bodyEquals('fields.Email', 'info@email.nl')

]);

[Sammyjo20]

Hi @bobmulder sorry for the delay - I like your idea! I'm happy if you want to go ahead and make a PR and then we can work together from that?

Could you make a PR for the v3 branch too? All the tests pass (phpstan doesn't but that's fine)

[bobmulder]

Hi @Sammyjo20, I will set up a first draft next week based on v3. I'll start with a RequestValidator and ResponseValidator. From there we can work on other ideas and details.

[Sammyjo20]

Sounds great @bobmulder! Saloon v3 is in kind of a "shadow" beta so it's now stable :) I'm going to be writing docs soon but it would be awesome if we could throw this into v3 too!

[bobmulder]

Do you have any deadline for v3, or no pressure?

[Sammyjo20]

Yeah - next couple of weeks I'm afraid. Basically before Ash Allen's book launch (early October) obviously I don't want to put pressure on so if that's unachievable it will be something for v4, unless it can be implemented in a backwards compatbile way? - Thinking about it - it could be right? It's just a wrapper around the response?

[bobmulder]

The first idea (of adding the *Validator classes) is totally backwards compatible since they won't touch any of the existing code (as far as I can see at this point). About the other ideas (providing a RequestValidator within assertSee and combining faking and assertions) I'm not sure but we can leave that be for now. Let's focus on the first part first, which could be added after v3.0!

[Sammyjo20]

Yeah I really like the idea of that @bobmulder

So the RequestValidator will be a wrap around the request - do you think it's a good idea to wrap around the "PendingRequest" instead? Might need some changes to Saloon but I'd be happy to make them . This is just because after a request is sent you may have something merged in on the connector or added by a header - if you're just inspecting the request then it might say the header wasn't present

[Sammyjo20]

From v3 too you'll be able to inspect the PSR-7 request too which is essentially the final stop before its passed to Guzzle