Better named test helper for building conn for unit tests

Moved from https://github.com/phoenixframework/phoenix/issues/3640.

The test helpers for creating a conn suitable for unit testing controller helpers and plugs are named in such a way it’s hard to find them. Even as a relatively experienced Phoenix developer bypass_through means nothing to me and I struggled to find it, and even then struggled to use it correctly.

Before I found it I tried:

  • build_conn()
  • build_conn() |> fetch_flash()
  • build_conn() |> fetch_session() |> fetch_flash()

Before reaching to slack where I was informed of the bypass_through helper and reaching for:

  • build_conn() |> bypass_through()
  • build_conn() |> bypass_through() |> fetch_flash()
  • build_conn() |> bypass_through() |> fetch_session() |> fetch_flash()

Etc etc you get the idea, what finally worked for me is:

build_conn() |> bypass_through(AppWeb.Router, [:pipeline]) |> get("/")

This is pretty ridiculous when all you want to do is assert a module puts a flash or redirects, in my case I was building a set of extracting authentication helpers to allow me to dry up my controllers. bypass_through is completely “non glanceable” when looking to solve this problem.

Ways to improve

My preferred solution for fixing this is to introduce build_conn_with_pipeline/2 that takes the router and the pipeline, its name will mean its at least findable when looking for this feature set, and can quite happily be built from and point to the more composable existing functions.

An alternate solution is to rename/alias bypass_through as something more obvious, run_pipeline maybe? (Although I’d make it not need the get in that scenario).

A final “least effort” solution, would be to improve the the build_conn documentation and/or error message presented by test helpers operating on a raw build_conn to highlight the need to run bypass_through when doing any kind of non endpoint conn testing…

As per the Github issue I’m happy to open a PR for any of these but I can’t afford to waste my limited open source time building something that that’d be rejected, so opening for discussion first.

If the original. deficiency is in the docs, we should start by soliciting and contributing improvements there before introducing another technical abstraction on top of the existing concepts, that may or may not be more accessible to various readers.

FWIW, “Programming Phoenix” has a worked example of using bypass_through to test an authentication plug in isolation in the section “Unit-Testing Plugs” (p141-147 in v1, p167-172 in the most recent).

For me the problematic part is mostly how get(conn, "/") is required to be added by the caller, and it’s not clear what the reasoning for it is. I’m (now after some research) aware of the tech. requirement to do a request, so the endpoint/router is called, but it’s unclear why this is not done by bypass_through directly. Especially as with bypass_through the following get is doing something vastly different (it doesn’t call any controller code, just endpoint + router) than you’d normally expect, so why not hide this completely.

Thats great but personally I don’t believe people should have to buy a book to be able to do things, documentation should be enough and we have examples in the documentation but because of the strange name they are hard to find when you are looking how to do this. There is nothing linking build_conn (the fundamental building block for conn tests) with bypass_through (which is apparently equally fundamental).

I think improving the docs would be a great starting point. Specially in regards to how the conn returned by build_conn is “empty” and how bypass_through can help.

The reason the HTTP method and path is required is because those are often used by plugs in the endpoint and router. Plug.Static being an obvious example. So the pipeline and router alone are not always enough to prime the connection.

I get that, my issue is with the naming / discovery of these. It was not at all obvious to me when working in an abstract sense with tests for a plug without any specific route in mind.

I have since used these functions in their “intended” use case and it seems to me that they are named for the inverse scenario to a normal get (or post etc) e.g. where you want to run the full pipeline except for the result of the final get (e.g. no controller etc). As such I think an alias of some sort is still appropriate