Merge pull request #100 from PSPDFKit-labs/credo

Improve code based on credo feedback
PSPDFKit-labs · Aug 20, 2020 · 87bd9f0 · 87bd9f0
2 parents c88c867 + 8d096da
commit 87bd9f0
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 55 deletions.
diff --git a/lib/bypass.ex b/lib/bypass.ex
@@ -1,28 +1,28 @@
 defmodule Bypass do
   @moduledoc """
-  Bypass provides a quick way to create a custom Plug that can be put
-  in place instead of an actual HTTP server to return prebaked responses
-  to client requests.
+  Bypass provides a quick way to create a custom Plug that can be put in place
+  instead of an actual HTTP server to return prebaked responses to client
+  requests.
 
   This module is the main interface to the library.
   """
 
   defstruct pid: nil, port: nil
 
   @typedoc """
-  Represents a Bypass server process
+  Represents a Bypass server process.
   """
   @type t :: %__MODULE__{pid: pid, port: non_neg_integer}
 
   import Bypass.Utils
   require Logger
 
   @doc """
-  Starts an Elixir process running a minimal Plug app. The process
-  is a HTTP handler and listens to requests on a TCP port on localhost.
+  Starts an Elixir process running a minimal Plug app. The process is a HTTP
+  handler and listens to requests on a TCP port on localhost.
 
-  Use the other functions in this module to declare which requests are
-  handled and set expectations on the calls.
+  Use the other functions in this module to declare which requests are handled
+  and set expectations on the calls.
   """
   def open(opts \\ []) do
     case DynamicSupervisor.start_child(Bypass.Supervisor, Bypass.Instance.child_spec(opts)) do
@@ -38,21 +38,18 @@ defmodule Bypass do
     end
   end
 
-  # Raise an error if called with an unknown framework
-  #
   defp setup_framework_integration(:ex_unit, bypass = %{pid: pid}) do
     ExUnit.Callbacks.on_exit({Bypass, pid}, fn ->
       do_verify_expectations(bypass.pid, ExUnit.AssertionError)
     end)
   end
 
   defp setup_framework_integration(:espec, _bypass) do
-    # Entry point for more advanced ESpec configurations
   end
 
   @doc """
-  Can be called to immediately verify if the declared request
-  expectations have been met.
+  Can be called to immediately verify if the declared request expectations have
+  been met.
 
   Returns `:ok` on success and raises an error on failure.
   """
@@ -104,16 +101,15 @@ defmodule Bypass do
   end
 
   @doc """
-  Re-opens the TCP socket on the same port.
-  Blocks until the operation is complete.
+  Re-opens the TCP socket on the same port. Blocks until the operation is
+  complete.
   """
   @spec up(Bypass.t()) :: :ok | {:error, :already_up}
   def up(%Bypass{pid: pid}),
     do: Bypass.Instance.call(pid, :up)
 
   @doc """
-  Closes the TCP socket.
-  Blocks until the operation is complete.
+  Closes the TCP socket. Blocks until the operation is complete.
   """
   @spec down(Bypass.t()) :: :ok | {:error, :already_down}
   def down(%Bypass{pid: pid}),

diff --git a/lib/bypass/application.ex b/lib/bypass/application.ex
@@ -1,4 +1,6 @@
 defmodule Bypass.Application do
+  @moduledoc false
+
   use Application
 
   def start(_type, _args) do

diff --git a/lib/bypass/instance.ex b/lib/bypass/instance.ex
@@ -1,4 +1,6 @@
 defmodule Bypass.Instance do
+  @moduledoc false
+
   use GenServer, restart: :transient
 
   import Bypass.Utils
@@ -264,7 +266,7 @@ defmodule Bypass.Instance do
     problem_route =
       expectations
       |> Enum.reject(fn {_route, expectations} -> expectations[:expected] == :none_or_more end)
