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

[5.7] Fix assertSessionDoesntHaveErrors() when there is no errors #27145

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

shadoWalker89
Copy link
Contributor

@shadoWalker89 shadoWalker89 commented Jan 12, 2019

When the response has no errors at all and we do

$response->assertSessionDoesntHaveErrors('can_be_invited_as_reviewer');

A php error will occur
Error : Call to a member function getBag() on null F:\projects\quick-conference\src\quick-conference\vendor\laravel\framework\src\Illuminate\Foundation\Testing\TestResponse.php:904

The reason is that the assertion method is not taking into consideration someone checking that a specific error is missing when there is no errors at all.

Having this in a test

$response->assertSessionDoesntHaveErrors('can_be_invited_as_reviewer');

intead of this

$response->assertSessionDoesntHaveErrors();

makes it clearer that the test is checking that the can_be_invited_as_reviewer is missing.

Edit: btw the assertJsonMissingValidationErrors() allows for the behavior that the PR is adding, this will make things more consistent.

@taylorotwell
Copy link
Member

Might be better to actually make an assertion? Even assertTrue(true) just so PHPUnit doesn't confusingly complain about no assertions being made in a test that only calls this method.

@shadoWalker89
Copy link
Contributor Author

You are totally right. Change applied.

@taylorotwell taylorotwell merged commit 87ca267 into laravel:5.7 Jan 14, 2019
@shadoWalker89 shadoWalker89 deleted the patch-1 branch January 14, 2019 16:25
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