test: contract tests improving coverage (toward #79)

test/hyper/cfg/budget_test.exs

@@ -58,4 +58,22 @@ defmodule Hyper.Cfg.BudgetTest do
     assert {:ok, config} = Budget.load()
     assert config.cpu_max_cap == nil
   end
+
+  # Refusal contracts on bad budget values: each must fail load with a specific
+  # error naming the offending key, never silently coerce or crash. Table-driven:
+  # one assertion shape, rows differ only in the bad env and expected error.
+  @bad_budgets [
+    {[mem_max: 123], {:error, {:bad_value, :mem_max, 123}}},
+    {[mem_max: "notabytes"], {:error, {:bad_value, :mem_max, "notabytes"}}},
+    {[cpu_max_load: "high"], {:error, {:not_a_number, :cpu_max_load, "high"}}}
+  ]
+
+  for {env, expected} <- @bad_budgets do
+    @env env
+    @expected expected
+    test "load/0 rejects #{inspect(@env)} with #{inspect(@expected)}" do
+      Application.put_env(:hyper, Budget, @env)
+      assert @expected = Budget.load()
+    end
+  end
 end

test/hyper/cfg/grpc_test.exs

@@ -50,6 +50,12 @@ defmodule Hyper.Cfg.GrpcTest do
     assert Grpc.load().cred == cred
   end
 
+  test "with no cred configured, load returns nil (plaintext)" do
+    # The plaintext default: an absent cred must yield nil, never a defaulted
+    # credential that would silently enable TLS.
+    assert Grpc.load().cred == nil
+  end
+
   test "server_options omits :cred and :adapter_opts at their plaintext defaults" do
     opts = Grpc.server_options(%Grpc{})
 

test/hyper/cfg/network_test.exs

@@ -30,5 +30,16 @@ defmodule Hyper.Cfg.NetworkTest do
         Network.uplink()
       end
     end
+
+    test "raises ArgumentError when network.uplink is not a string" do
+      # A non-string uplink must raise ArgumentError, not be silently coerced
+      # or passed to the setuid helper which would then build a broken netns.
+      on_exit(fn -> Toml.reload() end)
+      Toml.put_cache(%{"network" => %{"uplink" => 123}})
+
+      assert_raise ArgumentError, ~r/network\.uplink/, fn ->
+        Network.uplink()
+      end
+    end
   end
 end

test/hyper/cluster/routing_test.exs

@@ -0,0 +1,65 @@
+defmodule Hyper.Cluster.RoutingTest do
+  @moduledoc """
+  Routing registry contracts beyond the resolution paths covered by `HyperTest`:
+
+    * `register_self/1` is idempotent-by-error: registering a key that is
+      already held returns `{:error, {:already_registered, _}}` rather than
+      crashing or silently replacing the owner.
+    * `all/0` reports every registered VM supervisor paired with the node
+      its process lives on (the cluster-wide inventory callers rely on).
+  """
+
+  use ExUnit.Case, async: false
+
+  alias Hyper.Cluster.Routing
+
+  setup do
+    # Under `mix test --no-start` the app tree (which provisions Routing) is
+    # absent; start a test-scoped registry so these contracts are testable
+    # without the full supervision tree. Routing is a named singleton, so if
+    # another test (e.g. HyperTest) already started it, reuse that one rather
+    # than racing on start_supervised!/1.
+    if Process.whereis(Routing) do
+      :ok
+    else
+      _ = Application.ensure_all_started(:horde)
+
+      case start_supervised(Routing) do
+        {:ok, _pid} -> :ok
+        # Lost the start race to a parallel test: reuse the existing registry.
+        {:error, {:already_started, _pid}} -> :ok
+      end
+    end
+  end
+
+  # Horde materialises a registration into the local replica asynchronously, so
+  # poll until the entry is visible before asserting against it.
+  defp await_materialised?(key, tries \\ 200)
+  defp await_materialised?(_key, 0), do: false
+
+  defp await_materialised?(key, tries) do
+    if Horde.Registry.lookup(Routing, key) != [],
+      do: true,
+      else: await_materialised?(key, tries - 1)
+  end
+
+  test "register_self/1 returns {:error, {:already_registered, _}} for a held key" do
+    key = {Hyper.Vm.Id.generate(), :supervisor}
+    assert :ok = Routing.register_self(key)
+    assert await_materialised?(key)
+
+    # The same process registering the same key again must report the conflict,
+    # not replace the owner or crash -- callers branch on this tuple.
+    assert {:error, {:already_registered, pid}} = Routing.register_self(key)
+    assert pid == self()
+  end
+
+  test "all/0 lists every registered supervisor as {vm_id, node}" do
+    vm_id = Hyper.Vm.Id.generate()
+    :ok = Routing.register_self({vm_id, :supervisor})
+    assert await_materialised?({vm_id, :supervisor})
+
+    assert {^vm_id, node} = List.keyfind(Routing.all(), vm_id, 0)
+    assert node == node()
+  end
+end

test/hyper/grpc/codec_test.exs

@@ -3,9 +3,14 @@ defmodule Hyper.Grpc.CodecTest do
 
   alias Hyper.Grpc.Codec
   alias Hyper.Grpc.V1.CreateVmRequest
+  alias Hyper.Grpc.V1.CreateVmResponse
+  alias Hyper.Grpc.V1.GetVmResponse
   alias Hyper.Grpc.V1.GetVmUsageResponse
   alias Hyper.Grpc.V1.ListVmsResponse
+  alias Hyper.Grpc.V1.LoadImageRequest
+  alias Hyper.Grpc.V1.LoadImageResponse
   alias Hyper.Grpc.V1.StopVmResponse
+  alias Hyper.Vm.Spec
 
   test "usage encodes as microseconds on the wire" do
     assert %GetVmUsageResponse{vm_id: "v1", cpu_usec: 1_500_000} =
@@ -74,4 +79,110 @@ defmodule Hyper.Grpc.CodecTest do
   test "a malformed page_token maps to INVALID_ARGUMENT" do
     assert %GRPC.RPCError{status: 3} = Codec.to_grpc({:error, :bad_page_token})
   end
+
+  # `from_grpc/1` -- the decode boundary. The contract: an inbound CreateVmRequest
+  # maps to a domain Spec with the right field placement, or is refused with a
+  # specific reason the server then classifies to a gRPC status.
+  describe "from_grpc/1 CreateVmRequest" do
+    test "a well-formed request decodes to a Spec preserving every field" do
+      assert {:ok,
+              %Spec{
+                img_id: "img",
+                type: :micro,
+                arch: :aarch64,
+                boot_args: "console=ttyS0"
+              }} =
+               Codec.from_grpc(%CreateVmRequest{
+                 img_id: "img",
+                 instance_type: :INSTANCE_TYPE_MICRO,
+                 arch: :ARCHITECTURE_AARCH64,
+                 boot_args: "console=ttyS0"
+               })
+    end
+
+    test "a missing img_id (nil or empty) is refused" do
+      assert {:error, :missing_img_id} =
+               Codec.from_grpc(%CreateVmRequest{img_id: nil, instance_type: :INSTANCE_TYPE_MICRO})
+
+      assert {:error, :missing_img_id} =
+               Codec.from_grpc(%CreateVmRequest{img_id: "", instance_type: :INSTANCE_TYPE_MICRO})
+    end
+  end
+
+  describe "from_grpc/1 LoadImageRequest" do
+    test "a request with no label decodes to an empty opts list" do
+      assert {:ok, {"alpine:3.19", []}} =
+               Codec.from_grpc(%LoadImageRequest{image_ref: "alpine:3.19", label: nil})
+
+      assert {:ok, {"alpine:3.19", []}} =
+               Codec.from_grpc(%LoadImageRequest{image_ref: "alpine:3.19", label: ""})
+    end
+
+    test "a request carrying a label forwards it as a labelled opt" do
+      assert {:ok, {"alpine:3.19", [label: "prod"]}} =
+               Codec.from_grpc(%LoadImageRequest{image_ref: "alpine:3.19", label: "prod"})
+    end
+
+    test "a missing image_ref (nil or empty) is refused" do
+      assert {:error, :missing_image_ref} = Codec.from_grpc(%LoadImageRequest{image_ref: nil})
+      assert {:error, :missing_image_ref} = Codec.from_grpc(%LoadImageRequest{image_ref: ""})
+    end
+  end
+
+  # `to_grpc/1` -- the encode boundary. The contract: each domain result maps to
+  # the response message carrying exactly the fields a client reads.
+  describe "to_grpc/1 response encoding" do
+    test "a created result carries the vm_id and node string" do
+      assert %CreateVmResponse{vm_id: "vabc", node: "hyper@host"} =
+               Codec.to_grpc({:created, "vabc", :hyper@host})
+    end
+
+    test "a located result carries the vm_id and node string" do
+      assert %GetVmResponse{vm_id: "vabc", node: "hyper@host"} =
+               Codec.to_grpc({:located, "vabc", :hyper@host})
+    end
+
+    test "a loaded result carries the image id" do
+      assert %LoadImageResponse{img_id: "img-1"} = Codec.to_grpc({:loaded, "img-1"})
+    end
+  end
+
+  # `rpc_error/1` -- the status-classification contract. The server promises each
+  # domain reason a specific gRPC status; a mutation that swaps two statuses would
+  # silently change what clients see. Table-driven: one assertion shape, rows
+  # differ only in the reason and expected status.
+  describe "to_grpc {:error, _} status classification" do
+    @status_cases [
+      {:missing_img_id, GRPC.Status.invalid_argument()},
+      {:missing_image_ref, GRPC.Status.invalid_argument()},
+      {:invalid_ref, GRPC.Status.invalid_argument()},
+      {:machine_unreachable, GRPC.Status.unavailable()},
+      {:no_capacity, GRPC.Status.resource_exhausted()},
+      {:exhausted, GRPC.Status.resource_exhausted()}
+    ]
+
+    test "each domain reason maps to its promised gRPC status" do
+      for {reason, status} <- @status_cases do
+        assert %GRPC.RPCError{status: ^status} = Codec.to_grpc({:error, reason})
+      end
+    end
+
+    test "a missing-tools error names the offending tools in the message" do
+      status = GRPC.Status.failed_precondition()
+
+      assert %GRPC.RPCError{
+               status: ^status,
+               message: "node is missing required image tools: umoci, skopeo"
+             } = Codec.to_grpc({:error, {:missing_tools, ["umoci", "skopeo"]}})
+    end
+
+    test "an unrecognised reason never crashes and falls back to INTERNAL" do
+      status = GRPC.Status.internal()
+
+      assert %GRPC.RPCError{status: ^status, message: msg} =
+               Codec.to_grpc({:error, :something_truly_novel})
+
+      assert msg =~ "something_truly_novel"
+    end
+  end
 end

test/redist/file_test.exs

@@ -26,7 +26,7 @@ defmodule Redist.FileTest do
     on_exit(fn -> File.rm_rf!(tmp) end)
     # Install into a not-yet-existing nested path so we also prove parent dirs
     # are created on success and never created on failure.
-    {:ok, server: server, dest: Path.join([tmp, "nested", "asset.bin"])}
+    {:ok, server: server, dest: Path.join([tmp, "nested", "asset.bin"]), tmp: tmp}
   end
 
   @payloads [
@@ -70,5 +70,21 @@ defmodule Redist.FileTest do
     refute File.exists?(dest)
   end
 
+  test "install/3 surfaces an unwritable destination as {:install_failed, _}", %{
+    server: server,
+    tmp: tmp
+  } do
+    # The download and checksum succeed; the failure is copying onto a path
+    # that is already a directory. The contract: a failed install classifies
+    # the POSIX reason rather than crashing or leaving a partial file.
+    bytes = "real-bytes"
+    url = HttpServer.put_file(server, "asset.bin", bytes)
+    dest = Path.join(tmp, "iamafile")
+    File.mkdir_p!(dest)
+
+    assert {:error, {:install_failed, reason}} = RedistFile.install(url, sha256(bytes), dest)
+    assert reason == :eisdir
+  end
+
   defp sha256(bytes), do: :crypto.hash(:sha256, bytes) |> Base.encode16(case: :lower)
 end

test/redist/targz_test.exs

@@ -91,6 +91,19 @@ defmodule Redist.TargzTest do
     assert File.ls(dest) in [{:error, :enoent}, {:ok, []}]
   end
 
+  test "install/3 surfaces a corrupt archive as {:extract_failed, _}", %{
+    server: server,
+    dest: dest
+  } do
+    # Checksum passes (it is the hash of the bytes we serve); extraction fails
+    # because the payload is not a tarball. The contract: a corrupt archive
+    # classifies as extract_failed, distinct from the unsafe-path refusal.
+    bytes = :crypto.strong_rand_bytes(64)
+    url = HttpServer.put_file(server, "bogus.tar.gz", bytes)
+
+    assert {:error, {:extract_failed, _reason}} = Targz.install(url, sha256(bytes), dest)
+  end
+
   defp sha256(bytes), do: :crypto.hash(:sha256, bytes) |> Base.encode16(case: :lower)
 
   defp targz(files) do