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

[8.x] Add bad request test response assertion #38635

Closed
wants to merge 1 commit into from

Conversation

timmartin19
Copy link
Contributor

Inspired by PR #38553. A readability enhancement in the same manner.
Just like assertOk, assertUnauthorized and company this adds an
assertBadRequest to ensure that a 400 Bad Request was sent
in the response.

The intention is to increase readability and add broader support to the
existing suite of response code assertions which are much easier to
understand.

This introduces no breaking changes since it does not modify any
existing methods.

Also introduced a test case for the assert error path and the path where
there is no assertion error.

Inspired by PR laravel#38553.  A readability enhancement in the same manner.
Just like `assertOk`, `assertUnauthorized` and company this adds an
`assertBadRequest` to ensure that a [400 Bad Request](https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.1) was sent
in the response.

The intention is to increase readability and add broader support to the
existing suite of response code assertions which are much easier to
understand.

This introduces no breaking changes since it does not modify any
existing methods.

Also introduced a test case for the assert error path and the path where
there is no assertion error.
Comment on lines +495 to +504
$this->expectException(AssertionFailedError::class);

$this->expectExceptionMessage('Expected response status code');

$baseResponse = tap(new Response, function ($response) use ($statusCode) {
$response->setStatusCode($statusCode);
});

$response = TestResponse::fromBaseResponse($baseResponse);
$response->assertBadRequest();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this set of assertions was repeated a lot in this test class. It was outside the scope of this PR, but would people be interested in maybe converting some of these to using a dataProvider? Not 100% sure how it would look yet but it may help DRY this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously, that would be a separate PR

@taylorotwell
Copy link
Member

I honestly am getting a bit fatigued of these methods 😬 ... I almost always find just using assertStatus(400) easier to understand and read. I don't think I plan to accept anymore of these named methods for a while.

@timmartin19 timmartin19 deleted the add-assert-bad-request branch September 2, 2021 15:17
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.

2 participants