Refactor & test extended chrome args (#316)

lib/chromic_pdf.ex

@@ -268,7 +268,8 @@ defmodule ChromicPDF do
 
   ## Chrome options
 
-  By default, ChromicPDF will try to run a Chrome instance in the local environment. The following options allow to customize the generated command line.
+  By default, ChromicPDF will try to run a Chrome instance in the local environment. The
+  following options allow to customize the generated command line.
 
   ### Custom command line switches
 
@@ -278,22 +279,15 @@ defmodule ChromicPDF do
         [chrome_args: "--font-render-hinting=none"]
       end
 
-  In some cases, chromic_pdf's default args may conflict with the ones you would like to add.
-
-  E.g. `--disable-gpu`
-
-  The problem is that you can't simply override them like so:
-
-  E.g. in default args within chromic_pdf's code, `--disable-gpu` and then
-  passing `chrome_args: "--enable-gpu"` won't work.
-
-  In this case, use Keyword form of the `:chrome_args` option which
-  allows targeted removing of problematic default args so that they don't
-  disrupt your chrome configuration.
+  In some cases, ChromicPDF's default arguments (e.g. `--disable-gpu`) may conflict with the ones
+  you would like to add. In this case, use can supply a keyword list to the `:chrome_args` option
+  which allows targeted removing of default arguments.
 
       defp chromic_pdf_opts do
-        [chrome_args: [append: "--headless=new --angle=swiftshader",
-                       remove: ["--headless", "--disable-gpu"]]]
+        [chrome_args: [
+          append: "--headless=new --angle=swiftshader",
+          remove: ["--headless", "--disable-gpu"]
+        ]]
       end
 
   The `:chrome_executable` option allows to specify a custom Chrome/Chromium executable.

lib/chromic_pdf/pdf/chrome_runner.ex

@@ -57,7 +57,11 @@ defmodule ChromicPDF.ChromeRunner do
       )
   end
 
-  defp shell_command(extra_args, opts) do
+  # Public for unit tests.
+  @doc false
+  @spec shell_command(keyword()) :: binary()
+  @spec shell_command(binary() | [binary()], keyword()) :: binary()
+  def shell_command(extra_args \\ "", opts) do
     Enum.join([~s("#{executable(opts)}") | args(extra_args, opts)], " ")
   end
 
@@ -154,21 +158,23 @@ defmodule ChromicPDF.ChromeRunner do
   end
 
   defp append_if(list, _value, false), do: list
-  defp append_if(list, value, true), do: list ++ List.wrap(value)
+  defp append_if(list, value, true), do: append(list, value)
+
+  defp append(list, value), do: list ++ List.wrap(value)
 
   defp apply_chrome_args(list, nil), do: list
 
   defp apply_chrome_args(list, chrome_args) when is_binary(chrome_args) do
-    append_if(list, chrome_args, true)
+    append(list, chrome_args)
   end
 
   defp apply_chrome_args(list, extended) when is_list(extended) do
-    conflicting = List.wrap(Keyword.get(extended, :remove, []))
-    append = List.wrap(Keyword.get(extended, :append, []))
+    append = Keyword.get(extended, :append, [])
+    remove = Keyword.get(extended, :remove, []) |> List.wrap()
 
     list
-    |> Enum.reject(&Enum.member?(conflicting, &1))
-    |> append_if(append, true)
+    |> Enum.reject(&Enum.member?(remove, &1))
+    |> append(append)
   end
 
   defp no_sandbox?(opts), do: Keyword.get(opts, :no_sandbox, false)

test/unit/chromic_pdf/pdf/chrome_runner_test.exs

@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: Apache-2.0
+
+defmodule ChromicPDF.ChromeRunnerTest do
+  use ExUnit.Case, async: true
+  alias ChromicPDF.ChromeRunner
+
+  describe ":chrome_args option" do
+    test "additional arguments passed as binary" do
+      command = ChromeRunner.shell_command(chrome_args: "--some-extra-arg")
+      assert command =~ "--some-extra-arg"
+    end
+
+    test "removal of default arguments via extended :chrome_args" do
+      assert ChromeRunner.shell_command([]) =~ "--no-first-run"
+
+      command =
+        ChromeRunner.shell_command(
+          chrome_args: [remove: "--no-first-run", append: "--some-extra-arg"]
+        )
+
+      assert command =~ "--some-extra-arg"
+      refute command =~ "--no-first-run"
+    end
+  end
+end