From cd843cefeafbeafb800ee5808bf37e017a089d7d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 21:51:39 -0600 Subject: [PATCH 1/2] Fix: iOS bundle-id divergence and mob.deploy exiting 0 on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects found deploying one app to a physical iPhone + an Android phone with `bundle_id: "com.example.mishka_mob"` (Android's applicationId) and `ios_bundle_id: "com.genericjam.mishkamob"` in mob.exs. 1. `:ios_bundle_id` was silently ignored by the deployer. `Deployer.ios_bundle_id/0` was a stub delegating to `bundle_id/0`, so `mix mob.deploy --native --device ` installed the app under the configured id (NativeBuild honours it) and then pushed BEAMs at an id that was never installed: "App 'com.example.mishka_mob' is not installed on this device". Added `MobDev.Config.ios_bundle_id/0` and routed every iOS consumer through it — Deployer, Connector (including the sim `pgrep` sweep), `mob.provision` (it mints the profile that must cover the signed id), `mob.battery_bench_ios`. Android keeps `bundle_id/0`: the two ids often can't be the same string, since Apple forbids `_` and `com.example.*` is frequently claimed by another team. 2. `mix mob.deploy` exited 0 after printing "Failed on 1 device(s)". `failure_message/3` now drives a `Mix.raise` after the summary prints. `skipped` (app not installed for that platform) stays non-fatal, per the earlier fix that split it out of the Failed bucket; partial success IS fatal — the fan-out and reporting are unchanged, but a script can't see a missed device if the status code says all is well. 3. Sim and device builds disagreed about the bundle id. The sim bundle never stamped CFBundleIdentifier (inheriting ios/Info.plist) while the device bundle stamped the configured one. No signing step rewrites the id, so there was no reason to diverge: the sim path now stamps it too via the shared `NativeBuild.ios_bundle_id/1`, and both paths print the id they installed so the next simctl/devicectl command can use it. Rationale in decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 12 ++ ...e-id-single-source-and-deploy-exit-code.md | 72 ++++++++++++ lib/mix/tasks/mob.battery_bench_ios.ex | 4 +- lib/mix/tasks/mob.deploy.ex | 39 +++++++ lib/mix/tasks/mob.provision.ex | 4 +- lib/mob_dev/config.ex | 19 ++++ lib/mob_dev/connector.ex | 4 +- lib/mob_dev/deployer.ex | 9 +- lib/mob_dev/native_build.ex | 40 ++++++- test/mix/tasks/mob_deploy_beam_flags_test.exs | 48 ++++++++ test/mob_dev/config_test.exs | 105 +++++++++++++++++- test/mob_dev/native_build_test.exs | 31 ++++++ 12 files changed, 374 insertions(+), 13 deletions(-) create mode 100644 decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md diff --git a/AGENTS.md b/AGENTS.md index 90b6441..e98f0e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,6 +54,16 @@ mix test --exclude integration # skip the device-dependent ones prefers `Documents/otp/` over its complete signed bundle. Replace that directory rather than incrementally merging it, require `.beam` before transfer, and verify the received bootstrap bytes before restarting. +- **iOS bundle ids resolve through one function per side, never `bundle_id/0`.** + Consumers (deploy, connect, provision, battery bench) use + `MobDev.Config.ios_bundle_id/0`; the build (sim bundle, device bundle, + codesign) uses `NativeBuild.ios_bundle_id/1` over the loaded cfg. Both are + `:ios_bundle_id || :bundle_id`. Reaching for plain `bundle_id/0` on an + iOS path is the bug that installed an app under one id and pushed BEAMs at + another ("App '…' is not installed on this device" right after a + successful install). Android keeps `bundle_id/0` — the two ids often + cannot be the same string (Apple forbids `_`). See + `decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md`. - **`xcodebuild` errors get rewritten** to actionable hints by `diagnose_xcodebuild_failure/1` in `mob.provision`. Apple's verbatim text is preserved alongside our hint so the snippet stays google-able. Add new @@ -152,8 +162,10 @@ narrowing functions). Don't make them private: - `Mix.Tasks.Mob.Doctor.__zig_install_fix__/0` - `Mix.Tasks.Mob.Doctor.__zig_check_result__/1` - `Enable.inject_pythonx_dep/1`, `inject_pythonx_uv_init_gate/2`, `python_paths_module_template/1` +- `NativeBuild.ios_bundle_id/1` (the `:ios_bundle_id || :bundle_id` rule for the build side) - `Emulators.parse_simctl_json/1`, `find_emulator_binary/1` - `Provision.diagnose_xcodebuild_failure/1` +- `Mix.Tasks.Mob.Deploy.failure_message/3` (which bucket makes a deploy exit non-zero) If you make any of these private, every downstream test breaks loudly — but you'll lose the ability to evolve the parsers safely. diff --git a/decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md b/decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md new file mode 100644 index 0000000..ea98e77 --- /dev/null +++ b/decisions/2026-08-08-ios-bundle-id-single-source-and-deploy-exit-code.md @@ -0,0 +1,72 @@ +# One iOS bundle id everywhere, and a non-zero exit on failed deploy + +- Date: 2026-08-08 +- Status: accepted + +## Context + +Three defects found deploying one app to a physical iPhone and an Android +phone, with `bundle_id: "com.example.mishka_mob"` (Android's +`applicationId`, underscore and all) plus +`ios_bundle_id: "com.genericjam.mishkamob"` in `mob.exs`: + +1. `MobDev.Deployer` resolved the iOS id with `MobDev.Config.bundle_id/0`, + discarding `:ios_bundle_id`. `MobDev.NativeBuild` honoured it. So + `mix mob.deploy --native --device ` installed the app under the + configured id and then pushed BEAMs at an id that was never installed: + `App 'com.example.mishka_mob' is not installed on this device.` The only + workaround was clobbering `:bundle_id` — which Android may not accept + (Apple forbids `_` in a bundle id; `com.example.*` is frequently already + claimed by another Apple team). +2. That run printed `Failed on 1 device(s)` and exited **0**. +3. The iOS *simulator* bundle never stamped `CFBundleIdentifier`, so it + inherited whatever `ios/Info.plist` carried, while the *device* bundle + stamped the configured id. Same project, two ids, and no way to tell + which one a given build used. + +## Decision + +**One resolver per side.** `MobDev.Config.ios_bundle_id/0` +(`:ios_bundle_id || bundle_id/0`) is what every iOS-targeting *consumer* +resolves — `Deployer`, `Connector`, `mob.provision`, `mob.battery_bench_ios`. +`MobDev.NativeBuild.ios_bundle_id/1` (`cfg[:ios_bundle_id] || cfg[:bundle_id]`) +is the same rule over the already-loaded build config, used by the sim +bundle, the device bundle, and code signing. The two agree because +`load_config/0` resolves `cfg[:bundle_id]` through `Config.bundle_id/0`. + +`mob.provision` is included deliberately: it mints the provisioning profile, +which must cover the id `NativeBuild` actually signs. + +**Simulator now stamps `CFBundleIdentifier` too**, rather than accepting the +divergence and only reporting it. No signing step rewrites the id on either +path, so there was no technical reason for them to differ. For projects that +never set `:bundle_id`/`:ios_bundle_id` this is a no-op — `bundle_id/0` +already falls back to `ios/Info.plist`, so the stamped value equals the +inherited one. Both paths additionally *print* the id they installed, since +`simctl launch` / `devicectl` / `mob.connect` all need it and it appeared +nowhere in the build output. + +**`mix mob.deploy` exits non-zero when the `failed` bucket is non-empty** +(`Mix.Tasks.Mob.Deploy.failure_message/3` → `Mix.raise`), after the full +summary is printed. + +- `skipped` stays non-fatal. It means "app not installed for that platform", + the expected outcome of building `--ios` with an Android phone also + plugged in. This keeps faith with the earlier fix that split `skipped` out + of `Failed on N` in `format_summary/4`. +- Partial success is fatal. The fan-out is unchanged — every targeted device + is still attempted and reported, so an operator can see which ones got the + BEAMs. But a *script* cannot notice that one device missed out if the + status code says everything is fine, and that is precisely who the exit + code is for. + +## Consequences + +- A CI job that deploys to several devices and previously "passed" with one + device failing now fails. That is the point, but it is a behaviour change + for anyone who was relying on the old status code. +- `:ios_bundle_id` is now a genuinely usable setting rather than one the + build honours and the deployer ignores; cross-platform projects no longer + have to pick an id that satisfies both Apple and Android. +- `NativeBuild.ios_bundle_id/1` joins the public-but-undocumented seams + (listed in `AGENTS.md`) — public for testing, don't privatise. diff --git a/lib/mix/tasks/mob.battery_bench_ios.ex b/lib/mix/tasks/mob.battery_bench_ios.ex index dec409a..7107d12 100644 --- a/lib/mix/tasks/mob.battery_bench_ios.ex +++ b/lib/mix/tasks/mob.battery_bench_ios.ex @@ -201,7 +201,7 @@ defmodule Mix.Tasks.Mob.BatteryBenchIos do # device_id is what we pass to xcrun devicectl (install, launch, terminate). udid = device_id - pkg = MobDev.Config.bundle_id() + pkg = MobDev.Config.ios_bundle_id() cfg = MobDev.Config.load_mob_config() # Workspace discovery is only needed when building. Skip it with --no-build. @@ -645,7 +645,7 @@ defmodule Mix.Tasks.Mob.BatteryBenchIos do defp dry_run!(opts) do cfg = MobDev.Config.load_mob_config() - pkg = MobDev.Config.bundle_id() + pkg = MobDev.Config.ios_bundle_id() scheme = opts[:scheme] || cfg[:ios_scheme] || diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 6d76480..3e76764 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -116,6 +116,15 @@ defmodule Mix.Tasks.Mob.Deploy do # iOS simulator xcodebuild -scheme -destination 'platform=iOS Simulator,...' build xcrun simctl install booted .app + + ## Exit status + + Every targeted device is attempted and the full summary printed, then the + 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. """ @switches [ @@ -242,8 +251,38 @@ defmodule Mix.Tasks.Mob.Deploy do ) Enum.each(format_summary(deployed, failed, skipped, restart: restart), &IO.puts/1) + + # 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 + nil -> :ok + message -> Mix.raise(message) + end end + @doc """ + The `Mix.raise` message for a finished deploy, or `nil` when the run + should exit 0. + + A deploy that printed "Failed on N device(s)" used to still exit 0, so + CI and wrapper scripts read a failed deploy as a success. + + Only `failed` (a real error during push) is fatal. `skipped` is not: + it means "app not installed for that platform", the expected outcome of + e.g. building `--ios` with an Android phone also plugged in — the same + distinction `format_summary/4` renders. + + Partial success is still a failure. Every targeted device is still + attempted and reported before this runs, so the operator can see which + ones got the BEAMs; a *script* has no way to notice one device missed + 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: "Deploy failed on #{length(failed)} device(s) — see errors above" + @doc """ Build the per-deploy summary lines from the three device buckets. diff --git a/lib/mix/tasks/mob.provision.ex b/lib/mix/tasks/mob.provision.ex index 73f7b6f..866ba4b 100644 --- a/lib/mix/tasks/mob.provision.ex +++ b/lib/mix/tasks/mob.provision.ex @@ -282,7 +282,9 @@ defmodule Mix.Tasks.Mob.Provision do end defp check_bundle_id! do - bundle_id = MobDev.Config.bundle_id() + # Must match what NativeBuild signs the .app with, or the profile this + # task provisions won't cover the installed binary. + bundle_id = MobDev.Config.ios_bundle_id() IO.puts(" #{green()}✓#{reset()} Bundle ID — #{bundle_id}") bundle_id end diff --git a/lib/mob_dev/config.ex b/lib/mob_dev/config.ex index d5864a3..6c43ae2 100644 --- a/lib/mob_dev/config.ex +++ b/lib/mob_dev/config.ex @@ -28,6 +28,25 @@ defmodule MobDev.Config do "#{bundle_prefix()}.#{app_name()}" end + @doc """ + Returns the iOS bundle ID: `mob.exs`'s `:ios_bundle_id` when set, + otherwise `bundle_id/0`. + + iOS and Android often *cannot* share one identifier — Apple forbids + underscores in a bundle id (Android's `applicationId` allows them), and + a `com.example.*` id is frequently already claimed by another Apple team. + `:ios_bundle_id` is the per-platform escape hatch. + + Every iOS-targeting caller must resolve through here. The native build + signs and installs with this value, so a caller that talks to the + installed app by `bundle_id/0` instead (terminate/launch/`devicectl + copy`) addresses an id that was never installed — the failure surfaces + as "App '...' is not installed on this device" *after* a successful + install. + """ + @spec ios_bundle_id() :: String.t() + def ios_bundle_id, do: load_mob_config()[:ios_bundle_id] || bundle_id() + @doc """ Default reverse-DNS prefix when no platform manifest is available. Honors `MOB_BUNDLE_PREFIX` so users with a corporate prefix can set diff --git a/lib/mob_dev/connector.ex b/lib/mob_dev/connector.ex index e3b3498..a9b98f8 100644 --- a/lib/mob_dev/connector.ex +++ b/lib/mob_dev/connector.ex @@ -10,7 +10,7 @@ defmodule MobDev.Connector do defp bundle_id, do: MobDev.Config.bundle_id() defp android_package, do: bundle_id() - defp ios_bundle_id, do: bundle_id() + defp ios_bundle_id, do: MobDev.Config.ios_bundle_id() # ms to wait for node to appear @connect_timeout 25_000 # ms between polls @@ -161,7 +161,7 @@ defmodule MobDev.Connector do |> Enum.map(& &1.serial) |> MapSet.new() - case System.cmd("pgrep", ["-fl", bundle_id()], stderr_to_stdout: true) do + case System.cmd("pgrep", ["-fl", ios_bundle_id()], stderr_to_stdout: true) do {output, 0} -> output |> String.split("\n", trim: true) diff --git a/lib/mob_dev/deployer.ex b/lib/mob_dev/deployer.ex index 6ab3f24..829cee7 100644 --- a/lib/mob_dev/deployer.ex +++ b/lib/mob_dev/deployer.ex @@ -35,7 +35,7 @@ defmodule MobDev.Deployer do defp android_package, do: bundle_id() defp android_app_data, do: "/data/data/#{android_package()}/files" defp android_beams_dir, do: "#{android_app_data()}/otp/#{app_name()}" - defp ios_bundle_id, do: bundle_id() + defp ios_bundle_id, do: MobDev.Config.ios_bundle_id() defp ios_beams_dir do # The simulator's OTP_ROOT is resolved by `MobDev.Paths.sim_runtime_dir/1`. @@ -1176,9 +1176,12 @@ defmodule MobDev.Deployer do Then select your device in Xcode and press Run (⌘R). Alternatively, if you have another app with a different bundle ID already - installed on the device, update bundle_id in mob.exs to match it: + installed on the device, update the ID in mob.exs to match it: - config :mob_dev, bundle_id: "com.yourcompany.yourapp" + config :mob_dev, ios_bundle_id: "com.yourcompany.yourapp" + + (`ios_bundle_id` overrides `bundle_id` on iOS only — use it when + Android's applicationId isn't a legal Apple bundle ID.) """ else "devicectl copy failed: #{out}" diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index ea9e527..0d4095f 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -1669,7 +1669,7 @@ defmodule MobDev.NativeBuild do {:ok, sim_id} <- pick_ios_sim(device_id), binary_path = "ios/zig-out/#{display_name}", :ok <- check_path(binary_path, "iOS binary"), - {:ok, app_path} <- bundle_ios_app(binary_path, display_name), + {:ok, app_path} <- bundle_ios_app(binary_path, display_name, cfg), :ok <- copy_tflite_frameworks_ios( tflite_build, @@ -4240,7 +4240,14 @@ defmodule MobDev.NativeBuild do "Pass a full UDID or a case-insensitive prefix that matches " <> "exactly one booted sim. Run `mix mob.devices` to see what's available." - defp bundle_ios_app(binary_path, display_name) do + # The sim bundle used to keep whatever CFBundleIdentifier ios/Info.plist + # happened to carry while the device bundle stamped `ios_bundle_id/1` — two + # builds of one project installing under different ids, so + # `xcrun simctl launch ` failed and callers had to + # guess which id a given build had used. + defp bundle_ios_app(binary_path, display_name, cfg) do + bundle_id = ios_bundle_id(cfg) + build_dir = Path.join(System.tmp_dir!(), "mob_ios_bundle_#{System.unique_integer([:positive])}") @@ -4261,11 +4268,35 @@ defmodule MobDev.NativeBuild do File.cp!("ios/Info.plist", info_plist) apply_plugin_plist_keys!(info_plist) apply_fonts_to_ios_bundle!(info_plist, app_path) + plist_set!(info_plist, ":CFBundleIdentifier", bundle_id) if File.dir?("ios/Assets.xcassets/AppIcon.appiconset"), do: compile_ios_icons(app_path) + announce_bundle_id(bundle_id) {:ok, app_path} end end + # The installed bundle id is what every follow-up command needs (`xcrun + # simctl launch`, `xcrun devicectl`, `mix mob.connect`) and it appears + # nowhere else in the build output. + defp announce_bundle_id(bundle_id), + do: IO.puts(" Bundle identifier: #{IO.ANSI.cyan()}#{bundle_id}#{IO.ANSI.reset()}") + + @doc """ + The bundle id every iOS build path stamps and signs with: `mob.exs`'s + `:ios_bundle_id` when set, else `:bundle_id`. + + Single source of truth for the sim bundle, the device bundle, and code + signing — those three disagreeing is how a project ends up installed + under one id and addressed by another. Mirrors + `MobDev.Config.ios_bundle_id/0`, which is what the deploy/connect side + resolves (`cfg[:bundle_id]` is already `Config.bundle_id/0` by the time + `load_config/0` is done with it). + + Public for testing. + """ + @spec ios_bundle_id(keyword()) :: String.t() | nil + def ios_bundle_id(cfg), do: cfg[:ios_bundle_id] || cfg[:bundle_id] + defp compile_ios_icons(app_path) do actool_plist = Path.join(System.tmp_dir!(), "mob_actool_#{System.unique_integer([:positive])}.plist") @@ -4432,7 +4463,7 @@ defmodule MobDev.NativeBuild do defp bundle_ios_device_app(binary_path, otp_root, cfg, build_dir) do app_name = ios_display_name() app_module = Mix.Project.config() |> Keyword.fetch!(:app) |> Atom.to_string() - bundle_id = cfg[:ios_bundle_id] || cfg[:bundle_id] + bundle_id = ios_bundle_id(cfg) if is_nil(bundle_id), do: throw_bundle_id_error() @@ -4456,6 +4487,7 @@ defmodule MobDev.NativeBuild do plist_set!(info_plist, ":CFBundleIdentifier", bundle_id) plist_set!(info_plist, ":CFBundleExecutable", app_name) plist_set!(info_plist, ":CFBundleName", app_name) + announce_bundle_id(bundle_id) if File.dir?("ios/Assets.xcassets/AppIcon.appiconset"), do: compile_ios_device_icons(app_path) @@ -5805,7 +5837,7 @@ defmodule MobDev.NativeBuild do defp codesign_ios_device_app(app_path, cfg, build_dir) do sign_identity = cfg[:ios_sign_identity] team_id = cfg[:ios_team_id] - bundle_id = cfg[:ios_bundle_id] || cfg[:bundle_id] + bundle_id = ios_bundle_id(cfg) IO.puts(" === Code signing") entitlements = resolve_or_generate_entitlements(app_path, build_dir, team_id, bundle_id) diff --git a/test/mix/tasks/mob_deploy_beam_flags_test.exs b/test/mix/tasks/mob_deploy_beam_flags_test.exs index b93c53e..5db05fc 100644 --- a/test/mix/tasks/mob_deploy_beam_flags_test.exs +++ b/test/mix/tasks/mob_deploy_beam_flags_test.exs @@ -204,4 +204,52 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do refute joined =~ "Failed", "Bug fix: 5 not-installed devices must NOT count as failed" end end + + # ── failure_message/3 — deploy exit status ──────────────────────────────────── + # + # Original bug: a run that printed "Failed on 1 device(s)" still exited 0, + # so CI treated a failed deploy as a success. `failure_message/3` is the + # decision behind the `Mix.raise` — it must agree with `format_summary/4` + # about which bucket means failure. + + describe "failure_message/3" do + test "nothing attempted → no failure" do + assert Deploy.failure_message([], [], []) == nil + end + + test "all deployed → no failure" do + assert Deploy.failure_message([device("iPhone")], [], []) == nil + end + + test "skipped-because-not-installed is not a failure" do + skip = device("emulator-5554", "com.example not installed on emulator-5554") + assert Deploy.failure_message([], [], [skip]) == nil + end + + test "a failed device produces a message naming the count" do + message = Deploy.failure_message([], [device("buggy", "push timed out")], []) + assert message =~ "Deploy failed on 1 device(s)" + end + + test "partial success still fails — one bad device out of three" do + deployed = [device("iPhone"), device("emulator-5554")] + fail = device("emulator-5556", "adb push failed: broken pipe") + + assert Deploy.failure_message(deployed, [fail], []) =~ "Deploy failed on 1 device(s)" + end + + test "the iPhone bundle-id scenario from the bug report" do + # `mix mob.deploy --native --device ` installed the app, then the + # BEAM push hit the wrong bundle id. Summary said "Failed on 1", exit + # status said 0. + fail = device("Kevin's iPhone", "App 'com.example.mishka_mob' is not installed") + + assert Deploy.failure_message([], [fail], []) =~ "Deploy failed on 1 device(s)" + end + + test "counts every failed device, not just the first" do + failed = for i <- 1..3, do: device("emulator-#{i}", "adb push failed") + assert Deploy.failure_message([], failed, []) =~ "Deploy failed on 3 device(s)" + end + end end diff --git a/test/mob_dev/config_test.exs b/test/mob_dev/config_test.exs index 34ba588..ffb7a9c 100644 --- a/test/mob_dev/config_test.exs +++ b/test/mob_dev/config_test.exs @@ -1,8 +1,40 @@ defmodule MobDev.ConfigTest do - use ExUnit.Case, async: true + # bundle-id resolution reads mob.exs / ios/Info.plist relative to the cwd, + # so these tests chdir into a fixture project. + use ExUnit.Case, async: false alias MobDev.Config + setup do + cwd = File.cwd!() + tmp = Path.join(System.tmp_dir!(), "mob_config_test_#{System.unique_integer([:positive])}") + File.mkdir_p!(tmp) + File.cd!(tmp) + + on_exit(fn -> + File.cd!(cwd) + File.rm_rf!(tmp) + end) + + {:ok, tmp: tmp} + end + + defp write_mob_exs(tmp, body), do: File.write!(Path.join(tmp, "mob.exs"), body) + + defp write_info_plist(tmp, bundle_id) do + File.mkdir_p!(Path.join(tmp, "ios")) + + File.write!(Path.join([tmp, "ios", "Info.plist"]), """ + + + + CFBundleIdentifier + #{bundle_id} + + + """) + end + describe "parse_platforms/1" do test "nil (unset) defaults to both platforms" do assert Config.parse_platforms(nil) == [:android, :ios] @@ -34,4 +66,75 @@ defmodule MobDev.ConfigTest do assert Config.parse_platforms("ios") == [:android, :ios] end end + + # ── ios_bundle_id/0 ─────────────────────────────────────────────────────────── + # + # Original bug: `MobDev.Deployer` resolved the iOS id with plain + # `bundle_id/0`, discarding `:ios_bundle_id`. The native build honoured it, + # so `mix mob.deploy --native --device ` installed the app under the + # configured id and then pushed BEAMs to an id that was never installed + # ("App '...' is not installed on this device"). + # + # The two ids genuinely differ in real projects: Android's `applicationId` + # may contain underscores (`com.example.mishka_mob`) which Apple rejects. + + describe "ios_bundle_id/0" do + test ":ios_bundle_id wins over :bundle_id", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, + bundle_id: "com.example.mishka_mob", + ios_bundle_id: "com.genericjam.mishkamob" + """) + + assert Config.ios_bundle_id() == "com.genericjam.mishkamob" + end + + test "differs from bundle_id/0 — the Android id is left alone", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, + bundle_id: "com.example.mishka_mob", + ios_bundle_id: "com.genericjam.mishkamob" + """) + + assert Config.bundle_id() == "com.example.mishka_mob" + refute Config.ios_bundle_id() == Config.bundle_id() + end + + test "falls back to :bundle_id when :ios_bundle_id is unset", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, bundle_id: "com.example.shared" + """) + + assert Config.ios_bundle_id() == "com.example.shared" + end + + test "falls through the whole bundle_id/0 chain to ios/Info.plist", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, platforms: [:ios] + """) + + write_info_plist(tmp, "com.plisted.app") + + assert Config.ios_bundle_id() == "com.plisted.app" + end + + test ":ios_bundle_id beats ios/Info.plist", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, ios_bundle_id: "com.genericjam.mishkamob" + """) + + write_info_plist(tmp, "com.example.mishka_mob") + + assert Config.ios_bundle_id() == "com.genericjam.mishkamob" + end + + test "no mob.exs at all → same value as bundle_id/0" do + assert Config.ios_bundle_id() == Config.bundle_id() + end + end end diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index f87d6d8..1dbdae4 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -2218,4 +2218,35 @@ defmodule MobDev.NativeBuildTest do assert File.exists?(logo) end end + + # ── ios_bundle_id/1 ─────────────────────────────────────────────────────────── + # + # The sim bundle, the device bundle, and code signing must all stamp the + # same id. They used to disagree: the sim path never touched + # CFBundleIdentifier (so the sim app kept ios/Info.plist's value) while the + # device path stamped this one, and `MobDev.Deployer` addressed a third. + + describe "ios_bundle_id/1" do + test ":ios_bundle_id overrides :bundle_id" do + cfg = [bundle_id: "com.example.mishka_mob", ios_bundle_id: "com.genericjam.mishkamob"] + assert NativeBuild.ios_bundle_id(cfg) == "com.genericjam.mishkamob" + end + + test "falls back to :bundle_id when :ios_bundle_id is absent" do + assert NativeBuild.ios_bundle_id(bundle_id: "com.example.shared") == + "com.example.shared" + end + + test "an underscored Android applicationId does not leak into the iOS id" do + # Apple rejects `_` in a bundle id, so a project whose Android + # applicationId has one must set :ios_bundle_id — the case that + # surfaced the bug. + cfg = [bundle_id: "com.example.mishka_mob", ios_bundle_id: "com.genericjam.mishkamob"] + refute NativeBuild.ios_bundle_id(cfg) =~ "_" + end + + test "nil when neither key is set — load_config/0 always supplies :bundle_id" do + assert NativeBuild.ios_bundle_id([]) == nil + end + end end From c64e9dc7ea1958065667e3eaba3d5be7de19dc0d Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 4 Sep 2026 17:00:24 -0600 Subject: [PATCH 2/2] Finish the iOS bundle-id fix and stop it failing on an unrelated iPhone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial review of the rebased PR #44 found three blockers. All three are verified in source, and two of them were introduced or worsened by the PR itself. The device build looked its provisioning profile up by `cfg[:bundle_id]` while stamping and codesigning with `ios_bundle_id`. The PR changed `mob.provision` to mint the profile for the iOS id, so provision and deploy actively disagreed where before they were at least consistently wrong. The release path ignored `:ios_bundle_id` entirely — including three env vars named `MOB_IOS_*` reading the non-iOS setting, which stamps a submitted IPA with the Android applicationId. For the id that motivated this work that is `com.example.mishka_mob`, which App Store Connect rejects for the underscore. The AGENTS.md rule the PR adds was violated by the repo the moment it was written. Making failures fatal turned an unrelated iPhone into a broken build. Android reports "app not installed" as `:skipped`; iOS threw `{:error, ...}`, so a plain `mix mob.deploy` on a Mac with any iPhone attached that lacked the app went from exit 0 to exit 1. iOS now uses the same bucket. The requested-vs-incidental distinction — `--android` asking for Android and getting nothing should fail — is MOB-150 and deliberately not in here. Also fixed from the same review: `mix mob.uninstall` targeted the Android id on iOS and so removed nothing while reporting success; `mob.doctor` warned that `bundle_id` was unset for a project correctly setting only `ios_bundle_id`; and stamping `CFBundleIdentifier` crashed with a MatchError on an Info.plist lacking the key, since PlistBuddy `Set` fails on a missing key. Every headline fix in the original PR survived being reverted with the whole suite green — it tested both new resolvers and none of their call sites, which is where the bug lived both times. `Deployer.ios_bundle_id/0`, `Connector.ios_bundle_id/0` and `Uninstaller.resolve_apps_for_device/3` are now documented seams so the wiring is testable, and the paths that need a keychain or a device are pinned by source assertion. Nine mutations checked against an exact baseline, all caught, including the two the reviewer named: a skip absorbing a failure, and deleting the `Mix.raise` block while leaving the function that decides it fully covered. Device-verified on a physical iPhone SE with divergent ids (`bundle_id: com.example.mob_plugin_demo`, `ios_bundle_id: com.genericjam.mobplugindemo`): the app installs under the iOS id and the BEAM push targets the same id; and with the app uninstalled the run reports `Skipped on 1 device(s)` and exits 0 rather than failing. Worth recording that the original bug is worse than reported. On a machine that still has an older build under the other id, the push does not fail with "not installed" — it silently succeeds against the wrong app's container and prints a tick. Both containers accepted a write on the test device, and that is how the first attempt to reproduce the bug appeared to pass. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 5 + CHANGELOG.md | 45 ++++++++ lib/mix/tasks/mob.doctor.ex | 5 +- lib/mob_dev/connector.ex | 4 +- lib/mob_dev/deployer.ex | 66 ++++++----- lib/mob_dev/native_build.ex | 16 ++- lib/mob_dev/release.ex | 9 +- lib/mob_dev/uninstaller.ex | 12 +- test/mix/tasks/mob_deploy_beam_flags_test.exs | 16 +++ test/mob_dev/config_test.exs | 64 +++++++++++ test/mob_dev/ios_bundle_id_wiring_test.exs | 108 ++++++++++++++++++ 11 files changed, 317 insertions(+), 33 deletions(-) create mode 100644 test/mob_dev/ios_bundle_id_wiring_test.exs diff --git a/AGENTS.md b/AGENTS.md index e98f0e0..a5996b5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -163,9 +163,14 @@ narrowing functions). Don't make them private: - `Mix.Tasks.Mob.Doctor.__zig_check_result__/1` - `Enable.inject_pythonx_dep/1`, `inject_pythonx_uv_init_gate/2`, `python_paths_module_template/1` - `NativeBuild.ios_bundle_id/1` (the `:ios_bundle_id || :bundle_id` rule for the build side) +- `Deployer.ios_bundle_id/0`, `Connector.ios_bundle_id/0` (the same rule on the + consumer side — public so the WIRING is testable, not just the resolver; + reverting either to `bundle_id/0` was the original defect and the suite + did not notice) - `Emulators.parse_simctl_json/1`, `find_emulator_binary/1` - `Provision.diagnose_xcodebuild_failure/1` - `Mix.Tasks.Mob.Deploy.failure_message/3` (which bucket makes a deploy exit non-zero) +- `Uninstaller.resolve_apps_for_device/3` (which id gets uninstalled, per platform) If you make any of these private, every downstream test breaks loudly — but you'll lose the ability to evolve the parsers safely. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bcaced..d0211ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,51 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ## [Unreleased] +### 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 + attempted and the full summary still printed first — only the exit code + changed. A device that is *skipped* because the app is not installed on it + stays non-fatal on both platforms. **If you have CI that deploys to several + devices and has been passing, it may now fail** — check whether it was + passing on a partial deploy. + +### Added + +- **`:ios_bundle_id`** in `mob.exs`, for when Android's `applicationId` is not a + legal Apple bundle id. Apple forbids the underscores Android allows, and + `com.example.*` is often already claimed by another Apple team, so the two + frequently cannot be the same string. Every iOS path — deploy, connect, + provision, battery bench, uninstall, the simulator and device builds, + code-signing, and the release IPA — resolves `:ios_bundle_id || :bundle_id`. + Android keeps using `:bundle_id`. + +### Fixed + +- **iOS deploys installed the app under one bundle id and pushed BEAMs at + another.** `:ios_bundle_id` was resolved by the build but discarded by the + deployer and connector, so `mix mob.deploy --native --device ` installed + successfully and then failed with *"App '…' is not installed on this device."* + On a machine that also had an older build under the other id it was worse: the + push silently succeeded against the wrong app and reported success. +- **The simulator and device builds disagreed about the bundle id.** The + simulator build took it verbatim from `ios/Info.plist` while the device build + used the configured id, so `simctl launch ` failed. Both paths + now stamp and print the id they installed. +- **The device build looked its provisioning profile up by the Android id**, so + a profile minted by `mix mob.provision` (which uses the iOS id) was never + matched. +- **The release IPA was stamped and signed with the Android `applicationId`**, + which App Store Connect rejects when it contains an underscore. +- **`mix mob.uninstall` targeted the Android id on iOS devices**, removing + nothing and reporting success. +- **`mix mob.doctor` warned that `bundle_id` was unset** for a project that + correctly set only `ios_bundle_id`. +- **Stamping `CFBundleIdentifier` crashed on an `Info.plist` that lacked the + key** — plausible for `mix mob.adopt` projects — instead of adding it. + ## [0.6.33] - 2026-09-04 ### Fixed diff --git a/lib/mix/tasks/mob.doctor.ex b/lib/mix/tasks/mob.doctor.ex index 53351a7..0939966 100644 --- a/lib/mix/tasks/mob.doctor.ex +++ b/lib/mix/tasks/mob.doctor.ex @@ -567,7 +567,10 @@ defmodule Mix.Tasks.Mob.Doctor do end defp check_bundle_id(cfg) do - case cfg[:bundle_id] do + # `ios_bundle_id` alone is a legitimate configuration — an iOS-only project + # whose Apple id is the only one it needs. Warning about a missing + # `bundle_id` there sends the user to add a key nothing reads. + case cfg[:bundle_id] || cfg[:ios_bundle_id] do nil -> {:warn, "bundle_id", "not set in mob.exs (only needed for mob.battery_bench)", "Add to mob.exs: config :mob_dev, bundle_id: \"com.example.myapp\""} diff --git a/lib/mob_dev/connector.ex b/lib/mob_dev/connector.ex index a9b98f8..e46f83f 100644 --- a/lib/mob_dev/connector.ex +++ b/lib/mob_dev/connector.ex @@ -10,7 +10,9 @@ defmodule MobDev.Connector do defp bundle_id, do: MobDev.Config.bundle_id() defp android_package, do: bundle_id() - defp ios_bundle_id, do: MobDev.Config.ios_bundle_id() + @doc false + @spec ios_bundle_id() :: String.t() | nil + def ios_bundle_id, do: MobDev.Config.ios_bundle_id() # ms to wait for node to appear @connect_timeout 25_000 # ms between polls diff --git a/lib/mob_dev/deployer.ex b/lib/mob_dev/deployer.ex index 829cee7..f05cd9e 100644 --- a/lib/mob_dev/deployer.ex +++ b/lib/mob_dev/deployer.ex @@ -35,7 +35,9 @@ defmodule MobDev.Deployer do defp android_package, do: bundle_id() defp android_app_data, do: "/data/data/#{android_package()}/files" defp android_beams_dir, do: "#{android_app_data()}/otp/#{app_name()}" - defp ios_bundle_id, do: MobDev.Config.ios_bundle_id() + @doc false + @spec ios_bundle_id() :: String.t() | nil + def ios_bundle_id, do: MobDev.Config.ios_bundle_id() defp ios_beams_dir do # The simulator's OTP_ROOT is resolved by `MobDev.Paths.sim_runtime_dir/1`. @@ -1163,31 +1165,38 @@ defmodule MobDev.Deployer do :ok {out, _} -> - reason = - if String.contains?(out, "ContainerLookupErrorDomain") do - """ - App '#{bundle}' is not installed on this device. - - To fix this, you need to build and install the app on the device first. - The easiest way is to open the ios/ directory in Xcode and run on device: - - open ios/*.xcodeproj (or ios/*.xcworkspace) - - Then select your device in Xcode and press Run (⌘R). - - Alternatively, if you have another app with a different bundle ID already - installed on the device, update the ID in mob.exs to match it: - - config :mob_dev, ios_bundle_id: "com.yourcompany.yourapp" - - (`ios_bundle_id` overrides `bundle_id` on iOS only — use it when - Android's applicationId isn't a legal Apple bundle ID.) - """ - else - "devicectl copy failed: #{out}" - end - - throw({:error, reason}) + # "Not installed" is the same condition Android reports as :skipped + # (deploy_android/2), and it must be bucketed the same way. It means + # "this device is not a target for this app" — a phone that happens to + # be plugged in — not "the deploy failed". Since mob.deploy started + # returning a non-zero exit code for failures, tagging this as an + # error made a plain `mix mob.deploy` fail on any Mac with an + # unrelated iPhone attached. + if String.contains?(out, "ContainerLookupErrorDomain") do + throw( + {:skipped, + """ + App '#{bundle}' is not installed on this device. + + To fix this, you need to build and install the app on the device first. + The easiest way is to open the ios/ directory in Xcode and run on device: + + open ios/*.xcodeproj (or ios/*.xcworkspace) + + Then select your device in Xcode and press Run (⌘R). + + Alternatively, if you have another app with a different bundle ID already + installed on the device, update the ID in mob.exs to match it: + + config :mob_dev, ios_bundle_id: "com.yourcompany.yourapp" + + (`ios_bundle_id` overrides `bundle_id` on iOS only — use it when + Android's applicationId isn't a legal Apple bundle ID.) + """} + ) + else + throw({:error, "devicectl copy failed: #{out}"}) + end end received_bootstrap = Path.join(staging_parent, "received_#{app}.beam") @@ -1212,6 +1221,11 @@ defmodule MobDev.Deployer do Process.delete(:mob_ios_override_replaced) {:ok, device} catch + # Not annotated with override state: the copy never started, so nothing + # on the device was replaced and there is no partial override to warn about. + {:skipped, reason} -> + {:skipped, reason} + {:error, reason} -> {:error, annotate_override_state(reason, app)} after diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 0d4095f..2d95a39 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -4519,6 +4519,14 @@ defmodule MobDev.NativeBuild do end defp plist_set!(plist, key, value) do + # `Set` fails outright on a key that is not already present, which for an + # adopted project's hand-written Info.plist turned a previously working + # build into a MatchError. Add first and ignore its failure when the key + # does exist — the same idiom `release.ex` uses. + System.cmd("/usr/libexec/PlistBuddy", ["-c", "Add #{key} string #{value}", plist], + stderr_to_stdout: true + ) + {_, 0} = System.cmd("/usr/libexec/PlistBuddy", ["-c", "Set #{key} #{value}", plist], stderr_to_stdout: true @@ -6052,7 +6060,13 @@ defmodule MobDev.NativeBuild do # keychain and provisioning profile directories. Fails with a clear message only # when auto-detection itself finds multiple candidates and can't pick one. defp check_device_signing_config(cfg) do - bundle_id = cfg[:bundle_id] + # The iOS id, not the generic one. `bundle_ios_device_app/3` stamps this id + # and `codesign_ios_device_app/3` signs against the profile chosen here, so + # looking it up by the Android id searches for a profile that was never + # minted — `mix mob.provision` creates it for `ios_bundle_id`. The two ids + # often cannot be the same string, since Apple rejects the underscores + # Android's applicationId allows. + bundle_id = ios_bundle_id(cfg) with {:ok, identity} <- resolve_sign_identity(cfg[:ios_sign_identity], cfg[:ios_team_id]), {:ok, {profile_uuid, team_id}} <- diff --git a/lib/mob_dev/release.ex b/lib/mob_dev/release.ex index 00f4673..f624960 100644 --- a/lib/mob_dev/release.ex +++ b/lib/mob_dev/release.ex @@ -112,7 +112,12 @@ defmodule MobDev.Release do @doc false @spec resolve_distribution_signing(keyword()) :: {:ok, keyword()} | {:error, String.t()} def resolve_distribution_signing(cfg) do - bundle_id = cfg[:bundle_id] + # See `NativeBuild.check_device_signing_config/1`: the distribution profile + # must be found by the id the IPA is actually stamped with. Getting this + # wrong ships an App Store build under the Android applicationId, which for + # a `com.example.*` id containing an underscore App Store Connect rejects + # outright. + bundle_id = MobDev.NativeBuild.ios_bundle_id(cfg) with {:ok, identity} <- resolve_dist_identity(cfg[:ios_dist_sign_identity]), {:ok, {profile_uuid, team_id}} <- @@ -358,7 +363,7 @@ defmodule MobDev.Release do {"MOB_ELIXIR_LIB", Path.expand(elixir_lib)}, {"MOB_IOS_DEVICE_OTP_ROOT", otp_root}, {"MOB_IOS_EPMD_BUILD_SRC", epmd_src}, - {"MOB_IOS_BUNDLE_ID", cfg[:bundle_id]}, + {"MOB_IOS_BUNDLE_ID", MobDev.NativeBuild.ios_bundle_id(cfg)}, {"MOB_IOS_TEAM_ID", cfg[:ios_team_id]}, {"MOB_IOS_SIGN_IDENTITY", cfg[:ios_dist_sign_identity]}, {"MOB_IOS_PROFILE_UUID", cfg[:ios_dist_profile_uuid]}, diff --git a/lib/mob_dev/uninstaller.ex b/lib/mob_dev/uninstaller.ex index 0bb573b..64cb074 100644 --- a/lib/mob_dev/uninstaller.ex +++ b/lib/mob_dev/uninstaller.ex @@ -397,7 +397,9 @@ defmodule MobDev.Uninstaller do # ── App resolution per device ────────────────────────────────────────── - defp resolve_apps_for_device(device, opts, bundle_prefix) do + @doc false + @spec resolve_apps_for_device(Device.t(), keyword(), String.t()) :: [String.t()] + def resolve_apps_for_device(device, opts, bundle_prefix) do cond do opts[:bundle_id] -> [opts[:bundle_id]] @@ -406,10 +408,16 @@ defmodule MobDev.Uninstaller do list_matching_packages(device, bundle_prefix) true -> - [opts[:project_bundle_id] || MobDev.Config.bundle_id()] + # Per platform: an iOS device holds the app under `ios_bundle_id`, so + # uninstalling by the generic id silently removes nothing and reports + # success. Same defect class as the deploy path this release fixed. + [opts[:project_bundle_id] || default_bundle_id(device)] end end + defp default_bundle_id(%Device{platform: :ios}), do: MobDev.Config.ios_bundle_id() + defp default_bundle_id(_device), do: MobDev.Config.bundle_id() + defp list_matching_packages(%Device{platform: :android, serial: serial}, prefix) do {output, _} = System.cmd("adb", ["-s", serial, "shell", "pm", "list", "packages", prefix], diff --git a/test/mix/tasks/mob_deploy_beam_flags_test.exs b/test/mix/tasks/mob_deploy_beam_flags_test.exs index 5db05fc..32c7e19 100644 --- a/test/mix/tasks/mob_deploy_beam_flags_test.exs +++ b/test/mix/tasks/mob_deploy_beam_flags_test.exs @@ -226,6 +226,22 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do assert Deploy.failure_message([], [], [skip]) == nil end + test "a skipped device does not absorb a failed one" do + # The combination the real bug produces — an iPhone that failed and an + # Android emulator without the app, on one default run — and the only + # combination no test covered. Two mutations passed the whole describe + # block without it: an added `failure_message(_, _, [_ | _]), do: nil` + # clause ("any skip makes the run non-fatal"), and counting + # `length(failed) + length(skipped)`. + failed = device("iPhone", "push timed out") + skipped = device("emulator-5554", "com.example not installed on emulator-5554") + + message = Deploy.failure_message([], [failed], [skipped]) + + assert message =~ "Deploy failed on 1 device(s)", + "the skipped device must neither suppress the failure nor inflate the count" + end + test "a failed device produces a message naming the count" do message = Deploy.failure_message([], [device("buggy", "push timed out")], []) assert message =~ "Deploy failed on 1 device(s)" diff --git a/test/mob_dev/config_test.exs b/test/mob_dev/config_test.exs index ffb7a9c..276ae14 100644 --- a/test/mob_dev/config_test.exs +++ b/test/mob_dev/config_test.exs @@ -78,6 +78,70 @@ defmodule MobDev.ConfigTest do # The two ids genuinely differ in real projects: Android's `applicationId` # may contain underscores (`com.example.mishka_mob`) which Apple rejects. + describe "uninstall resolves the id per platform" do + # An iOS device holds the app under `ios_bundle_id`, so uninstalling by the + # Android applicationId removes nothing and reports success — the same + # defect class as the deploy path, and missed by the original audit. + # + # Lives here rather than in `MobDev.UninstallerTest` because resolution + # reads `mob.exs` from the current working directory, and that module is + # `async: true` — changing the cwd there would corrupt its neighbours. + setup %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, + bundle_id: "com.example.mishka_mob", + ios_bundle_id: "com.genericjam.mishkamob" + """) + + :ok + end + + test "an iOS device resolves the iOS id" do + device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios} + + assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == + ["com.genericjam.mishkamob"] + end + + test "an Android device keeps the applicationId" do + device = %MobDev.Device{name: "emu", serial: "emulator-5554", platform: :android} + + assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == + ["com.example.mishka_mob"] + end + + test "an explicit --bundle-id still wins" do + device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios} + + assert MobDev.Uninstaller.resolve_apps_for_device(device, [bundle_id: "com.o.x"], "com.") == + ["com.o.x"] + end + end + + describe "the consumers are actually wired to it (MOB-150 review S1)" do + # The resolver was tested; the WIRING was not. Reverting either consumer to + # `defp ios_bundle_id, do: bundle_id()` — the original defect, byte for + # byte, in both modules — left the entire suite green. These pin the seam + # rather than the rule. + for {mod, name} <- [{MobDev.Deployer, "Deployer"}, {MobDev.Connector, "Connector"}] do + test "#{name}.ios_bundle_id/0 resolves the iOS id, not the Android one", %{tmp: tmp} do + write_mob_exs(tmp, """ + import Config + config :mob_dev, + bundle_id: "com.example.mishka_mob", + ios_bundle_id: "com.genericjam.mishkamob" + """) + + assert unquote(mod).ios_bundle_id() == "com.genericjam.mishkamob" + + refute unquote(mod).ios_bundle_id() == MobDev.Config.bundle_id(), + "an iOS consumer resolving the Android id installs under one id " <> + "and pushes at another" + end + end + end + describe "ios_bundle_id/0" do test ":ios_bundle_id wins over :bundle_id", %{tmp: tmp} do write_mob_exs(tmp, """ diff --git a/test/mob_dev/ios_bundle_id_wiring_test.exs b/test/mob_dev/ios_bundle_id_wiring_test.exs new file mode 100644 index 0000000..6e5a740 --- /dev/null +++ b/test/mob_dev/ios_bundle_id_wiring_test.exs @@ -0,0 +1,108 @@ +# credo:disable-for-this-file Jump.CredoChecks.VacuousTest +# +# These assert on source text rather than calling application code, which is +# 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 + @moduledoc """ + Every iOS path must resolve its bundle id through the `:ios_bundle_id || + :bundle_id` rule. + + Reaching for the generic `bundle_id/0` on an iOS path is the defect this was + written to eliminate: the app installs under one id and everything afterwards + addresses another. It was found twice in the deploy path, and a review then + found three more sites the original audit missed. + + These are source assertions because the paths need a keychain, a provisioning + profile or a physical device to execute. The behavioural seams are tested in + `MobDev.ConfigTest`; these cover what cannot be reached from a unit test. + """ + use ExUnit.Case, async: true + + @native_build File.read!(Path.expand("../../lib/mob_dev/native_build.ex", __DIR__)) + @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__)) + + defp region(source, from, to) do + unless String.contains?(source, from), do: flunk("marker not found: #{inspect(from)}") + [_, rest] = String.split(source, from, parts: 2) + unless String.contains?(rest, to), do: flunk("end marker not found: #{inspect(to)}") + [body | _] = String.split(rest, to, parts: 2) + body + end + + describe "the device build signs against a profile for the id it stamps" do + test "check_device_signing_config/1 looks the profile up by the iOS id" do + # Without this the build searches for a profile minted for the Android + # applicationId. `mix mob.provision` creates it for `ios_bundle_id`, so + # the two disagree and the build falls back to a wildcard or fails + # naming an id Apple would reject outright. + body = region(@native_build, "defp check_device_signing_config(cfg) do", "\n end") + + assert body =~ "bundle_id = ios_bundle_id(cfg)" + refute body =~ "bundle_id = cfg[:bundle_id]" + end + end + + describe "the release/IPA path" do + test "resolves the distribution profile by the iOS id" do + body = region(@release, "def resolve_distribution_signing(cfg) do", "\n end") + + assert body =~ "MobDev.NativeBuild.ios_bundle_id(cfg)" + refute body =~ "bundle_id = cfg[:bundle_id]" + end + + test "MOB_IOS_BUNDLE_ID carries the iOS id" do + # The env var is named for iOS and fed the App Store build's + # CFBundleIdentifier. Reading `:bundle_id` here stamps a submitted IPA + # with the Android applicationId — which, for the id that motivated this + # work, App Store Connect rejects for containing an underscore. + assert @release =~ ~s|{"MOB_IOS_BUNDLE_ID", MobDev.NativeBuild.ios_bundle_id(cfg)}| + refute @release =~ ~s|{"MOB_IOS_BUNDLE_ID", cfg[:bundle_id]}| + end + end + + describe "a device without the app is skipped, not failed" do + test "the iOS not-installed branch throws :skipped" do + # Android already reports this as `{:skipped, ...}` — it means "this + # device is not a target", not "the deploy failed". Once mob.deploy + # started returning a non-zero exit code, tagging the iOS case as an + # error made a plain `mix mob.deploy` fail on any Mac with an unrelated + # iPhone attached. + body = region(@deployer, "ContainerLookupErrorDomain", "devicectl copy failed") + + assert body =~ "throw(\n {:skipped," + refute body =~ "throw({:error, reason})" + end + + test "the catch clause returns it as skipped without override annotation" do + # The copy never began, so nothing on the device was replaced and there + # is no partial-override warning to attach. + body = region(@deployer, " catch\n # Not annotated", "\n after") + + assert body =~ "{:skipped, reason} ->" + refute body =~ "{:skipped, reason} ->\n {:error," + end + end + + describe "the exit code is actually wired to the decision" do + test "failure_message/3 drives a Mix.raise, after the summary" do + # `failure_message/3` is well tested as a pure function, but deleting the + # block that CALLS it left the whole suite green — the deploy would go + # back to exiting 0 on failure, the original bug, with the function that + # 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 =~ "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 + end + end +end