-      |> Enum.find(fn {_route, expectations} -> length(expectations.results) == 0 end)
+      |> Enum.find(fn {_route, expectations} -> Enum.empty?(expectations.results) end)
 
     case problem_route do
       {route, _} ->
@@ -434,7 +436,7 @@ defmodule Bypass.Instance do
   defp so_reuseport() do
     case :os.type() do
       {:unix, :linux} -> [{:raw, 1, 15, <<1::32-native>>}]
-      {:unix, :darwin} -> [{:raw, 65535, 512, <<1::32-native>>}]
+      {:unix, :darwin} -> [{:raw, 65_535, 512, <<1::32-native>>}]
       _ -> []
     end
   end

diff --git a/lib/bypass/plug.ex b/lib/bypass/plug.ex
@@ -1,4 +1,6 @@
 defmodule Bypass.Plug do
+  @moduledoc false
+
   def init([pid]), do: pid
 
   def call(%{method: method, request_path: request_path} = conn, pid) do

diff --git a/test/bypass_test.exs b/test/bypass_test.exs
@@ -2,18 +2,7 @@ defmodule BypassTest do
   use ExUnit.Case
   doctest Bypass
 
-  if Code.ensure_loaded?(ExUnit.CaptureLog) do
-    defdelegate capture_log(fun), to: ExUnit.CaptureLog
-  else
-    # Shim capture_log for Elixir 1.0
-    defp capture_log(fun) do
-      ExUnit.CaptureIO.capture_io(:user, fn ->
-        fun.()
-        Logger.flush()
-      end)
-      |> String.strip()
-    end
-  end
+  defdelegate capture_log(fun), to: ExUnit.CaptureLog
 
   test "show ISSUE #51" do
     Enum.each(
@@ -37,7 +26,6 @@ defmodule BypassTest do
   defp specify_port(port, expect_fun) do
     bypass = Bypass.open(port: port)
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn conn ->
@@ -62,7 +50,6 @@ defmodule BypassTest do
   defp down_socket(expect_fun) do
     bypass = Bypass.open()
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn conn -> Plug.Conn.send_resp(conn, 200, "") end
@@ -101,13 +88,12 @@ defmodule BypassTest do
   defp not_called(expect_fun) do
     bypass = Bypass.open()
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn _conn -> assert false end
     ])
 
-    # Override Bypass' on_exit handler
+    # Override Bypass' on_exit handler.
     ExUnit.Callbacks.on_exit({Bypass, bypass.pid}, fn ->
       exit_result = Bypass.Instance.call(bypass.pid, :on_exit)
       assert {:error, :not_called, {:any, :any}} = exit_result
@@ -125,7 +111,6 @@ defmodule BypassTest do
   defp pass(expect_fun) do
     bypass = Bypass.open()
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn _conn ->
@@ -152,11 +137,10 @@ defmodule BypassTest do
   defp closing_in_flight(expect_fun) do
     bypass = Bypass.open()
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn _conn ->
-        # mark the request as arrived, since we're shutting it down now
+        # Mark the request as arrived, since we're shutting it down now.
         Bypass.pass(bypass)
         Bypass.down(bypass)
       end
@@ -179,7 +163,6 @@ defmodule BypassTest do
     ref = make_ref()
     bypass = Bypass.open()
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       fn conn ->
@@ -192,8 +175,8 @@ defmodule BypassTest do
 
     assert {:ok, 200, ""} = request(bypass.port)
 
-    # Here we make sure that Bypass.down waits until the plug process finishes its work
-    # before shutting down
+    # Here we make sure that Bypass.down waits until the plug process finishes
+    # its work before shutting down.
     refute_received ^ref
     Bypass.down(bypass)
     assert_received ^ref
@@ -236,8 +219,8 @@ defmodule BypassTest do
         end)
       end)
 
