Skip to content

[9.x] change status code assertions on TestResponseTest. #44661

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

Closed
wants to merge 1 commit into from
Closed

[9.x] change status code assertions on TestResponseTest. #44661

wants to merge 1 commit into from

Conversation

rschoonheim
Copy link

Hello People,

Earlier today I noticed a pull request (#44658) containing a test case I didn't understand. The author copied a test case that was testing another assertion on the TestResponse object. I noticed there were many test cases that were expecting a status code 500 - server error while their assertions expected something else and were passing.

This pull request changes the status codes of the test cases and removes redundant exception expectations.

@taylorotwell
Copy link
Member

taylorotwell commented Oct 19, 2022

Those tests are testing that assertions fail on a non matching status code though?

@rschoonheim
Copy link
Author

Those tests are testing that assertions fail on a non matching status code though?

Didn't thought that way. My expectation by testAssertOk is testing of the AssertOk functions passes I would expect another function testAssertOkFails the failing behaviour.

Might be a personal preference making it a none issue.

Thanks @taylorotwell!

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