-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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.8] Fix assertJsonMissingValidationErrors() on empty response #28595
[5.8] Fix assertJsonMissingValidationErrors() on empty response #28595
Conversation
b9e45c9
to
313a2a0
Compare
I guess I don't understand. If you already asserted that the response code is a 204 it feels redundant and unnecessary to even check this? |
@taylorotwell No i don't check the status code. /** @test */
public function validates_name_is_unique()
{
$cluster = factory(Cluster::class)->create();
$otherCluster = factory(Cluster::class)->create(['name' => "Existing name"]);
$this->signIn();
// So here i check that cannot update using another cluster name
$this->putJson(route('clusters.update', $cluster), $this->validPayload([
'name' => $otherCluster->name,
]))
->assertJsonValidationErrors('name');
// Here i check i can update the cluster without changing the name
// means the validation unique rule is ignoring the name of the model being update
$this->putJson(route('clusters.update', $cluster), $this->validPayload([
'name' => $cluster->name,
]))
// ------------------------------> This why i made the PR <---------------------------------
// Before the PR i get the error : Invalid JSON was returned from the route.
->assertJsonMissingValidationErrors('name');
} Here i'm not interested in the status code, but in the absence of the error on the |
@taylorotwell Could you reconsider this PR? I've run in to this problem before too. Some api endpoints don't have to return anything, just a 200 status code so the front-end knows the call was successful. If the controller includes validation then you can never use the The reason for adding the If you do plan to merge this PR, i would remove the |
@shadoWalker89 you're performing multiple HTTP requests in a single test method which isn't supported. |
@driesvints Why do you think it's not supported ? Btw, |
@shadoWalker89 You can usually make multiple calls in one test, but it can lead to some weird problems. Take a look at this thread: #27060 |
I have an endpoint where i update a resource and return a http 204 (no content) if the update was successful.
Calling
->assertJsonMissingValidationErrors('some_field')
to check that there is no errors should pass the test, but i get this error.Invalid JSON was returned from the route.
That is because the
TestResponse::json()
method will throw this error when trying to decode a response that is not a valid json.