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

TestResponse, assertSessionHasErrors, getBag() on array #42494

Closed
tanthammar opened this issue May 24, 2022 · 10 comments
Closed

TestResponse, assertSessionHasErrors, getBag() on array #42494

tanthammar opened this issue May 24, 2022 · 10 comments

Comments

@tanthammar
Copy link
Contributor

  • Laravel Version: 9.13.0
  • PHP Version: 8.1.5
  • Database Driver & Version: MySQL 8.0.27
  • Session driver: redis, 6.2.1, "predis" client
  • Laravel Pest 1.21.3
  • PHP unit: 9.5.20

Description:

This test used to work, I never changed it from my initial Laravel, Jetstream (Livewire, teams, pest) installation.

test('password is not confirmed with invalid password', function () {
    $user = User::factory()->create();

    $response = $this->actingAs($user)->post('/user/confirm-password', [
        'password' => 'wrong-password',
    ]);

    $response->assertSessionHasErrors();
});

Now $response->assertSessionHasErrors(); is failing.

Error : Call to a member function getBag() on array
 /Users/tinahammar/Sites/lv9/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:1327
 /Users/tinahammar/Sites/lv9/tests/Feature/User/PasswordConfirmationTest.php:34

I did print out the $errors from the Illuminate/Testing/TestResponse->assertSessionHasErrors(...) method, and $this->session()->get('errors') does return an array:

[
    "default" => array:2 [
        "format" => ":message"
        "messages" => array:1 [
                "password" => array:1 [
                        0 => "The provided password was incorrect."
                ]
          ]
    ]
]

And array doesn't have the getBag() method...

    public function assertSessionHasErrors($keys = [], $format = null, $errorBag = 'default')
    {
        $this->assertSessionHas('errors');

        $keys = (array) $keys;

//dumped
        ray(get_class($this->session())); // Illuminate\Session\Store
        ray($this->session()->get('errors')); // php array
//end dumped

        $errors = $this->session()->get('errors')->getBag($errorBag); // error

        foreach ($keys as $key => $value) {
            if (is_int($key)) {
                PHPUnit::assertTrue($errors->has($value), "Session missing error: $value");
            } else {
                PHPUnit::assertContains(is_bool($value) ? (string) $value : $value, $errors->get($key, $format));
            }
        }

        return $this;
    }
@driesvints
Copy link
Member

I don't get the same issue on a fresh install with pest tests enabled.

@tanthammar
Copy link
Contributor Author

I never figured out why Fortify decides to return an array instead of an error bag, so I had to create my own macro to fix the failing test.
If anyone else has this issue, I added this to my AppServiceProvider boot method.

        if (app()->runningUnitTests()) {
            //based on TestResponse->assertSessionHasErrors, sometimes $errors are an array and not an Error Bag
            TestResponse::macro('assertSessionBagOrArrayHasErrors', function ($keys = [], $format = null, $errorBag = 'default'): self {

                $this->assertSessionHas('errors');

                $keys = (array)$keys;

                $errors = $this->session()->get('errors');

                if (is_array($errors)) {
                    $errors = collect(data_get($errors, $errorBag . '.messages', []));
                    PHPUnit::assertTrue($errors->has($keys), "Session missing error: " . implode(', ', $keys));
                } else {
                    $errors = $errors->getBag($errorBag);
                    foreach ($keys as $key => $value) {
                        if (is_int($key)) {
                            PHPUnit::assertTrue($errors->has($value), "Session missing error: $value");
                        } else {
                            PHPUnit::assertContains(is_bool($value) ? (string)$value : $value, $errors->get($key, $format));
                        }
                    }
                }

                return $this;

            });

@tanthammar
Copy link
Contributor Author

@driesvints Would you accept a PR for the TestResponse->assertSessionHasErrors() method? Based on the above, where the $errors can be an array?

@driesvints
Copy link
Member

@tanthammar I think it's important that we attempt to recreate the situation on a fresh laravel app first to fully understand where this is coming from.

@tanthammar
Copy link
Contributor Author

I'd love to, I just don't know how to trace back to the source of the array conversion. Doing some "dd" in the Fortify controller and in the Fortify password confirm view, the type of $errors is correct. It is only in the test where it becomes an array.

@driesvints
Copy link
Member

Then I feel that we shouldn't take any action as this is only happing for you specifically. I feel we can't warrant a change for that as everything works as expected for a default Laravel app. I'm sorry for that.

@tanthammar
Copy link
Contributor Author

tanthammar commented Jun 7, 2022

@driesvints I finally found what it was.
Some time ago, in some dev version of Laravel, there was an option to serialize session as json.
@Krisell told me, and I activated it to try, and forgot about it ;) ... (Bad move)

config/session 'serialization' => 'json',

With the help of @Krisell who directed me to;
#42090

I also found this quote :-)
image

Anyway. If the option comes back, I know how to solve it in assertSessionHasErrors() or, by the looks of it, the PR might solve it.

And if needed to note, yes, the test works again in its original form.

@Krisell
Copy link
Contributor

Krisell commented Jun 8, 2022

It seems like the start or loadSession methods on the session Store is not called in TestResponse which also means that marshalErrorBag (rebuilding the errors from JSON to the ErrorBag instance) doesn't take place.

Adding $this->session()->start(); in the constructor of TestResponse resolves this, but I'm not sure of the full implications of that. Is it reasonable to call start() before interacting with the session even in TestResponse?

@Krisell
Copy link
Contributor

Krisell commented Jun 8, 2022

The following seems reasonable, and passes all framework tests and also the suite of a mid-sized application.

TestResponse.php

protected function session()
{
    $session = app('session.store');

    if (! $session->isStarted()) {
        $session->start();
    }
    
    return $session;
}

@driesvints
Copy link
Member

@Krisell thanks for investigating 👍

Feel free to attempt a PR with that if you want. Try to provide some background to Taylor why that would be needed.

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

No branches or pull requests

3 participants