Make mob.uninstall actually honour :ios_bundle_id, and stop the deploy error lying

CHANGELOG.md

@@ -21,6 +21,21 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev).
 
 ### Fixed
 
+- **`mix mob.deploy --help` and `mix mob.mutate --help` refused to help.**
+  Strict option parsing turned the flag people try first, on the task they run
+  most, into `Unrecognized or invalid option(s): --help` — telling the reader
+  to go and read the help. Both now defer to `MobDev.TaskHelp`, as
+  `mix mob.uninstall` already did.
+- **`mix mob.uninstall` ignored `:ios_bundle_id`.** The task resolved one
+  bundle id for both platforms and passed it down, so the per-platform clause
+  that was supposed to pick `:ios_bundle_id` for an iOS device never ran. On a
+  device holding the app under a different iOS id, uninstall removed nothing
+  and reported success. The accompanying test passed an opts shape the task
+  never sends, which is why the suite stayed green.
+- **`mix mob.deploy`'s error told you the equals form was required for
+  dash-prefixed values.** It stopped being true in the same commit that wrote
+  it, once the spaced form was handled.
+
 - **`--beam-flags "-S 4:4 -A 4"` aborted the deploy.** `OptionParser` will not
   consume a dash-prefixed argument as a string value, so the spelling this
   repo prints in seven places — the moduledoc, five README recipes and both

lib/mix/tasks/mob.deploy.ex

@@ -179,6 +179,14 @@ defmodule Mix.Tasks.Mob.Deploy do
 
   @impl Mix.Task
   def run(args) do
+    if MobDev.TaskHelp.help_requested?(args) do
+      MobDev.TaskHelp.print_module_help(__MODULE__)
+    else
+      do_run(args)
+    end
+  end
+
+  defp do_run(args) do
     {opts, argv, invalid} =
       args
       |> join_dashed_values()
@@ -495,8 +503,7 @@ defmodule Mix.Tasks.Mob.Deploy do
       end)
 
     "Unrecognized or invalid option(s): #{names}\n\n" <>
