diff --git a/CHANGELOG.md b/CHANGELOG.md index 668b797..d479a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ### Fixed +- **`mix mob.connect --name` is now honored end-to-end** (MOB-69). The + option was parsed at the task layer, but `Connector.connect_all/1` + called `ensure_local_dist/1` which hardcoded `Node.start(:"mob_dev@127.0.0.1", ...)`. + By the time `start_iex/3` tried to honor `--name`, `Node.alive?/0` was + already true and the second `Node.start` was skipped — so the multi- + session workflow (one `--name mob_dev_N@127.0.0.1` per developer, + documented in the task's moduledoc) crashed with the wrong node name + registered in EPMD. Threaded `:name` through `connect_all/1` into a + new `ensure_local_dist/2`, backed by a public + `Connector.local_name_from_opts/1` helper so the option-plumbing is + unit-testable. + - **`mix mob.deploy --slim` is no longer a silent no-op** (MOB-73). `NativeBuild.build_all/1` stored `slim` in the process dict (`Process.put(:mob_slim, slim)`) but the actual gate at diff --git a/lib/mix/tasks/mob.connect.ex b/lib/mix/tasks/mob.connect.ex index 49a6106..c0fd39d 100644 --- a/lib/mix/tasks/mob.connect.ex +++ b/lib/mix/tasks/mob.connect.ex @@ -156,7 +156,12 @@ defmodule Mix.Tasks.Mob.Connect do end {connected, _failed} = - MobDev.Connector.connect_all(cookie: cookie, only: only, platforms: platforms) + MobDev.Connector.connect_all( + cookie: cookie, + only: only, + platforms: platforms, + name: local_name + ) if connected == [] do IO.puts("\n#{IO.ANSI.yellow()}No nodes connected. Nothing to do.#{IO.ANSI.reset()}\n") diff --git a/lib/mob_dev/connector.ex b/lib/mob_dev/connector.ex index e46f83f..babe855 100644 --- a/lib/mob_dev/connector.ex +++ b/lib/mob_dev/connector.ex @@ -30,6 +30,14 @@ defmodule MobDev.Connector do @spec connect_all(keyword()) :: {[Device.t()], [Device.t()]} def connect_all(opts \\ []) do cookie = Keyword.get(opts, :cookie, :mob_secret) + # The `mob.connect --name` option is documented for the multi-session + # workflow (one IEx per developer, distinct EPMD names). Before MOB-69 + # this arg was accepted at the Mix-task layer, but connect_all/1 + # unconditionally started distribution under `mob_dev@127.0.0.1` — so + # by the time `start_iex/3` tried to honor `--name`, `Node.alive?/0` + # was already true and the Node.start was skipped. Threading the name + # here fixes it end-to-end. + local_name = local_name_from_opts(opts) only = opts |> Keyword.get(:only, []) |> List.wrap() platforms = opts |> Keyword.get(:platforms, [:android, :ios]) |> List.wrap() @@ -64,7 +72,7 @@ defmodule MobDev.Connector do Enum.each(tunneled, &restart_app/1) # Start distribution on the Mac side - ensure_local_dist(cookie) + ensure_local_dist(local_name, cookie) # Activate accessibility on iOS simulators so ui_tree() returns elements. # SwiftUI lazily populates its a11y tree; this one-time activation persists @@ -217,12 +225,30 @@ defmodule MobDev.Connector do IO.puts(" done") end - defp ensure_local_dist(cookie) do + @doc """ + Reads the `:name` option from `connect_all/1`'s keyword list and returns + it as an atom. Falls back to `:"mob_dev@127.0.0.1"` when unset — the + historical default. + + Public so the option-plumbing is unit-testable without needing to actually + call `Node.start/2` (which would mutate BEAM-global distribution state + and require an `async: false` test module). See MOB-69. + """ + @spec local_name_from_opts(keyword()) :: node() + def local_name_from_opts(opts) when is_list(opts) do + case Keyword.get(opts, :name) do + nil -> :"mob_dev@127.0.0.1" + name when is_atom(name) -> name + name when is_binary(name) -> String.to_atom(name) + end + end + + defp ensure_local_dist(local_name, cookie) do unless Node.alive?() do # On Nix and some Linux setups, EPMD is not started automatically. # Try to start it before Node.start so distribution can register. start_epmd() - handle_dist_start(Node.start(:"mob_dev@127.0.0.1", :longnames), cookie) + handle_dist_start(Node.start(local_name, :longnames), cookie) end end diff --git a/test/mob_dev/connector_test.exs b/test/mob_dev/connector_test.exs index e96b289..7e3c6f4 100644 --- a/test/mob_dev/connector_test.exs +++ b/test/mob_dev/connector_test.exs @@ -6,6 +6,32 @@ defmodule MobDev.ConnectorTest do # ── filter_only/2 ──────────────────────────────────────────────────────────── + describe "local_name_from_opts/1 (MOB-69)" do + test "returns the historical default when :name is not set" do + # Preserves the pre-MOB-69 behaviour when a caller doesn't pass --name. + # If someone reverts this to hardcode `mob_dev@127.0.0.1` inside + # ensure_local_dist/2 (bypassing the opts), this test still passes + # because it exercises the helper alone. The next two assertions + # are the ones that flip on revert. + assert Connector.local_name_from_opts([]) == :"mob_dev@127.0.0.1" + end + + test "honors :name when passed as an atom (Mix task normalizes to atom first)" do + # Revert-verify: change `local_name_from_opts(opts)` back to + # `:"mob_dev@127.0.0.1"` and this fails. + assert Connector.local_name_from_opts(name: :"mob_dev_2@127.0.0.1") == + :"mob_dev_2@127.0.0.1" + end + + test "honors :name when passed as a string (defensive against callers that skipped normalization)" do + # Belt-and-suspenders for the multi-session workflow — the Mix task + # normalizes to atom, but Verify.load_verified-style callers could + # pass strings and this helper handles both. + assert Connector.local_name_from_opts(name: "mob_dev_3@127.0.0.1") == + :"mob_dev_3@127.0.0.1" + end + end + describe "filter_only/2" do setup do devices = [