diff --git a/CHANGELOG.md b/CHANGELOG.md index d0211ee..8d246d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,35 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ### Changed +- **`mix mob.deploy` no longer exits 0 after shipping nothing.** Four ways it + could, all fixed (MOB-150): + - `--android --native` with no `sdk.dir` in `android/local.properties` + printed a skip warning, built nothing, and succeeded. The old rule was + `ok_count == length(results)`, which is `0 == 0` for a run that produced no + results at all. + - `--ios` / `--android` where every device of that platform was skipped. + A skip stays non-fatal when it is incidental — a phone that happens to be + attached — and is fatal when the run named that platform. The rule is per + **platform**, not per device: one simulator deploying while a stale one is + skipped is a success, not a failure. + - `--device X` that reached X and deployed nothing to it, and `--device NOPE` + matching no device at all. + - `--ios` on Linux, which resolves to no platforms and enumerated no devices. + + A `--native` run that built the artifact and found no device to push it to + still exits 0 — "build the APK now, attach the phone after" is unchanged. + +### Added + +- **`mix mob.deploy --json`** — a machine-readable result on stdout listing the + deployed, failed and skipped devices with their per-device reasons, and an + `outcome` that mirrors the exit status. Progress output is redirected to + stderr for the run, so `mix mob.deploy --json | jq` receives exactly one + document. Emitted on the native-build failure path too, which is when a + caller most needs it. + +### Changed + - **`mix mob.deploy` now exits non-zero when a device fails.** A run that printed `Failed on 1 device(s)` previously still returned status 0, so CI and wrapper scripts read a failed deploy as success. Every device is still diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 3e76764..31526ef 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -33,6 +33,8 @@ defmodule Mix.Tasks.Mob.Deploy do for scripted scenarios where you need a specific naming scheme. * `--schedulers ` — set BEAM scheduler count (saved to mob.exs) * `--beam-flags ""` — arbitrary BEAM flags string (saved to mob.exs) + * `--json` — machine-readable result on stdout; progress goes to + stderr, so `mix mob.deploy --json | jq` gets one document * `--slim` — strip OTP source/debug for size measurement on a real device. OFF by default for dev iteration (the strip pass adds ~5-10s per build); use this @@ -123,12 +125,34 @@ defmodule Mix.Tasks.Mob.Deploy do task exits non-zero if **any** device landed in the `Failed on N device(s)` bucket — including a partial success where other devices deployed fine. - Devices under `Skipped on N device(s)` (app not installed for that - platform) do not fail the run. + Devices under `Skipped on N device(s)` (app not installed for that platform) + do not fail the run — *unless you named that platform*. A skip means "this + device is not a target for this app", which is ordinary when it is a phone + that happens to be attached, and a failure when the run asked for it: + + * `mix mob.deploy` with an unrelated phone attached — exit 0. + * `mix mob.deploy --ios` where every iOS device was skipped — exit 1. + * `mix mob.deploy --ios` where one simulator deployed and a stale one was + skipped — exit 0. A partial success is a success; the rule is per + platform, not per device. + * `mix mob.deploy --device X` that reached X and deployed nothing — exit 1. + * `mix mob.deploy --device NOPE` matching no device — exit 1. + * `mix mob.deploy --android --native` that built the APK with no device + attached — exit 0. The artifact is what was asked for. + + `--native` fails the run when a platform you named built nothing at all, which + is what a missing `sdk.dir` in `android/local.properties` produces. """ + alias MobDev.Device + @switches [ native: :boolean, + # Machine-readable result on stdout, for a caller that needs to know which + # targets got what without parsing coloured prose. Progress is redirected + # to stderr for the run (see the group-leader swap in `run/1`) so stdout + # carries exactly one document. + json: :boolean, restart: :boolean, android: :boolean, ios: :boolean, @@ -157,6 +181,22 @@ defmodule Mix.Tasks.Mob.Deploy do def run(args) do {opts, _, _} = OptionParser.parse(args, switches: @switches) + # Under --json, stdout must carry ONE document and nothing else. Every + # progress line in this task, the deployer and the native build is a plain + # `IO.puts/1`, which resolves `:stdio` through the group leader — so + # repointing it sends all of them, and the `into: IO.stream()` subprocess + # output too, to stderr. The JSON is then written to the real stdout + # explicitly. Rewriting several hundred call sites to take a device would + # be the alternative. + if opts[:json] do + # Capture the real stdout FIRST. `:standard_io` also resolves through the + # group leader, so swapping it without saving this sends the document to + # stderr along with the prose — the flag then emits nothing a pipe can + # read, which is the whole failure it exists to prevent. + Process.put(:mob_deploy_stdout, Process.group_leader()) + Process.group_leader(self(), Process.whereis(:standard_error)) + end + restart = Keyword.get(opts, :restart, true) native = Keyword.get(opts, :native, false) device_id = opts[:device] @@ -220,7 +260,8 @@ defmodule Mix.Tasks.Mob.Deploy do MobDev.NativeBuild.build_all( platforms: platforms, device: effective_device_id, - slim: slim + slim: slim, + requested: requested_platforms(opts) ) end @@ -233,6 +274,7 @@ defmodule Mix.Tasks.Mob.Deploy do "#{IO.ANSI.yellow()}Run `mix mob.doctor` to check your environment, or `mix mob.deploy` (without --native) once the issue is fixed.#{IO.ANSI.reset()}" ) + emit_json(opts, [], [], [], "Native build failed") Mix.raise("Native build failed") end @@ -254,7 +296,19 @@ defmodule Mix.Tasks.Mob.Deploy do # The full summary is printed first, then the status code is set — the # fan-out across devices is unchanged, only the exit code is. - case failure_message(deployed, failed, skipped) do + message = + missing_device_message(device_id, deployed, failed, skipped) || + failure_message( + deployed, + failed, + skipped, + requested_platforms(opts), + native and native_ok == true + ) + + emit_json(opts, deployed, failed, skipped, message) + + case message do nil -> :ok message -> Mix.raise(message) end @@ -278,11 +332,138 @@ defmodule Mix.Tasks.Mob.Deploy do out if the status code says everything is fine. """ @spec failure_message([Device.t()], [Device.t()], [Device.t()]) :: String.t() | nil - def failure_message(_deployed, [], _skipped), do: nil + def failure_message(deployed, failed, skipped), + do: failure_message(deployed, failed, skipped, []) + + @doc """ + As `failure_message/3`, but knowing which platforms were explicitly asked for. + + A skipped device is normally not a failure — it means "this device is not a + target for this app", the expected outcome of an Android phone being attached + during a default run. It IS a failure when the run named that platform: a + `mix mob.deploy --android` that skips every Android device asked for + something and got nothing, and must not report success. + + Pass `[]` for requested and every skip is incidental, which is the + `failure_message/3` behaviour. + """ + @spec failure_message([Device.t()], [Device.t()], [Device.t()], [atom()]) :: String.t() | nil + def failure_message(_deployed, failed, _skipped, _requested) when failed != [], + do: "Deploy failed on #{length(failed)} device(s) — see errors above" - def failure_message(_deployed, failed, _skipped), + def failure_message(deployed, failed, skipped, requested), + do: failure_message(deployed, failed, skipped, requested, false) + + @doc """ + As `failure_message/4`, but knowing whether a native build succeeded. + + A `--native` run that built the artifact and found no device to push it to + did its main job. Failing it would break "build the APK now, attach the phone + after", which used to exit 0. + """ + @spec failure_message([Device.t()], [Device.t()], [Device.t()], [atom()], boolean()) :: + String.t() | nil + def failure_message(_deployed, failed, _skipped, _requested, _native) when failed != [], do: "Deploy failed on #{length(failed)} device(s) — see errors above" + def failure_message(deployed, _failed, skipped, requested, native_built?) do + # Per PLATFORM, not per device. `deploy_all/1` only enumerates devices for + # platforms in the resolved list, and the resolved list is a subset of the + # requested one, so "is this skip's platform requested?" is always true and + # the rule would reduce to "any skip at all is fatal once you name a + # platform". That fails a perfectly good run: two booted simulators with + # the app on only the one you are working on, or a spare phone plugged in. + # A platform is unserved only when it skipped AND nothing of it landed. + unserved = + Enum.filter(requested, fn platform -> + Enum.any?(skipped, &(&1.platform == platform)) and + not Enum.any?(deployed, &(&1.platform == platform)) + end) + + cond do + unserved != [] -> + "Deploy reached no #{names(unserved)} device — every one was skipped, " <> + "and you asked for it" + + # Asked for a platform and reached nothing at all. Covers `--ios` on + # Linux, where platform resolution yields an empty list, so no device is + # even enumerated and every bucket is empty. + # + # Not when a native build succeeded: that run produced the artifact it + # was asked for and merely had nowhere to push it. + requested != [] and deployed == [] and skipped == [] and not native_built? -> + "Deploy reached no device for #{names(requested)} — none was connected" + + true -> + nil + end + end + + defp names(platforms), do: platforms |> Enum.map(&"--#{&1}") |> Enum.join(", ") + + # Written to the REAL stdout, not the group leader — which under --json now + # points at stderr so the progress prose gets out of the document's way. + defp emit_json(opts, deployed, failed, skipped, message) do + if opts[:json] do + json = Jason.encode!(json_result(deployed, failed, skipped, message), pretty: true) + IO.puts(Process.get(:mob_deploy_stdout, :standard_io), json) + end + end + + @doc """ + The machine-readable result of a finished deploy. + + Exists because an agent driving `mix mob.deploy` otherwise has to infer the + outcome from coloured prose, and the exit code alone does not say *which* + target missed out. `outcome` mirrors the exit status: `"ok"` when the task + returns 0, `"error"` when it raises. + """ + @spec json_result([Device.t()], [Device.t()], [Device.t()], String.t() | nil) :: map() + def json_result(deployed, failed, skipped, message) do + %{ + "outcome" => if(message, do: "error", else: "ok"), + "message" => message, + "deployed" => Enum.map(deployed, &json_device/1), + "failed" => Enum.map(failed, &json_device/1), + "skipped" => Enum.map(skipped, &json_device/1) + } + end + + defp json_device(%MobDev.Device{} = device) do + %{ + "name" => device.name, + "serial" => device.serial, + "platform" => to_string(device.platform), + "reason" => device.error + } + end + + @doc """ + The message for a run that named a device and did not find it, or `nil`. + + `mix mob.deploy --device NOPE` printed "No devices found." and exited 0. The + device filter matches nothing, every bucket comes back empty, and a run that + shipped to a device you named by id is indistinguishable from one that + shipped nowhere. + + Only fires when a device was named: with no `--device`, an empty run is the + ordinary "nothing is plugged in" case and stays non-fatal. + """ + @spec missing_device_message(String.t() | nil, [Device.t()], [Device.t()], [Device.t()]) :: + String.t() | nil + def missing_device_message(nil, _deployed, _failed, _skipped), do: nil + + def missing_device_message(device_id, [], [], []), + do: "No device matched --device #{device_id} — nothing was deployed" + + # Found, but nothing landed on it. Naming a device by id is at least as + # explicit as naming a platform, so a run that shipped nowhere must say so. + # `failed` is left to `failure_message/5`, which reports the actual error. + def missing_device_message(device_id, [], [], skipped) when skipped != [], + do: "--device #{device_id} was skipped — nothing was deployed to it" + + def missing_device_message(_device_id, _deployed, _failed, _skipped), do: nil + @doc """ Build the per-deploy summary lines from the three device buckets. @@ -353,6 +534,20 @@ defmodule Mix.Tasks.Mob.Deploy do acc ++ [header | rows] end + @doc """ + The platforms the user explicitly asked for, from the raw flags. + + Deliberately NOT `resolve_platforms/1`, which collapses "no flag given" into + every platform with a scaffold. That distinction is the whole point: a device + skipped during a default run is incidental (a phone that happens to be + attached), while one skipped during `--android` is a request that went + unserved. Returns `[]` when no platform flag was given. + """ + @spec requested_platforms(keyword()) :: [:android | :ios] + def requested_platforms(opts) do + Enum.filter([:android, :ios], &(opts[&1] == true)) + end + defp resolve_platforms(opts) do android = opts[:android] ios = opts[:ios] diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 2d95a39..6e8941b 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -79,6 +79,12 @@ defmodule MobDev.NativeBuild do results not File.dir?("android") -> + if :android in Keyword.get(opts, :requested, []) do + IO.puts( + " #{IO.ANSI.yellow()}⚠ Skipping Android build — no android/ directory in this project#{IO.ANSI.reset()}" + ) + end + results not android_toolchain_available?() -> @@ -115,6 +121,12 @@ defmodule MobDev.NativeBuild do [build_ios(cfg, device_id) | results] true -> + if :ios in Keyword.get(opts, :requested, []) do + IO.puts( + " #{IO.ANSI.yellow()}⚠ Skipping iOS build — no ios/build.zig in this project#{IO.ANSI.reset()}" + ) + end + results end else @@ -137,10 +149,68 @@ defmodule MobDev.NativeBuild do ) end) - ok_count = Enum.count(results, &match?({:ok, _}, &1)) - ok_count == length(results) + # Intersect with the NARROWED platform list. `narrow_platforms_for_device/2` + # above drops Android when the target is an iOS UDID — including one this + # task auto-detected rather than one the user named — and counting that as + # an unserved `--android` request fails a build that did exactly what was + # asked of it. + requested = Enum.filter(Keyword.get(opts, :requested, []), &(&1 in platforms)) + + case build_outcome(results, requested) do + :ok -> + true + + {:error, message} -> + IO.puts(" #{IO.ANSI.red()}✗ #{message}#{IO.ANSI.reset()}") + false + end end + @doc """ + Whether a native build run succeeded, given what it produced and what the + user explicitly asked for. + + `results` entries are `{:ok, label}` / `{:error, label, reason}` where label + is the display name ("Android", "iOS", "iOS (device)"). + + `requested` is the platforms named by an explicit `--android` / `--ios` + flag — NOT the resolved platform list, which collapses "no flag given" into + every platform and would make an ordinary skip fatal. + + The rule this exists for: `ok_count == length(results)` is `0 == 0` for a run + that built nothing, so `mix mob.deploy --android --native` with no `sdk.dir` + printed a warning, built nothing, and reported success. A skip is fine when + nobody asked for that platform; it is a failure when they did. + """ + @spec build_outcome([{:ok, String.t()} | {:error, String.t(), term()}], [atom()]) :: + :ok | {:error, String.t()} + def build_outcome(results, requested) do + built = results |> Enum.map(&result_platform/1) |> Enum.reject(&is_nil/1) |> Enum.uniq() + failed = Enum.filter(results, &match?({:error, _, _}, &1)) + missing = requested -- built + + cond do + failed != [] -> + {:error, "#{length(failed)} native build(s) failed — see errors above"} + + missing != [] -> + names = missing |> Enum.map(&"--#{&1}") |> Enum.join(", ") + + {:error, + "nothing was built for #{names}, which you asked for — see the skip reason above"} + + true -> + :ok + end + end + + defp result_platform({:ok, label}), do: label_platform(label) + defp result_platform({:error, label, _reason}), do: label_platform(label) + + defp label_platform("Android" <> _), do: :android + defp label_platform("iOS" <> _), do: :ios + defp label_platform(_), do: nil + # ── Android ────────────────────────────────────────────────────────────────── defp build_android(cfg, device_id) do diff --git a/test/mix/tasks/mob_deploy_beam_flags_test.exs b/test/mix/tasks/mob_deploy_beam_flags_test.exs index 32c7e19..f619a2c 100644 --- a/test/mix/tasks/mob_deploy_beam_flags_test.exs +++ b/test/mix/tasks/mob_deploy_beam_flags_test.exs @@ -118,6 +118,11 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do defp device(name, error \\ nil), do: %MobDev.Device{name: name, serial: name, platform: :android, error: error} + # The requested-vs-incidental rule keys on `:platform`, so the fixtures + # have to be able to be an iPhone. + defp ios_device(name, error \\ nil), + do: %MobDev.Device{name: name, serial: name, platform: :ios, error: error} + defp strip_ansi(line), do: String.replace(line, ~r/\e\[[0-9;]*m/, "") test "all three buckets empty → 'No devices found' hint" do @@ -268,4 +273,194 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do assert Deploy.failure_message([], failed, []) =~ "Deploy failed on 3 device(s)" end end + + # ── MOB-150: a run that shipped nothing must not report success ────────────── + # + # PR #44 made a *failed* device fatal and deliberately left a *skipped* one + # alone, on the grounds that a skip means "this device is not a target". + # That is right for an incidental skip and wrong for a requested one, and it + # left three ways to exit 0 having deployed nothing. + + describe "requested_platforms/1" do + test "reads the raw flags, not the resolved platform list" do + assert Deploy.requested_platforms(android: true) == [:android] + assert Deploy.requested_platforms(ios: true) == [:ios] + assert Deploy.requested_platforms(android: true, ios: true) == [:android, :ios] + end + + test "a negated flag is not a request" do + # `--no-android` must not register as "you asked for Android", which + # would make the run fatal for a platform the user explicitly turned off. + # + # A review flagged the original `&opts[&1]` as a truthiness bug here. It + # was not: `false` is falsy, so that already excluded a negated flag, and + # mutating it back leaves this test green. The explicit `== true` is + # clarity about intent, not a fix — worth keeping, worth not + # misdescribing. + assert Deploy.requested_platforms(android: false) == [] + assert Deploy.requested_platforms(android: false, ios: true) == [:ios] + end + + test "no flag means nothing was explicitly requested" do + # The load-bearing case. `resolve_platforms/1` turns "no flag" into every + # platform with a scaffold; using that here would make an ordinary + # incidental skip fatal on every default run. + assert Deploy.requested_platforms([]) == [] + assert Deploy.requested_platforms(restart: true, slim: false) == [] + end + end + + describe "failure_message/4 — a skip you asked for" do + test "an incidental skip is still not a failure" do + skip = ios_device("iPhone", "app not installed") + assert Deploy.failure_message([device("emulator")], [], [skip], []) == nil + end + + test "every device of a platform you named being skipped is a failure" do + skip = ios_device("iPhone", "app not installed") + + message = Deploy.failure_message([], [], [skip], [:ios]) + + assert message =~ "reached no --ios device" + end + + test "a partial success is NOT a failure — one sim skipped, another deployed" do + # The false positive that made the first version of this rule unshippable. + # Two booted simulators with the app on only the one you are working on, + # or a spare phone plugged in, is an ordinary setup — and `--ios` is the + # only way to scope a run on macOS, where the default is both platforms. + # + # The earlier rule filtered SKIPPED devices by requested platform, which + # `deploy_all/1` makes a tautology: it only enumerates devices for + # platforms in the resolved list, and that list is a subset of the + # requested one. So the filter was always true and the rule reduced to + # "any skip at all is fatal once you name a platform". + deployed = ios_device("iPhone 17 Pro") + skipped = ios_device("stale sim", "app not installed") + + assert Deploy.failure_message([deployed], [], [skipped], [:ios]) == nil + end + + test "one platform fully skipped still fails when the other succeeded" do + # `--android --ios` where every Android device skipped: iOS working must + # not mask the half that was asked for and got nothing. + message = + Deploy.failure_message( + [ios_device("iPhone")], + [], + [device("emulator-5554", "not installed")], + [:android, :ios] + ) + + assert message =~ "--android" + refute message =~ "--ios" + end + + test "a real failure still outranks a skip" do + failed = device("buggy", "push timed out") + skip = ios_device("iPhone", "app not installed") + + assert Deploy.failure_message([], [failed], [skip], [:ios]) =~ "failed on 1 device(s)" + end + + test "asking for a platform and reaching nothing at all is a failure" do + # `--ios` on Linux resolves to no platforms, so no device is even + # enumerated: every bucket is empty and the run previously exited 0. + assert Deploy.failure_message([], [], [], [:ios]) =~ "reached no device for --ios" + end + + test "a successful native build with no device attached is not a failure" do + # "Build the APK now, attach the phone after" exited 0 before this ticket + # and must keep doing so — the run produced the artifact it was asked + # for and merely had nowhere to push it. + assert Deploy.failure_message([], [], [], [:android], true) == nil + + # Without a native build the run's only purpose was to push, and it + # pushed nowhere. + assert Deploy.failure_message([], [], [], [:android], false) =~ "none was connected" + end + + test "reaching nothing without asking for anything is fine" do + assert Deploy.failure_message([], [], [], []) == nil + end + + test "failure_message/3 keeps its old meaning" do + skip = ios_device("iPhone", "app not installed") + assert Deploy.failure_message([], [], [skip]) == nil + end + end + + describe "missing_device_message/4" do + test "a named device that was not found is a failure" do + message = Deploy.missing_device_message("NOPE", [], [], []) + assert message =~ "No device matched --device NOPE" + end + + test "no --device means an empty run is just nothing plugged in" do + assert Deploy.missing_device_message(nil, [], [], []) == nil + end + + test "a named device that WAS found and deployed is not a failure" do + assert Deploy.missing_device_message("serial", [device("serial")], [], []) == nil + end + + test "a named device that failed is left to failure_message" do + # It reports the actual error; double-reporting would mask the reason. + assert Deploy.missing_device_message("serial", [], [device("serial", "boom")], []) == nil + end + + test "a named device that was found but skipped is a failure" do + # Naming a device by id is at least as explicit as naming a platform, so + # a run that shipped nothing to it must say so. This exited 0. + skipped = [device("emulator-5554", "app not installed")] + + assert Deploy.missing_device_message("emulator-5554", [], [], skipped) =~ + "was skipped — nothing was deployed" + end + end + + describe "json_result/4" do + # An agent driving mob.deploy otherwise infers the outcome from coloured + # prose, and the exit code alone does not say WHICH target missed out. + test "a clean run reports ok and lists what was deployed" do + result = Deploy.json_result([device("emulator-5554")], [], [], nil) + + assert result["outcome"] == "ok" + assert result["message"] == nil + assert [%{"serial" => "emulator-5554", "platform" => "android"}] = result["deployed"] + end + + test "outcome mirrors the exit status, not the buckets" do + # A skip is fatal or not depending on whether it was requested, so the + # buckets alone cannot tell a caller what the exit code will be. The + # message is the decision, and outcome must follow it. + skip = ios_device("iPhone", "app not installed") + + assert Deploy.json_result([], [], [skip], nil)["outcome"] == "ok" + assert Deploy.json_result([], [], [skip], "asked for --ios")["outcome"] == "error" + end + + test "each bucket carries the per-device reason" do + failed = device("buggy", "push timed out") + result = Deploy.json_result([], [failed], [], "Deploy failed on 1 device(s)") + + assert [%{"name" => "buggy", "reason" => "push timed out"}] = result["failed"] + assert result["message"] == "Deploy failed on 1 device(s)" + end + + test "each device reports its own platform" do + # Only one assertion checked `platform`, and it expected "android", so + # hardcoding that string passed the suite. + result = Deploy.json_result([ios_device("iPhone")], [], [device("emu")], nil) + + assert [%{"platform" => "ios"}] = result["deployed"] + assert [%{"platform" => "android"}] = result["skipped"] + end + + test "it survives a round trip through Jason" do + result = Deploy.json_result([device("a")], [device("b", "boom")], [ios_device("c")], nil) + + assert result |> Jason.encode!() |> Jason.decode!() == result + end + end end diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index 1dbdae4..57e94ea 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -2249,4 +2249,55 @@ defmodule MobDev.NativeBuildTest do assert NativeBuild.ios_bundle_id([]) == nil end end + + describe "build_outcome/2 — MOB-150" do + # The observed bug: `mix mob.deploy --android --native` with no `sdk.dir` + # in android/local.properties printed a skip warning, built nothing, and + # exited 0. The old rule was `ok_count == length(results)`, which is + # `0 == 0` for a run that produced no results at all. + + test "everything asked for was built" do + assert NativeBuild.build_outcome([{:ok, "Android"}], [:android]) == :ok + end + + test "a build that failed is a failure" do + results = [{:error, "Android", "gradle exited 1"}] + assert {:error, message} = NativeBuild.build_outcome(results, [:android]) + assert message =~ "1 native build(s) failed" + end + + test "building nothing when nothing was asked for is fine" do + # A default `mix mob.deploy --native` on a machine with only the iOS + # toolchain: Android is skipped and nobody asked for it. + assert NativeBuild.build_outcome([], []) == :ok + assert NativeBuild.build_outcome([{:ok, "iOS"}], []) == :ok + end + + test "building nothing for a platform you named is a failure" do + assert {:error, message} = NativeBuild.build_outcome([], [:android]) + assert message =~ "nothing was built for --android" + end + + test "building only the other platform you named is a failure" do + # `--android --ios` where the Android toolchain is missing: iOS + # succeeding must not mask the half that was skipped. + assert {:error, message} = + NativeBuild.build_outcome([{:ok, "iOS"}], [:android, :ios]) + + assert message =~ "--android" + refute message =~ "--ios" + end + + test "the device iOS label counts as iOS" do + # The physical-device chain reports "iOS (device)", not "iOS", and a + # prefix match is what keeps that from reading as an unserved request. + assert NativeBuild.build_outcome([{:ok, "iOS (device)"}], [:ios]) == :ok + end + + test "a failure outranks an unserved request" do + results = [{:error, "iOS", "zig exited 2"}] + assert {:error, message} = NativeBuild.build_outcome(results, [:android, :ios]) + assert message =~ "failed" + end + end end diff --git a/test/mob_dev/ios_bundle_id_wiring_test.exs b/test/mob_dev/wiring_test.exs similarity index 56% rename from test/mob_dev/ios_bundle_id_wiring_test.exs rename to test/mob_dev/wiring_test.exs index 6e5a740..d2690e7 100644 --- a/test/mob_dev/ios_bundle_id_wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -4,8 +4,18 @@ # what the check is designed to catch. It is the right shape here: the paths # guarded need a keychain, a provisioning profile or a physical device to run, # and every one of these fixes was silently revertible before they existed. -defmodule MobDev.IosBundleIdWiringTest do +defmodule MobDev.WiringTest do @moduledoc """ + Assertions that a decision is actually WIRED to the code path it governs. + + Every finding that put a test in here had the same shape: a well-covered + pure function, and a call site nothing checked — so deleting the call left + the suite green and fully restored the bug. That has now happened four + times (the two bundle-id resolvers, the `Mix.raise` on a failed deploy, and + the `requested:` flag on the native build). + + ## iOS bundle ids + Every iOS path must resolve its bundle id through the `:ios_bundle_id || :bundle_id` rule. @@ -24,6 +34,14 @@ defmodule MobDev.IosBundleIdWiringTest do @release File.read!(Path.expand("../../lib/mob_dev/release.ex", __DIR__)) @deployer File.read!(Path.expand("../../lib/mob_dev/deployer.ex", __DIR__)) @deploy_task File.read!(Path.expand("../../lib/mix/tasks/mob.deploy.ex", __DIR__)) + @native_build_src File.read!(Path.expand("../../lib/mob_dev/native_build.ex", __DIR__)) + + defp index_of(hay, needle) do + case :binary.match(hay, needle) do + {i, _} -> i + :nomatch -> flunk("expected to find #{inspect(needle)}") + end + end defp region(source, from, to) do unless String.contains?(source, from), do: flunk("marker not found: #{inspect(from)}") @@ -95,14 +113,70 @@ defmodule MobDev.IosBundleIdWiringTest do # decides it still perfectly covered. body = region(@deploy_task, "Enum.each(format_summary(", "\n end") - assert body =~ "case failure_message(deployed, failed, skipped) do" + assert body =~ "failure_message(" + assert body =~ "missing_device_message(device_id, deployed, failed, skipped)" assert body =~ "message -> Mix.raise(message)" # Order matters: raising before the summary loses the per-device detail - # for every device in the run, which is what an operator reads. - summary_at = 0 - raise_at = :binary.match(body, "Mix.raise(message)") |> elem(0) - assert summary_at < raise_at + # for every device in the run, which is what an operator reads. Measured + # against the whole file — anchoring the region ON the summary made this + # `assert 0 < raise_at`, which cannot fail. + assert index_of(@deploy_task, "Enum.each(format_summary(") < + index_of(@deploy_task, "Mix.raise(message)") + end + end + + describe "the native build's honest-exit wiring (MOB-150)" do + # `build_outcome/2` is well covered as a pure function, but nothing in the + # suite calls `build_all/1` — so deleting either half of the wiring left + # 2244 tests green and fully restored the bug: `mix mob.deploy --android + # --native` with no sdk.dir builds nothing and exits 0. Same shape as the + # `Mix.raise` gap above: the decision was covered, the call was not. + test "the task tells the build which platforms were asked for" do + body = region(@deploy_task, "MobDev.NativeBuild.build_all(", "\n )") + + assert body =~ "requested: requested_platforms(opts)" + end + + test "the build reads that back and runs it through build_outcome/2" do + body = region(@native_build_src, "requested =", "\n end") + + assert body =~ "Keyword.get(opts, :requested, [])" + assert body =~ "case build_outcome(results, requested) do" + end + + test "an unserved request is intersected with the narrowed platform list" do + # An auto-detected iPhone narrows Android out of the build. Counting that + # as an unserved `--android` fails a run that did what was asked. + body = region(@native_build_src, "requested =", "case build_outcome") + + assert body =~ "Enum.filter(Keyword.get(opts, :requested, []), &(&1 in platforms))" + end + end + + describe "--json is declared and pipeable" do + # Deleting `json: :boolean` from @switches makes OptionParser drop the flag + # into `invalid`, `opts[:json]` becomes nil, and no document is emitted — + # with every json_result/4 test still green, because none of them went + # through the switch. + test "the switch exists" do + assert @deploy_task =~ "json: :boolean" + end + + test "progress is redirected so stdout carries one document" do + # The comment used to claim progress went to stderr. It did not: every + # line in this task, the deployer and the native build is a plain + # IO.puts/1 to :stdio, so `--json | jq` got ANSI prose and died. + assert @deploy_task =~ "Process.group_leader(self(), Process.whereis(:standard_error))" + assert @deploy_task =~ "Process.put(:mob_deploy_stdout, Process.group_leader())" + assert @deploy_task =~ "IO.puts(Process.get(:mob_deploy_stdout, :standard_io), json)" + end + + test "a native-build failure still emits a document" do + # The one path where a caller most needs the result was the one that + # produced none, because the emit sat after the early raise. + assert @deploy_task =~ + ~r/emit_json\(opts, \[\], \[\], \[\], "Native build failed"\)\s*Mix\.raise/ end end end