diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a9881a..06cdcae 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 9d2e2f3..7158f9b 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/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 """ diff --git a/lib/mix/tasks/mob.mutate.ex b/lib/mix/tasks/mob.mutate.ex index c00c75b..c0eaa25 100644 --- a/lib/mix/tasks/mob.mutate.ex +++ b/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 diff --git a/lib/mix/tasks/mob.uninstall.ex b/lib/mix/tasks/mob.uninstall.ex index d2839e7..8b78a15 100644 --- a/lib/mix/tasks/mob.uninstall.ex +++ b/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 diff --git a/lib/mob_dev/uninstaller.ex b/lib/mob_dev/uninstaller.ex index 64cb074..f9d1f14 100644 --- a/lib/mob_dev/uninstaller.ex +++ b/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 diff --git a/test/mix/tasks/mob_deploy_parsing_test.exs b/test/mix/tasks/mob_deploy_parsing_test.exs index ed11d26..d4fab47 100644 --- a/test/mix/tasks/mob_deploy_parsing_test.exs +++ b/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 diff --git a/test/mob_dev/config_test.exs b/test/mob_dev/config_test.exs index 276ae14..99acd31 100644 --- a/test/mob_dev/config_test.exs +++ b/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} diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index b9e9f04..f68e2e9 100644 --- a/test/mob_dev/wiring_test.exs +++ b/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