Migration de Phoenix 1.4.10 à 1.5.7 (#1457)

* Start upgrading Phoenix * Follow changelog advice * Fix deprecation warning * Fix deprecation warning * Adapt code for deprecation (pubsub) * Adapt layout rendering as recommended * Upgrade phoenix_html_sanitizer * Fix difference seen in PhoenixDiff This is not required right now, but could be confusing to others (or myself) later. * Upgrade phoenix_live_reload * Upgrade gettext * Update timex to remove compilation warnings * Add git to the CircleCI build * Use alpine-compatible git install command * Add missing openssh-client package * Try to work around CircleCI build problem * Update eternal to fix deprecation warning * Fix duplicate init_test_session/2 error * Log where the git binary is * Attempt to save the git binary * Fix incantation to share the git binary * Add quickfix for missing git binary Until etalab/transport-ops#19 is handled. * Fix major blocking point Many tests would previously fail with the following error: ``` function Phoenix.Template.HTML.encode_to_iodata!/1 is undefined (module Phoenix.Template.HTML is not available) ``` After a fair bit of digging, I found that the default config today is different: https://github.com/jayjun/phoenix/blob/master/lib/phoenix/template.ex#L223-L225 `Phoenix.HTML.Engine` should be used instead of `Phoenix.Template.HTML`. I am removing the entry completely since: - `html` will be handled fine with the default configuration - `json` will leverage `Phoenix.json_library`, which is configured just above * Add useful boilerplate for later debugging * Remove fetch_flash since we already have fetch_live_flash The default boilerplate does not use both, and fetch_live_flash includes the previous one (https://github.com/phoenixframework/phoenix_live_view/blob/v0.15.4/lib/phoenix_live_view/router.ex#L132). * Remove unused resource * Bump up Elixir dependency * Remove TODO for credo * Bump phoenix_live_view to latest * Try to fix solution tests by updating the chrome container * Update hound to latest * Stop using dialyxir release candidate & adjust configuration * Try to save some time during build debugging The build fails at PLT and I want to debug that, but each build will also recompiled dev + test each time, so I'm setting intermediate snapshots to hopefully shave off 7 minutes per build. * Fix deprecation warning * Attempt to reduce time taken by PLT construction for now * Add note * Fix linter error * Bump up yarn dependencies * Make browser window size deterministic (fixes #1504) * set chrome driver version to 86 As 88 make find_element function fail. * Add useful doc * Switch to dialyxir master version to get plt_local_path option * Save PLT files to a well-defined place * Ignore PLT files * Attempt to optimize build time (#1503, #1505) * Allow 20 minutes without STDOUT output for Dialyzer PLT construction * Attempt to fix dev/test compilation caching inefficience * Empty commit to trigger CI build * Revert to app_tree Co-authored-by: Francis <[email protected]>
etalab · Feb 18, 2021 · 54c6ee3 · 54c6ee3
1 parent 05de718
commit 54c6ee3
Show file tree

Hide file tree

Showing 18 changed files with 126 additions and 59 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -1,5 +1,7 @@
 version: 2
 
+# useful information at https://circleci.com/docs/2.0/language-elixir/
+
 default_docker: &default_docker
   image: betagouv/transport:0.4.5
   environment:
@@ -28,7 +30,7 @@ defaults_with_postgres_and_phantom: &defaults_with_postgres_and_phantom
   <<: *defaults
   docker:
     - *default_docker
-    - image: selenium/standalone-chrome:3.141.59-oxygen
+    - image: selenium/standalone-chrome:86.0
     - image: mdillon/postgis:11-alpine
       environment:
         POSTGRES_USER: root
@@ -41,14 +43,19 @@ jobs:
     steps:
       - checkout
 
+      # TODO: remove this once https://github.com/etalab/transport-ops/issues/19 is solved
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - restore_cache:
           keys:
-            - elixir-v8-{{ checksum "mix.lock" }}
-
+            - elixir-v9-build-{{ checksum "mix.lock" }}
+  
       - run:
           name: Install hex
           command: mix local.hex --force
-      
+
       - run:
           name: Install rebar
           command: mix local.rebar --force
@@ -69,6 +76,13 @@ jobs:
           name: Compile mix dependencies for MIX_ENV=test
           command: mix deps.compile
 
+      - save_cache:
+          key: elixir-v9-build-{{ checksum "mix.lock" }}
+          paths:
+            - ~/transport/_build
+            - ~/transport/deps
+            - ~/.mix
+
       - run:
           name: Install yarn dependencies
           command: cd ~/transport/apps/transport/client && yarn install
@@ -77,29 +91,42 @@ jobs:
           name: Compile assets
           command: cd ~/transport/apps/transport/client && npm run deploy
 
+      # NOTE: I think this should be moved to the dialyzer parallel section,
+      # but then we need to think about how the persistence/caching will be
+      # handled, and think about concurrent access (?) with various jobs to the cache.
+
+      - restore_cache:
+          keys:
+            - elixir-v8-plt-{{ checksum "mix.lock" }}
+
       - run:
           name: Build PLT
           command: MIX_ENV=dev mix dialyzer --plt
+          # PLT construction can stay up quite a bit without generating any output
+          # We add a bit of tolerance here
+          no_output_timeout: 20m
 
+      - save_cache:
+          key: elixir-v8-plt-{{ checksum "mix.lock" }}
+          paths:
+            - ~/transport/dialyzer-plt
+
       - persist_to_workspace:
           root: ~/transport
           paths:
             - .
 
-      - save_cache:
-          key: elixir-v8-{{ checksum "mix.lock" }}
-          paths:
-            - ~/transport/_build
-            - ~/transport/deps
-            - ~/.mix
-
   test_lint:
     <<: *defaults
 
     steps:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run linters
           command: |
@@ -125,6 +152,10 @@ jobs:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run dialyzer (static analysis)
           command: MIX_ENV=dev mix dialyzer
@@ -135,6 +166,10 @@ jobs:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run tests
           command: |
@@ -148,6 +183,10 @@ jobs:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run tests
           command: |
@@ -161,6 +200,10 @@ jobs:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run tests
           command: |
@@ -174,6 +217,10 @@ jobs:
       - attach_workspace:
           at: ~/transport
 
+      - run:
+          name: Install git (required if we use git-hosted hex packages)
+          command: apk add --update git
+
       - run:
           name: Run tests
           command: |

diff --git a/.gitignore b/.gitignore
@@ -37,3 +37,6 @@ apps/transport/priv/static/js/*
 # the blog build folder
 /blog/public
 .clever.json
+
+dialyzer-plt/*.plt
+dialyzer-plt/*.plt.hash
diff --git a/apps/gbfs/mix.exs b/apps/gbfs/mix.exs
@@ -33,7 +33,7 @@ defmodule GBFS.MixProject do
       {:cachex, "~> 3.3"},
       {:httpoison, "~> 1.5.0"},
       {:exshape, "~> 2.2.6"},
-      {:phoenix, "~> 1.4"},
+      {:phoenix, "~> 1.5.7"},
       {:iconv, "~> 1.0.10"},
       {:sweet_xml, ">= 0.0.0"},
       {:jason, ">= 0.0.0"},

diff --git a/apps/gbfs/test/support/conn_case.ex b/apps/gbfs/test/support/conn_case.ex
@@ -19,7 +19,8 @@ defmodule GBFS.ConnCase do
   using do
     quote do
       # Import conveniences for testing with connections
-      use Phoenix.ConnTest
+      import Plug.Conn
+      import Phoenix.ConnTest
       alias GBFS.Router.Helpers, as: Routes
 
       # The default endpoint for testing

diff --git a/apps/transport/client/javascripts/app.js b/apps/transport/client/javascripts/app.js
@@ -4,3 +4,10 @@ import LiveSocket from 'phoenix_live_view'
 const csrfToken = document.querySelector('meta[name=\'csrf-token\']').getAttribute('content')
 const liveSocket = new LiveSocket('/live', Socket, { params: { _csrf_token: csrfToken } })
 liveSocket.connect()
+
+// expose liveSocket on window for web console debug logs and latency simulation:
+// >> liveSocket.enableDebug()
+// >> liveSocket.enableLatencySim(1000)  // enabled for duration of browser session
+// >> liveSocket.disableLatencySim()
+
+// window.liveSocket = liveSocket
diff --git a/apps/transport/client/yarn.lock b/apps/transport/client/yarn.lock
@@ -4827,13 +4827,13 @@ performance-now@^2.1.0:
   integrity sha1-Ywn04OX6kT7BxpMHrjZLSzd8nns=
 
 "phoenix@file:../../../deps/phoenix":
-  version "1.4.17"
+  version "1.5.7"
 
 "phoenix_html@file:../../../deps/phoenix_html":
   version "2.14.3"
 
 "phoenix_live_view@file:../../../deps/phoenix_live_view":
-  version "0.14.0"
+  version "0.15.4"
 
 picomatch@^2.0.4, picomatch@^2.0.5, picomatch@^2.2.1:
   version "2.2.2"

diff --git a/apps/transport/lib/transport/application.ex b/apps/transport/lib/transport/application.ex
@@ -16,7 +16,8 @@ defmodule Transport.Application do
         supervisor(TransportWeb.Endpoint, []),
         supervisor(ImportDataWorker, []),
         CSVDocuments,
-        SearchCommunes
+        SearchCommunes,
+        {Phoenix.PubSub, [name: TransportWeb.PubSub, adapter: Phoenix.PubSub.PG2]}
       ]
       |> add_scheduler()
       ## manually add a children supervisor that is not scheduled

diff --git a/apps/transport/lib/transport_web.ex b/apps/transport/lib/transport_web.ex
@@ -36,7 +36,7 @@ defmodule TransportWeb do
         namespace: TransportWeb
 
       # Import convenience functions from controllers
-      import Phoenix.Controller, only: [get_flash: 2, view_module: 1, get_csrf_token: 0]
+      import Phoenix.Controller, only: [get_flash: 2, view_module: 1, get_csrf_token: 0, view_template: 1]
 
       # Use all HTML functionality (forms, tags, etc)
       use TransportWeb.InputHelpers

diff --git a/apps/transport/lib/transport_web/router.ex b/apps/transport/lib/transport_web/router.ex
@@ -11,7 +11,6 @@ defmodule TransportWeb.Router do
   pipeline :browser do
     plug(:accepts, ["html"])
     plug(:fetch_session)
-    plug(:fetch_flash)
     plug(:fetch_live_flash)
     plug(:protect_from_forgery)
     plug(:put_secure_browser_headers)

diff --git a/apps/transport/lib/transport_web/templates/layout/app.html.eex b/apps/transport/lib/transport_web/templates/layout/app.html.eex
@@ -58,7 +58,7 @@
         <% end %>
       <% end %>
 
-      <%= render(@view_module, @view_template, assigns) %>
+      <%= @inner_content %>
     </main>
 
     <%= render(LayoutView, "_footer.html", assigns) %>

diff --git a/apps/transport/mix.exs b/apps/transport/mix.exs
@@ -5,7 +5,7 @@ defmodule Transport.Mixfile do
     [
       app: :transport,
       version: "0.0.1",
-      elixir: "~> 1.8",
+      elixir: "~> 1.10",
       elixirc_paths: elixirc_paths(Mix.env()),
       build_path: "../../_build",
       config_path: "../../config/config.exs",
@@ -47,20 +47,21 @@ defmodule Transport.Mixfile do
       {:exconstructor, "~> 1.1"},
       {:gettext, "~> 0.11"},
       {:httpoison, "~> 1.5"},
-      {:phoenix, "~> 1.4.10"},
+      {:phoenix, "~> 1.5.7"},
       {:phoenix_html, "~> 2.13"},
       {:phoenix_markdown, "~> 1.0"},
-      {:phoenix_live_view, "~> 0.14.0"},
+      {:phoenix_live_view, "~> 0.15.0"},
       {:phoenix_html_sanitizer, "~> 1.1.0"},
       {:floki, ">= 0.0.0", only: :test},
-      {:plug_cowboy, "~> 1.0"},
+      {:plug_cowboy, "~> 2.1"},
       {:recon, "~> 2.4"},
       {:vex, "~> 0.8"},
       {:quantum, "~> 2.3"},
       {:timex, "~> 3.5"},
       {:sentry, "~> 7.1"},
       {:scrivener, "~> 2.5"},
-      {:scrivener_html, "~> 1.8"},
+      # We'll use upstream again once https://github.com/mgwidmann/scrivener_html/pull/98 is merged
+      {:scrivener_html, git: "https://github.com/thbar/scrivener_html.git", branch: "phoenix_1.5"},
       {:scrivener_list, "~>2.0"},
       {:jason, "~> 1.1"},
       {:open_api_spex, "~> 3.8"},

diff --git a/apps/transport/test/support/channel_case.ex b/apps/transport/test/support/channel_case.ex
@@ -18,7 +18,7 @@ defmodule TransportWeb.ChannelCase do
   using do
     quote do
       # Import conveniences for testing with channels
-      use Phoenix.ChannelTest
+      import Phoenix.ChannelTest
 
       # The default endpoint for testing
       @endpoint TransportWeb.Endpoint

diff --git a/apps/transport/test/support/conn_case.ex b/apps/transport/test/support/conn_case.ex
@@ -19,7 +19,12 @@ defmodule TransportWeb.ConnCase do
   using do
     quote do
       # Import conveniences for testing with connections
-      use Phoenix.ConnTest
+      import Plug.Conn
+      # NOTE: init_test_session delegates to Plug.Test as seen here,
+      # https://github.com/phoenixframework/phoenix/blob/v1.5.7/lib/phoenix/test/conn_test.ex#L253
+      # which is also imported, hence generating a compilation error.
+      # This is due to how we configured the various "cases" I believe, and we'll have to clean that up.
+      import Phoenix.ConnTest, except: [init_test_session: 2]
       import TransportWeb.Router.Helpers
 
       # The default endpoint for testing

diff --git a/apps/transport/test/support/user_facing_case.ex b/apps/transport/test/support/user_facing_case.ex
@@ -9,7 +9,10 @@ defmodule TransportWeb.UserFacingCase do
   using do
     quote do
       use Hound.Helpers
-      hound_session()
+      # NOTE: use a deterministic window size to make sure we do not meet failures
+      # due to find_element needing "below the fold" search
+      # See https://github.com/HashNuke/hound/issues/186
+      hound_session(driver: %{chromeOptions: %{args: ["--window-size=1024,768"]}})
     end
   end
 end
diff --git a/config/config.exs b/config/config.exs
@@ -14,7 +14,7 @@ config :gbfs, jcdecaux_apikey: System.get_env("JCDECAUX_APIKEY")
 config :gbfs, GBFS.Endpoint,
   secret_key_base: System.get_env("SECRET_KEY_BASE"),
   render_errors: [view: GBFS.ErrorView, accepts: ~w(json)],
-  pubsub: [name: GBFS.PubSub, adapter: Phoenix.PubSub.PG2],
+  pubsub_server: GBFS.PubSub, # TODO: verify if this is truly needed? unsure.
   server: false
 
 # Configures the endpoint
@@ -26,19 +26,13 @@ config :transport, TransportWeb.Endpoint,
     layout: {TransportWeb.LayoutView, "app.html"},
     accepts: ~w(html json)
   ],
-  pubsub: [name: Transport.PubSub,
-           adapter: Phoenix.PubSub.PG2],
+  pubsub_server: TransportWeb.PubSub,
   live_view: [
     signing_salt: System.get_env("SECRET_KEY_BASE")
   ]
 
 config :phoenix, :json_library, Jason
 
-# Configures format encoders
-config :phoenix, :format_encoders,
-  html: Phoenix.Template.HTML,
-  json: Jason
-
 # Configures Elixir's Logger
 config :logger,
   handle_otp_reports: true,

diff --git a/dialyzer-plt/.gitkeep b/dialyzer-plt/.gitkeep
diff --git a/mix.exs b/mix.exs
@@ -8,13 +8,19 @@ defmodule Transport.MixProject do
       deps: deps(),
       aliases: aliases(),
       test_coverage: [tool: ExCoveralls],
-      dialyzer: [plt_add_deps: :transitive, plt_add_apps: [:mix]]
+      dialyzer: [
+        plt_add_deps: :app_tree,
+        plt_add_apps: [:mix],
+        plt_local_path: "dialyzer-plt",
+        plt_core_path: "dialyzer-plt"
+      ]
     ]
   end
 
   defp deps do
     [
-      {:dialyxir, "~> 1.0.0-rc.7", runtime: false},
+      # NOTE: using master to be able to use `plt_local_path` option
+      {:dialyxir, "~> 1.0.0", only: :dev, runtime: false, git: "https://github.com/jeremyjh/dialyxir.git"},
       {:excoveralls, "~> 0.10", only: :test}
     ]
   end