Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Yet Another) Assertions implementation #124

Closed
wants to merge 9 commits into from

Conversation

claudiodekker
Copy link
Member

@claudiodekker claudiodekker commented Jul 17, 2020

Motivation

Like others, I've been in search of a way to test the backend outputs towards an Inertia view. While Macros are the obvious solution that others have also already discovered & suggested via PR, there were some things there that just didn't feel 'right' to me.

For example, while I do believe #51 to be a great implementation, I wasn't a big fan of the idea of having to swap out the TestCase class in our own projects. Similarly, while the suggestions made in #105 aren't bad either, but having to run Inertia::fake() was kind of a dealbreaker to me.

So, here's my attempt.

Usage

This PR suggests a couple of assertion helpers that are available out of the box once you update to whatever version this PR ships in. To use, simply chain them on your TestResponse responses.

Screenshot 2020-09-02 at 19 44 39

Available Methods

The methods made available by this PR closely reflect those available in Laravel itself.
(In fact, I just got a notification that a few changes I suggested were just merged into the framework)


Assert whether the given page is an Inertia-rendered view

$response->assertInertia();

// or, also check whether the page is a specific component
$response->assertInertia('example');

// or, also check whether all of the given props match
$response->assertInertia('example', [
    'foo' => 'bar'
]);

Return all the available Inertia props for the page

$response->inertiaProps(); 

Assert whether the Inertia-rendered view has a specific property set

$response->assertInertiaHas('key');

// or, against deeply nested values
$response->assertInertiaHas('deeply.nested.key');

Apart from checking whether the property is set, the same method can be used to assert that the values match

$response->assertInertiaHas('key', 'matches-this-value');

// or, for deeply nested values
$response->assertInertiaHas('deeply.nested.key', 'also-match-against-this-value');

It's also possible to assert directly against a Laravel Model (or any other Arrayable or Responsable class)

$user = UserFactory::new()->create(['name' => 'John Doe']);

// ... (Make HTTP request etc.)

$response->assertInertiaHas('user', $user);
$response->assertInertiaHas('deeply.nested.user', $user);

It's also possible to check against a closure

$response->assertInertiaHas('foo', function ($value) {
    return $value === 'bar';
});

// or, again, for deeply nested values
$response->assertInertiaHas('deeply.nested.foo', function ($value) {
    return $value === 'bar';
});

Next, you can also check against a whole array of properties. It'll simply loop over them using the assertInertiaHas method described above:

$response->assertInertiaHasAll([
    'foo',
    'bar.baz',
    'another.nested.key' => 'example-value'
]);

Finally, you can assert that a property was not set:

$response->assertInertiaMissing('key');

// or, for deeply nested values
$response->assertInertiaMissing('deeply.nested.key');

Details & (Breaking) Changes

This PR bootstraps the Macros directly onto the TestResponse from the ServiceProvider, and only does so if we're running in a testing environment.

I've also made it compatible with Laravel 6 and earlier, as that's where the TestResponse namespace was changed from Illuminate\Foundation\Testing\TestResponse to Illuminate\Testing\TestResponse.

Finally, I've changed the package constraints of this repo to be only compatible with Laravel 5.4 and up, because Laravel 5.3 depends upon phpunit/phpunit: ~5.4, which was when the non-namespaced PHPUnit classes (e.g. PHPUnit_Framework_Assert) were still used. (Laravel 5.4 changed this).

While this might seem like a breaking change, I'm fairly confident it isn't, as on those versions other things in this package already likely wouldn't have worked 😅

Let me know if this is something you're happy with, or whether you want me to tweak certain things.
(Or, alternatively, there's no hard feelings if you decide to just decline this PR altogether, so feel free to do so..)

@johanvanhelden
Copy link

johanvanhelden commented Jul 22, 2020

Thanks for the effort! I love the implementation.

How would you handle testing against resources and collections?
In my projects I only pass resources to my Inertia pages to prevent unwanted attributes from bleeding to the front end.

So would something like this be possible?

$projects = factory(Project::class, 5)->states(['public', 'published'])->create();

// snip

$response->assertPropValue('projects.data', ProjectResource::collection($projects->sortByDesc('published_at')));

I've tried myself to make a macro to handle this, but it is quite a pain. Especially when utilizing nested resources, like this:

<?php

// snip

/** @extends JsonResource<\App\Models\Project> */
class ProjectResource extends JsonResource
{
    public function toArray($request)
    {
        return [
            'uuid' => $this->uuid,
            'name' => $this->name,
            
            'owner'   => new UserResource($this->owner),
            'images'  => ImageResource::collection($this->images),

            'urls' => [
                'show' => 'demo.com',
            ],
        ];
    }
}

I just can not get a proper value assert against a resource collection like that.

There is always an object/array typing difference between the prop value and the actual collection in the test.

@claudiodekker
Copy link
Member Author

claudiodekker commented Jul 23, 2020

Interesting use case, I hadn't considered that one.. I'll take a look at it when have some time. Thanks for the feedback!

@claudiodekker
Copy link
Member Author

claudiodekker commented Jul 24, 2020

Alright, so, after letting your question brew for a bit, I realized that normally my answer would be that "you're holding it wrong", because Eloquent API Resources are meant for exactly that: APIs. And since Inertia doesn't really have an API, I would instead recomment to look into using Eloquent's built-in $hidden property to hide attributes from the view.

However, at the same time, @reinink did support Eloquent API Resources as per 2b966ac, so I've still gone ahead and implemented it. Enjoy!


Usage is as you'd expect:

$response->assertInertiaHas('key', EloquentApiResource::collection([...]));

// or, for deeply nested values
$response->assertInertiaHas('deeply.nested.key', EloquentApiResource::make(...));

@johanvanhelden
Copy link

johanvanhelden commented Jul 24, 2020

Thank you @claudiodekker ! I will test it out tomorrow (hopefully).

The reason why I love API Resources so much is because you can create different resources for the same model for different situations, conditionally add data, etc. For example, you can create a ProjectIndexResource for a list of projects and a ProjectResource that is used everywhere else. And if your project has an API, you can add them there as well. Win-win.

It's just easy to use and extremely flexible compared to the the all-or-nothing $hidden.

@johanvanhelden
Copy link

johanvanhelden commented Jul 25, 2020

Reporting back after testing. It seems to work great for single-depth resources. But if I start nesting resources, I can not really get the assertions to work. Could you tell me if I am doing something wrong @claudiodekker ?

I'll try to be as detailed as possible.

This is what it looks like in the Vue inspector:
Screenshot from 2020-07-25 11-57-09

This is what the UserResource looks like:

namespace App\Http\Resources;

use Illuminate\Http\Resources\Json\JsonResource;

class UserResource extends JsonResource
{
    public function toArray($request)
    {
        return [
            'name'  => $this->name,
            'email' => $this->email,

            'roles' => RoleResource::collection($this->whenLoaded('roles')),
        ];
    }
}

And the RoleResource:

namespace App\Http\Resources;

use Illuminate\Http\Resources\Json\JsonResource;

class RoleResource extends JsonResource
{
    public function toArray($request)
    {
        return [
            'name' => $this->name,
        ];
    }
}

Finally, the controller:

class ProfileController extends Controller
{
    public function index()
    {
        return Inertia::render('Profile/Index', [
            'user' => new UserResource(Auth::user()->load('roles')),
        ]);
    }

There is a user object with a roles array inside it. So I assumed this would work:
$response->assertInertiaHas('user.roles', RoleResource::collection($user->roles));

But I get the following error: null does not match expected type "array".

When I try to dump the roles using dump($response->inertiaProps()['user']['roles']); I get:
Cannot use object of type stdClass as array

So I think there is still an issue with nested collections.

This is the test I used:

    /** @test */
    public function shows_the_current_user(): void
    {
        $user = factory(User::class)->state('user')->create(['name' => 'Test Name'])->load('roles');

        $response = $this
            ->actingAs($user)
            ->get(route('profile.index'))

            ->assertSee('Test Name');

        $response->assertInertia(); // == OK
        $response->assertInertiaComponent('Profile/Index'); // == OK
        $response->assertInertiaHas('user', new UserResource($user));  // == OK
        $response->assertInertiaHas('user.roles', RoleResource::collection($user->roles)); // == Error
        dump($response->inertiaProps()['user']['roles']); // == Error
    }

I also do not really understand why the user asserts does work and the user.roles assert fails. I would have expected the user assert to also fail 🤔

P.s. to test this code I dropped my own code to add assertions and noticed an assertInertiaCount was missing. I think this would be a useful addition. ->assertInertiaCount(5, 'projects.data') for example.

@claudiodekker
Copy link
Member Author

claudiodekker commented Jul 26, 2020

Alright, so, I looked into this for you. Here's what's happening:

The problem

The error you're getting (Error: null does not match expected type "array".) occurs because of the JsonResource which Eloquent API Resources (eventually) extend. If we take a look at how Inertia prepares properties for the front-end, you can see it calls $prop = $prop->toResponse($request)->getData();. I'll guide you through the relevant bits of 'call stack' below:

  1. return (new ResourceResponse($this))->toResponse($request);
  2. $this->wrap(...)
  3. $data = [$this->wrapper() => $data];
  4. return get_class($this->resource)::$wrap;

Basically, what happens, is that Laravel looks for the $wrap property on your Resource implementation (e.g. UserResource::class). However, since it most likely won't have one, it'll instead use the inherited property from the JsonResource class that it extends: public static $wrap = 'data';

If we then return/take a step back up the call-stack, you can see that this 'data'-value gets used as the key in a new array, where it's value is the existing instance data (read: the data is wrapped). This then again gets returned, and eventually gets send to the browser as a 'Response' object.

However, since Inertia makes this call instead, it then has a JsonResponse object, which is somewhat useless to it.
So, what it does in addition, is calling ->getData() on it, to get the 'json decoded' representation of the response data. This then gets set on the view as a prop.

Fix number 1 (in your code)

This call:

$response->assertInertiaHas('user.roles', RoleResource::collection($user->roles));

Should actually be:

$response->assertInertiaHas('user.data.roles', RoleResource::collection($user->roles));

Because the prop you set, is only 'user'. Everything below that gets 'wrapped' in the data key by the Resource:

return Inertia::render('Profile/Index', [
    'user' => new UserResource(Auth::user()->load('roles')),
]);

However, doing this, while more correct, still wouldn't make ANY behavioural difference. Here's why:

If we take a look at that ->getData() call earlier, you can see it plainly makes a json_decode(...) call, with the result being an object instead of an associative array (e.g.: json_decode(..., true)).

Since this PR (as well as Laravel's ->assertHas) uses Arr::get() and Arr::has() to navigate through array properties using dot-notation, and since it cannot (or rather, refuses to) walk through objects, this is where it instead gives back null, which is the default fallback-value in case a key is not found.

This also explains the other error you're experiencing: Cannot use object of type stdClass as array.
To get that working, use this instead:

dump($response->inertiaProps()['user']->data->roles);

"Fix" number 2 (in Inertia.. Spoiler alert: Not needed.)

The solution appears to be to change the way Inertia itself prepares Responsable data to the view, by making a tiny one-line change that effects pretty much nothing else, because further down the line (right now) this stdClass gets re-encoded to JSON anyway:

- $prop = $prop->toResponse($request)->getData();
+ $prop = $prop->toResponse($request)->getData(true);

That way, the Arr::get() and Arr::has() helpers can walk over the properties, deeply nested and everything (albeit with the data-wrapper in between).

However, even if we make this change, it only places us in an entirely different problem space:

Due to the serialization that happens, contexts gets lost, so, doing something like this:

$response->assertInertiaHas('example.data.children', $children);

will give you the following error:
Screenshot 2020-07-26 at 19 47 48

Alternatively, if we try to match against a child resource that is a JsonResource, such as this:

$response->assertInertiaHas('example.data.children', UserResource::collection($children));

We'll still get this error, because of the wrapping that I explained earlier:
Screenshot 2020-07-26 at 19 49 57

But wait..

Since we're asserting against what is essentially a deeply-nested array, shouldn't the top-level comparison already take care of this? Because if that's the case, we don't even need to assert against (deeply) nested resources/collections:

Screenshot 2020-07-26 at 20 00 28
Screenshot 2020-07-26 at 20 02 56

And it appears it does! You can validate this behaviour yourself, using a slightly modified example of your own test:

    /** @test */
    public function shows_the_current_user(): void
    {
        $user = factory(User::class)->state('user')->create(['name' => 'Test Name']);
        // Let's fetch the objects twice, so that we that they're reliable test subjects:
        $userA = User::find($user->id)->load('roles');
        $userB = User::find($user->id);

        $response = $this
            ->actingAs($user)
            ->get(route('profile.index'))
            ->assertSee('Test Name');

        $response->assertInertia(); // == OK
        $response->assertInertiaComponent('Profile/Index'); // == OK
        $response->assertInertiaHas('user', new UserResource($userA));  // == OK
        $response->assertInertiaHas('user', new UserResource($userB)); // == Error ('roles' not loaded)
        dump($response->inertiaProps()['user']->data->roles); // == OK
    }

@claudiodekker
Copy link
Member Author

claudiodekker commented Sep 2, 2020

Added IDE testing autocompletion helpers to this branch of laravel-inertia, so if you're using it and would like to take advantage of them, I suggest you run composer update 😎

Screenshot 2020-09-02 at 19 44 39

@johanvanhelden
Copy link

Amazing! Thanks for the work @claudiodekker ! I really hope it gets merged!

@claudiodekker
Copy link
Member Author

@johanvanhelden For now, you can require this package instead of using this fork.
I needed it in a production app and didn't want to rely on a fork of inertia, so decided to put it in a package 😅

@johanvanhelden
Copy link

@johanvanhelden For now, you can require this package instead of using this fork.
I needed it in a production app and didn't want to rely on a fork of inertia, so decided to put it in a package sweat_smile

Oh, sounds good, thanks for making a package!

@innocenzi
Copy link
Contributor

Good call making it a package, thanks @claudiodekker! I added it to awesome-inertia until it gets deprecated.

@johanvanhelden
Copy link

@claudiodekker since you seem to be working more closely with @reinink , do you know if there are any plans to make this part of the core now?

@claudiodekker
Copy link
Member Author

claudiodekker commented Nov 18, 2020

Hi @johanvanhelden, that's true, but for now we're fairly comfortable with keeping this a separate package, which you can consider semi-official and can expect to remain up to date with Inertia's changes.

That said, if and when we make this part of Inertia's core, I'll let you know by adding a message to this issue.

@claudiodekker
Copy link
Member Author

claudiodekker commented Mar 2, 2021

Superseded by #220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants