Provide sent/2, sent_html/2, etc in connection test

[josevalim]

Today, we want at least to assert three things in a controller test:

1. What is the status code
2. What is the content type
3. What is the response body

The only way to do this if with 3 assertions today:

assert conn.status == 200
assert get_resp_header(conn, "content-type") =~ "text/html"
assert conn.resp_body =~ "some body"

This approach is, however, very verbose. I would like to introduce some helpers to make it easier for us to assert those things. For example, we could rewrite all above as:

assert sent_html(conn, 200) =~ "some body"

sent_html/2 checks if a response was sent, checks the status code, the content type and returns the response body. We can also provide sent_json/2, similar to above, but that also already decodes the response:

body = sent_json(conn, 200)
assert body["errors"] == ...

Both those helpers will be built on top of sent/2, which works as above, except it doesn't check the content type.

Finally, we should also have a helper for redirected_to, which has already been added to master:

assert redirected_to(conn) == "/path/it/was/redirected/to"
assert redirected_to(conn, 301) == "/path/it/was/redirected/to"

Those functions can also help us in providing better error messages until we improve the reporting mechanism in ExUnit. Thoughts? /cc @chrismccord @parkerl @batate

[chrismccord]

I like these a lot :+1

On Apr 14, 2015, at 5:20 AM, José Valim notifications@github.com wrote:

Today, we want at least to assert three things in a controller test:

What is the status code What is the content type What is the response body The only way to do this if with 3 assertions today:

assert conn.status == 200 assert get_resp_header(conn, "content-type") =~ "text/html" assert conn.resp_body =~ "some body" This approach is, however, very verbose. I would like to introduce some helpers to make it easier for us to assert those things. For example, we could rewrite all above as:

assert sent_html(conn, 200) =~ "some body" sent_html/2 checks if a response was sent, checks the status code, the content type and returns the response body. We can also provide sent_json/2, similar to above, but that also already decodes the response:

body = sent_json(conn, 200) assert body["errors"] == ... Both those helpers will be built on top of sent/2, which works as above, except it doesn't check the content type.

Finally, we should also have a helper for redirected_to, which has already been added to master:

assert redirected_to(conn) == "/path/it/was/redirected/to" assert redirected_to(conn, 301) == "/path/it/was/redirected/to" Those functions can also help us in providing better error messages until we improve the reporting mechanism in ExUnit. Thoughts? /cc @chrismccord @parkerl @batate

— Reply to this email directly or view it on GitHub.

[batate]

I like the concepts very much too.

I am not yet convinced on the name or the rollup in sent_html... it reads poorly and perhaps doesn't separate concerns enough. Perhaps we could base the assertions on contents of the conn, and provide the various shorthands that would make this beautiful.

Maybe have options for sent (I think response or responds_with is better) which would allow users to test any or all of the responses, like this:

assert response(conn, code: 200/201/:success, body: some_regexp, type: :html/:json, etc, )

I think this type of response would allow quite a rich assertion. It very much fits what we're trying to do with exunit revisions.

It would also be pretty easy to extend. We could allow configuration in test-helper to allow other key/value pairs.

That could also assert redirection status as well, and help to abbreviate some of the concepts we use in tests. For example

or

assert response(conn, code: :redirect, to: some/full/path/with/or/without/host, ...)

I really like this concept, Jose.

-bt

On Tue, Apr 14, 2015 at 5:48 AM, Chris McCord notifications@github.com wrote:

I like these a lot :+1

On Apr 14, 2015, at 5:20 AM, José Valim notifications@github.com wrote:

Today, we want at least to assert three things in a controller test:

What is the status code What is the content type What is the response body The only way to do this if with 3 assertions today:

assert conn.status == 200 assert get_resp_header(conn, "content-type") =~ "text/html" assert conn.resp_body =~ "some body" This approach is, however, very verbose. I would like to introduce some helpers to make it easier for us to assert those things. For example, we could rewrite all above as:

assert sent_html(conn, 200) =~ "some body" sent_html/2 checks if a response was sent, checks the status code, the content type and returns the response body. We can also provide sent_json/2, similar to above, but that also already decodes the response:

body = sent_json(conn, 200) assert body["errors"] == ... Both those helpers will be built on top of sent/2, which works as above, except it doesn't check the content type.

Finally, we should also have a helper for redirected_to, which has already been added to master:

assert redirected_to(conn) == "/path/it/was/redirected/to" assert redirected_to(conn, 301) == "/path/it/was/redirected/to" Those functions can also help us in providing better error messages until we improve the reporting mechanism in ExUnit. Thoughts? /cc @chrismccord @parkerl @batate

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/phoenixframework/phoenix/issues/767#issuecomment-92765501 .

Bruce Tate President, RapidRed, LLC Phone: 512.772.4312 Fax: 512 857-0415

Author of Seven Languages in Seven Weeks, Deploying Rails Applications, From Java to Ruby, Rails: Up and Running, Beyond Java, 6 others.

[chrismccord]

The issue I have with a response function is it becomes less obvious what is and isn't supported. If a user is browsing our test docs/code, it would be nice to have an at-a-glance look into what functions they can use and when. Other than a listing of great examples, a catch-all response is going to be less clear. @batate You make a good point about extension. I think José's proposed functions would allow you to wrap up the via the map approach if you needed it, but it would be a lot more hand rolling of sticking functions together.

[josevalim]

I have talked to @batate and the main concern was with the name. assert sent_html(conn, ...) does not read nicely. At the end, we have arrived to the following:

assert response(conn, 200)
assert html_response(conn, 200) =~ "<html>"
assert "can't be blank" in json_response(conn, 200)["errors"]

So instead of sent, we are using response. The functions work as previously described. This is also semantically better because we were not effectively asserting that something was sent, but if a response was set or sent (Plug supports both).

Bruce also mentioned we should support the status code as an atom. redirected_to/2 will be the same as previously proposed.

Thoughts?

[batate]

Thanks Jose for a great idea. I like this API a lot.

To clarify, both types of codes would be supported: exact code:

assert response(conn, 200) =~ ... assert response(conn, 201) =~ ...

or

assert response(conn, :success) =~ ...

which means the code is between 200 and 299, inclusive.

This allows a less restrictive test, supporting both redirect permanently and redirect temporarily return codes, for example.

-bt

On Tue, Apr 14, 2015 at 10:18 AM, José Valim notifications@github.com wrote:

I have talked to @batate https://github.com/batate and the main concern was with the name. assert sent_html(conn, ...) does not read nicely. At the end, we have arrived to the following:

assert response(conn, 200) assert html_response(conn, 200) =~ "" assert "can't be blank" in json_response(conn, 200)["errors"]

So instead of sent, we are using response. The functions work as previously described. This is also semantically better because we were not effectively asserting that something was sent, but if a response was set or sent (Plug supports both).

Bruce also mentioned we should support the status code as an atom. redirected_to/2 will be the same as previously proposed.

Thoughts?

— Reply to this email directly or view it on GitHub https://github.com/phoenixframework/phoenix/issues/767#issuecomment-92902422 .

Bruce Tate President, RapidRed, LLC Phone: 512.772.4312 Fax: 512 857-0415

Author of Seven Languages in Seven Weeks, Deploying Rails Applications, From Java to Ruby, Rails: Up and Running, Beyond Java, 6 others.

[chrismccord]

:+1: on response and friends. I also like the option to use atoms for the status

[duijf]

Hi all,

New here, but I'd love to try and implement this tomorrow if it isn't already taken :)

[chrismccord]

@josevalim Is already on it