vt-elixir / ja_resource

A behaviour to reduce boilerplate code in your JSON-API compliant Phoenix controllers without sacrificing flexibility.
Other
113 stars 33 forks source link

`assert_error_sent` for 404s Not Working? #72

Closed jherdman closed 6 years ago

jherdman commented 6 years ago

I generated a Phoenix 1.3 controller using ja_resource 0.3, and my DELETE test looks like this:

  describe "delete source" do
    setup [:create_source]

    test "deletes chosen source", %{conn: conn, source: source} do
      conn = delete conn, source_path(conn, :delete, source)

      assert response(conn, 204)

      assert_error_sent 404, fn ->
        get conn, source_path(conn, :show, source)
      end
    end

This test is failing:

  1) test delete source deletes chosen source (DailyBugleWeb.SourceControllerTest)
     test/daily_bugle_web/controllers/source_controller_test.exs:103
     expected error to be sent as 404 status, but response sent 404 without error
     code: assert_error_sent 404, fn ->
     stacktrace:
       (phoenix) lib/phoenix/test/conn_test.ex:600: Phoenix.ConnTest.assert_error_sent/2
       test/daily_bugle_web/controllers/source_controller_test.exs:108: (test

I'm fairly new to Phoenix and Elixir, so I'm not entirely sure what's wrong here. I think the gist is that the subsequent GET request isn't throwing an error. I'd love to pair on a solution as a learning opportunity.

jherdman commented 6 years ago

OK, I think I've narrowed down the problem. In JaResource.Record the method used to get the record doesn't throw an error. If I override the handle_show callback like this:

  def handle_show(_, id) do
    Repo.get!(Source, id)
  end

Everything works as expected.

beerlington commented 6 years ago

Hi @jherdman - this seems to be just something that JaResource is opinionated about. I can't find anything that says the server has to raise an exception when you return a 404. This comment from Chris Mccord makes it sound like there isn't a right or wrong way to do it in Phoenix. I personally just check the status code as Chris suggests, rather than testing explicitly for an exception. I'm happy to reconsider how JaResource handles it though if a case can be made for it.

jherdman commented 6 years ago

This was really helpful, thank you for your response. I'm pretty sure I understand what's going on here a lot more now. I consider this issue closed. I think checking the status code is the better way to go as well.