Skip to content
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

PR: Ping to Wake Heroku Dyno #39

Merged
merged 6 commits into from
Jun 2, 2020
Merged

PR: Ping to Wake Heroku Dyno #39

merged 6 commits into from
Jun 2, 2020

Conversation

nelsonic
Copy link
Member

Similar to dwyl/phoenix-content-negotiation-tutorial#5
This does not add any functionality, it's just to wake the Heroku Dyno
so the UX is better for the person reading the tutorial.
Review/merge after #35 👍

@nelsonic
Copy link
Member Author

Well this is lame ...

On localhost I'm seeing:

20:44:20.728 [info]  Already up
...........................

  1) test GET /ping (GIF) renders 1x1 pixel (AppWeb.PingControllerTest)
     test/app_web/controllers/ping_controller_test.exs:5
     ** (CaseClauseError) no case clause matching: "ping"
     code: conn = get(conn, "/ping")
     stacktrace:
       (app 1.5.3) lib/app_web/views/item_view.ex:29: AppWeb.ItemView.filter/2
       (app 1.5.3) lib/app_web/templates/item/index.html.eex:17: AppWeb.ItemView."index.html"/1
       (phoenix 1.5.3) lib/phoenix/view.ex:310: Phoenix.View.render_within/3
       (phoenix 1.5.3) lib/phoenix/view.ex:472: Phoenix.View.render_to_iodata/3
       (phoenix 1.5.3) lib/phoenix/controller.ex:776: Phoenix.Controller.render_and_send/4
       (app 1.5.3) lib/app_web/controllers/item_controller.ex:1: AppWeb.ItemController.action/2
       (app 1.5.3) lib/app_web/controllers/item_controller.ex:1: AppWeb.ItemController.phoenix_controller_pipeline/2
       (phoenix 1.5.3) lib/phoenix/router.ex:352: Phoenix.Router.__call__/2
       (app 1.5.3) lib/app_web/endpoint.ex:1: AppWeb.Endpoint.plug_builder_call/2
       (app 1.5.3) lib/app_web/endpoint.ex:1: AppWeb.Endpoint.call/2
       (phoenix 1.5.3) lib/phoenix/test/conn_test.ex:225: Phoenix.ConnTest.dispatch/5
       test/app_web/controllers/ping_controller_test.exs:6: (test)



Finished in 0.5 seconds
28 tests, 1 failure

Randomized with seed 894831

When I run mix phx.routes I see it there:

ping_path  GET     /ping                                    AppWeb.PingController :ping

After running mix deps.clean --all followed by mix deps.get and then mix test
I see:

===> Compiling parse_trans
==> ping
Compiling 1 file (.ex)

== Compilation error in file lib/ping.ex ==
** (CompileError) lib/ping.ex:2: module Plug.Conn is not loaded and could not be found

could not compile dependency :ping, "mix compile" failed. You can recompile this dependency with "mix deps.compile ping", update it with "mix deps.update ping" or clean it with "mix deps.clean ping"

This is consistent with the error on Travis-CI:
https://travis-ci.com/github/dwyl/phoenix-todo-list-tutorial/builds/168870290#L335
image

So it seems we can't rely on the fact that Plug is included in Phoenix (which is a bummer!)
We have to make Plug an explicit dependency of ping for this to work.

@nelsonic
Copy link
Member Author

This is working on: https://phoenix-content-negotiation.herokuapp.com/ping
image
(the 1x1 pixel is transparent, so it's not visible in this screenshot but it rendered)

@nelsonic
Copy link
Member Author

Ok, I really don't understand this. It works on the other PR: dwyl/phoenix-content-negotiation-tutorial#5 which has virtually identical code.

@nelsonic
Copy link
Member Author

Figured it out!
The /:filter route was taking precedence over the /ping route:

get "/:filter", ItemController, :index
# see: https://github.com/dwyl/ping
get "/ping", PingController, :ping

Moving the /ping route definition above the /:filter:

# see: https://github.com/dwyl/ping
get "/ping", PingController, :ping
get "/", ItemController, :index
resources "/items", ItemController
get "/items/toggle/:id", ItemController, :toggle
get "/clear", ItemController, :clear_completed
# this route will "catch all" so has to be last:
get "/:filter", ItemController, :index

Appears to work now. 👍

@codecov
Copy link

codecov bot commented May 30, 2020

Codecov Report

Merging #39 into master will increase coverage by 8.16%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #39      +/-   ##
===========================================
+ Coverage   91.83%   100.00%   +8.16%     
===========================================
  Files           5         6       +1     
  Lines          49        50       +1     
===========================================
+ Hits           45        50       +5     
+ Misses          4         0       -4     
Impacted Files Coverage Δ
lib/app_web/controllers/ping_controller.ex 100.00% <100.00%> (ø)
lib/app_web/views/item_view.ex 100.00% <100.00%> (ø)
lib/app_web/controllers/item_controller.ex 100.00% <0.00%> (+13.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3449bd1...61a2303. Read the comment docs.

This was referenced May 30, 2020
@nelsonic nelsonic requested a review from SimonLab June 1, 2020 06:44
Copy link
Member

@SimonLab SimonLab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏓

@SimonLab SimonLab merged commit e9572fb into master Jun 2, 2020
@SimonLab SimonLab deleted the ping-to-wake-heroku-dyno branch June 2, 2020 18:32
HotategaiDev added a commit to HotategaiDev/phoenix-todo-list-tutorial- that referenced this pull request Dec 4, 2023
HotategaiDev added a commit to HotategaiDev/phoenix-todo-list-tutorial- that referenced this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants