Return errors on missing commands rather than raise

This makes it possible for the caller to decide whether a missing
command should crash. This is different from the semantics of
System.cmd/3 and MuonTrap.cmd/3, but it's so convenient so long as we
don't run into a command that does useful work and returns failure exit
statuses. To help prevent that, the exit status on missing command is
256 which should be unusual.

This also adds tests and documentation for posterity.

lib/vintage_net/command.ex

@@ -1,20 +1,39 @@
 defmodule VintageNet.Command do
   @moduledoc false
 
+  @doc """
+  System.cmd wrapper to force paths
+
+  This helper function updates calls to `System.cmd/3` to force them to use the
+  PATHs configured on VintageNet for resolving where executables are.
+
+  It has one major difference in API - if a command does not exist, an error
+  exit status is returned with a message. `System.cmd/3` raises in this
+  situation. This means that the caller needs to check the exit status if they
+  care.
+  """
   @spec cmd(Path.t(), [binary()], keyword()) ::
           {Collectable.t(), exit_status :: non_neg_integer()}
   def cmd(command, args, opts \\ []) when is_binary(command) do
-    new_opts = force_path_env(opts)
-
-    System.cmd(find_executable!(command), args, new_opts)
+    with {:ok, command_path} <- find_executable(command) do
+      new_opts = force_path_env(opts)
+      System.cmd(command_path, args, new_opts)
+    end
   end
 
+  @doc """
+  Muontrap.cmd wrapper to force paths and options
+
+  This is similar to `cmd/3`, but it also adds common Muontrap options. It is
+  intended for long running commands or commands that may hang.
+  """
   @spec muon_cmd(Path.t(), [binary()], keyword()) ::
           {Collectable.t(), exit_status :: non_neg_integer()}
   def muon_cmd(command, args, opts \\ []) when is_binary(command) do
-    new_opts = opts |> force_path_env() |> add_muon_options()
-
-    MuonTrap.cmd(find_executable!(command), args, new_opts)
+    with {:ok, command_path} <- find_executable(command) do
+      new_opts = opts |> force_path_env() |> add_muon_options()
+      MuonTrap.cmd(command_path, args, new_opts)
+    end
   end
 
   @doc """
@@ -28,21 +47,17 @@ defmodule VintageNet.Command do
     )
   end
 
-  defp find_executable!(command) do
-    find_executable(command) ||
-      raise(RuntimeError, "Can't find '#{command}' in '#{path_env()}'")
-  end
-
+  # Note that error return value has to be compatible with System.cmd
   defp find_executable(command) do
     paths = String.split(path_env(), ":")
 
-    Enum.find_value(paths, fn path ->
-      full_path = Path.join(path, command)
+    path = paths |> Enum.map(&Path.join(&1, command)) |> Enum.find(&File.exists?/1)
 
-      if File.exists?(full_path) do
-        full_path
-      end
-    end)
+    if path do
+      {:ok, path}
+    else
+      {"'#{command}' not found in PATH='#{path_env()}'", 256}
+    end
   end
 
   defp force_path_env(opts) do

test/vintage_net/command_test.exs

@@ -0,0 +1,25 @@
+defmodule VintageNet.CommandTest do
+  use ExUnit.Case
+  doctest VintageNet.Command
+  alias VintageNet.Command
+
+  # See test/fixtures/root for what commands are available
+
+  test "cmd" do
+    assert {"hello\n", 0} = Command.cmd("echo", ["hello"])
+    assert {"", 1} = Command.cmd("false", [])
+    assert {_reason, 256} = Command.cmd("missing_command", [])
+  end
+
+  test "muon_cmd" do
+    assert {"hello\n", 0} = Command.muon_cmd("echo", ["hello"])
+    assert {"", 1} = Command.muon_cmd("false", [])
+    assert {_reason, 256} = Command.cmd("missing_command", [])
+  end
+
+  test "PATH overridden with VintageNet's version" do
+    expected_path = Application.get_env(:vintage_net, :path)
+    assert {expected_path <> "\n", 0} == Command.cmd("sh", ["-c", "echo $PATH"])
+    assert {expected_path <> "\n", 0} == Command.muon_cmd("sh", ["-c", "echo $PATH"])
+  end
+end