-      "Run `mix help mob.deploy` for the accepted options.\n" <>
-      "A value beginning with `-` needs the equals form: --beam-flags=\"-S 4:4\"."
+      "Run `mix help mob.deploy` for the accepted options."
   end
 
   @doc """

lib/mix/tasks/mob.mutate.ex

@@ -60,6 +60,14 @@ defmodule Mix.Tasks.Mob.Mutate do
 
   @impl Mix.Task
   def run(args) do
+    if MobDev.TaskHelp.help_requested?(args) do
+      MobDev.TaskHelp.print_module_help(__MODULE__)
+    else
+      do_run(args)
+    end
+  end
+
+  defp do_run(args) do
     {opts, _argv, invalid} = OptionParser.parse(args, strict: @switches)
 
     unless invalid == [] do

lib/mix/tasks/mob.uninstall.ex

@@ -104,11 +104,14 @@ defmodule Mix.Tasks.Mob.Uninstall do
     {opts, _positional, _} = OptionParser.parse(args, strict: @switches, aliases: @aliases)
     device_ids = Keyword.get_values(opts, :device)
 
-    project_bundle_id =
-      case Mix.Project.get() do
-        nil -> nil
-        _ -> MobDev.Config.bundle_id()
-      end
+    # Whether we are in a Mix project — NOT which id to use. Resolving that
+    # here picked one id for both platforms, and the `||` in
+    # `resolve_apps_for_device/3` then short-circuited past the per-platform
+    # clause, so the iOS branch was dead on every real invocation and
+    # `mix mob.uninstall` removed nothing on a device holding the app under
+    # `:ios_bundle_id`. The test that was supposed to cover it passed `[]` for
+    # opts, which is not what this task passes.
+    in_project? = Mix.Project.get() != nil
 
     uninstaller_opts = [
       device_ids: device_ids,
@@ -117,7 +120,7 @@ defmodule Mix.Tasks.Mob.Uninstall do
       bundle_id: opts[:bundle_id],
       all_apps: Keyword.get(opts, :all_apps, false),
       bundle_prefix: opts[:bundle_prefix],
-      project_bundle_id: project_bundle_id
+      in_project: in_project?
     ]
 
     case Uninstaller.plan(uninstaller_opts) do

lib/mob_dev/uninstaller.ex

@@ -407,11 +407,14 @@ defmodule MobDev.Uninstaller do
       opts[:all_apps] ->
         list_matching_packages(device, bundle_prefix)
 
+      opts[:in_project] ->
+        # Per platform, resolved HERE because only here is the device known.
+        # An iOS device holds the app under `:ios_bundle_id`; uninstalling by
+        # the Android applicationId removes nothing and reports success.
+        [default_bundle_id(device)]
+
       true ->
-        # 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
 

test/mix/tasks/mob_deploy_parsing_test.exs

@@ -85,8 +85,22 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do
       assert message =~ "Unrecognized or invalid"
     end
 
-    test "the message points at the equals form for dashed values" do
-      assert Deploy.invalid_options_message([{"--beam-flags", nil}]) =~ "--beam-flags="
+    test "the message does not claim the equals form is required" do
+      # It was, before `join_dashed_values/1` landed in the same commit that
+      # wrote this advice. Telling someone their working spelling is wrong is
+      # its own small version of a tool that lies.
+      message = Deploy.invalid_options_message([{"--beam-flags", nil}])
+
+      refute message =~ "equals form"
+      assert message =~ "mix help mob.deploy"
+    end
+  end
+
+  describe "--help" do
+    test "the helper recognises both spellings" do
+      assert MobDev.TaskHelp.help_requested?(["--help"])
+      assert MobDev.TaskHelp.help_requested?(["-h"])
+      refute MobDev.TaskHelp.help_requested?(["--device", "x"])
     end
   end
 end

test/mob_dev/config_test.exs

@@ -97,20 +97,33 @@ defmodule MobDev.ConfigTest do
       :ok
     end
 
+    # `[]` is not what the task passes, and that is why this suite stayed green
+    # while the feature did not work: `mix mob.uninstall` sent a
+    # `project_bundle_id` it had already resolved, the `||` short-circuited,
+    # and the per-platform clause was dead on every real invocation. These now
+    # use the task's own opts shape.
+    @task_opts [in_project: true]
+
     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.") ==
+      assert MobDev.Uninstaller.resolve_apps_for_device(device, @task_opts, "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.") ==
+      assert MobDev.Uninstaller.resolve_apps_for_device(device, @task_opts, "com.") ==
                ["com.example.mishka_mob"]
     end
 
+    test "outside a Mix project there is no project app to uninstall" do
+      device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios}
+
+      assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == []
+    end
+
     test "an explicit --bundle-id still wins" do
       device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios}
 

test/mob_dev/wiring_test.exs

@@ -211,4 +211,32 @@ defmodule MobDev.WiringTest do
                ~r/unless invalid == \[\] do\s*Mix\.raise\(invalid_options_message\(invalid\)\)/
     end
   end
+
+  describe "mob.uninstall resolves the bundle id where the device is known" do
+    @uninstall_task File.read!(Path.expand("../../lib/mix/tasks/mob.uninstall.ex", __DIR__))
+
+    test "the task passes whether it is in a project, not which id to use" do
+      # The gap that hid the bug: the task resolved one id for both platforms
+      # and passed it down, so the `||` in resolve_apps_for_device/3
+      # short-circuited past the per-platform clause. The unit test passed an
+      # opts shape the task never sends, so it could not see any of that.
+      assert @uninstall_task =~ "in_project: in_project?"
+
+      refute @uninstall_task =~ "project_bundle_id:",
+             "resolving the id in the task is what made the iOS branch dead code"
+    end
+  end
+
+  describe "--help is answered, not refused" do
+    # Strict parsing made `mix mob.deploy --help` fail with "Unrecognized or
+    # invalid option(s): --help" — the flag people try first, on the task they
+    # run most, telling them to go read the help. `mob.uninstall` had already
+    # solved this with MobDev.TaskHelp; deploy and mutate had not.
+    for task <- ~w(mob.deploy mob.mutate) do
+      test "#{task} defers to MobDev.TaskHelp" do
+        path = Path.expand("../../lib/mix/tasks/#{unquote(task)}.ex", __DIR__)
+        assert File.read!(path) =~ "MobDev.TaskHelp.help_requested?(args)"
+      end
+    end
+  end
 end