Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/mix/tasks/mob.connect.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
32 changes: 29 additions & 3 deletions lib/mob_dev/connector.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
26 changes: 26 additions & 0 deletions test/mob_dev/connector_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
Loading