-    # Here we make sure that Bypass.down waits until the plug process finishes its work
-    # before shutting down
+    # Here we make sure that Bypass.down waits until the plug process finishes
+    # its work before shutting down.
     refute_received ^ref
     :timer.sleep(200)
     Bypass.down(bypass)
@@ -256,7 +239,7 @@ defmodule BypassTest do
       assert {:ok, 500, ""} = request(bypass.port)
     end)
 
-    # Override Bypass' on_exit handler
+    # Override Bypass' on_exit handler.
     ExUnit.Callbacks.on_exit({Bypass, bypass.pid}, fn ->
       exit_result = Bypass.Instance.call(bypass.pid, :on_exit)
       assert {:error, :unexpected_request, {:any, :any}} = exit_result
@@ -282,7 +265,7 @@ defmodule BypassTest do
       assert_receive :request_received
     end)
 
-    # Override Bypass' on_exit handler
+    # Override Bypass' on_exit handler.
     ExUnit.Callbacks.on_exit({Bypass, bypass.pid}, fn ->
       :ok == Bypass.Instance.call(bypass.pid, :on_exit)
     end)
@@ -303,7 +286,7 @@ defmodule BypassTest do
     assert_receive :request_received
     refute_receive :request_received
 
-    # Override Bypass' on_exit handler
+    # Override Bypass' on_exit handler.
     ExUnit.Callbacks.on_exit({Bypass, bypass.pid}, fn ->
       exit_result = Bypass.Instance.call(bypass.pid, :on_exit)
       assert {:error, :too_many_requests, {:any, :any}} = exit_result
@@ -347,7 +330,6 @@ defmodule BypassTest do
     method = "POST"
     path = "/this"
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       method,
@@ -382,7 +364,6 @@ defmodule BypassTest do
     pattern = "/this/:resource/get/:id"
     path = "/this/my_resource/get/1234"
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       method,
@@ -421,7 +402,6 @@ defmodule BypassTest do
     paths = ["/this", "/that"]
 
     Enum.each(paths, fn path ->
-      # one of Bypass.expect or Bypass.expect_once
       apply(Bypass, expect_fun, [
         bypass,
         method,
@@ -448,11 +428,11 @@ defmodule BypassTest do
   @doc ~S"""
   Open a new HTTP connection and perform the request. We don't want to use httpc, hackney or another
   "high-level" HTTP client, since they do connection pooling and we will sometimes get a connection
-  closed error and not a failed to connect error, when we test Bypass.down
+  closed error and not a failed to connect error, when we test Bypass.down.
   """
   def request(port, path \\ "/example_path", method \\ "POST") do
     with {:ok, conn} <- Mint.HTTP.connect(:http, "127.0.0.1", port),
-         {:ok, conn, ref} = Mint.HTTP.request(conn, method, path, [], "") do
+         {:ok, conn, ref} <- Mint.HTTP.request(conn, method, path, [], "") do
       receive_responses(conn, ref, 100, [])
     end
   end
@@ -490,11 +470,11 @@ defmodule BypassTest do
     end
   end
 
-  test "Bypass.expect/4 can be used to define a specific route and then redefined" do
+  test "Bypass.expect/4 can be used to define a specific route and then redefine it later" do
     :expect |> specific_route_redefined
   end
 
-  test "Bypass.expect_once/4 can be used to define a specific route and then redefined" do
+  test "Bypass.expect_once/4 can be used to define a specific route and then redefine it later" do
     :expect_once |> specific_route_redefined
   end
 
@@ -503,7 +483,6 @@ defmodule BypassTest do
     method = "POST"
     path = "/this"
 
-    # one of Bypass.expect or Bypass.expect_once
     apply(Bypass, expect_fun, [
       bypass,
       method,
@@ -519,7 +498,7 @@ defmodule BypassTest do
       assert {:ok, 200, ""} = request(bypass.port, path)
     end)
 
-    # redefining the expect
+    # Redefine the expect
     apply(Bypass, expect_fun, [
       bypass,
       method,