Replace 'not found' with 'internal server error' in assert_error_sent/2 examples

[phoebe100]

Replace 'not found' with 'internal server error' in assert_error_sent/2 code examples.
This could reduce confusion since "not found" errors are no re-raised.
Closes #5480

[luka-TU]

@phoebe100 sorry, I thought, generally speaking, hitting a missing route should result in a 404 and not in Internal Server Error?

[phoebe100]

hitting a missing route should result in a 404

True, and a running phoenix application will do that.

But when testing controllers, you should differentiate:
a) If you want to verify, that you get back a 404 for a non-existing route, simply use:

conn = get(build_conn(), "/users/not-found")
assert response(conn, 404)

b) If you want to verify, that some error was raised while processing the route and that it was linked to an expected status code, then use assert_error_sent/2

Since requesting unknown routes do not result in (re-)raising an error anymore, assert_error_sent/2is not the right function for testing unknown routes. This commit just tries to clean up the example code in the docs, so that people are not tempted to use that function for testing unknown routes.

[luka-TU]

@phoebe100 I think I get what you are trying to say. What throws me off the the name of the method: assert_error_sent: it suggests that we should expect and error be sent when in reality it is to check if an error was raised. In python this naming is a bit more to the point imho, with assertRaises.

[phoebe100]

it suggests that we should expect and error be sent when in reality it is to check if an error was raised

Actually, it's both. The intention of the function is to assert:

• that an error was raised
• that the error is wrapped in a response with a specific HTTP status
• and a specific content (you can pattern match on the returned response tuple as mentioned in the docs)

So it's all about asserting error translation.

[luka-TU]

@phoebe100 this is just my opinion, but not getting an error raised for a 404 in combination with this method name do cause some confusion.

[stefanchrobot]

not getting an error raised for a 404 in combination with this method name do cause some confusion.

@luka-TU Agreed, hence the original reported issue. I think the docs could still use the 404 example, but it should also have a clear note that NoRouteError is not re-raised and can't be tested with assert_error_sent (I think the note should be added regardless if the docs use the 404 or 500 example).

[luka-TU]

@stefanchrobot the more detailed the docs are - the better, agree. At the same time, maybe the naming of these methods could have been a bit better, borrowing from some other languages like in the example above.

[chrismccord]

❤️❤️❤️🐥🔥