diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d246d4..dc19c64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ## [Unreleased] +### Fixed + +- **`mix mob.deploy` silently ignored unknown options.** `-d` was never aliased + to `--device` here — `mob.connect` has aliased it all along — so + `mix mob.deploy -d ` deployed to *every* connected device instead of + the one named, without a word. A typo'd `--devcie` did the same. Parsing is + strict now and the run refuses, naming the offending options. + ### Changed - **`mix mob.deploy` no longer exits 0 after shipping nothing.** Four ways it diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 31526ef..1b79bc2 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -179,7 +179,18 @@ defmodule Mix.Tasks.Mob.Deploy do @impl Mix.Task def run(args) do - {opts, _, _} = OptionParser.parse(args, switches: @switches) + {opts, _argv, invalid} = + OptionParser.parse(args, strict: @switches, aliases: [d: :device]) + + # `switches:` silently discards anything it does not recognise, so + # `mix mob.deploy -d ` — `-d` was never aliased here, though + # `mob.connect` has aliased it all along — deployed to every device + # instead of the one named, and said nothing. A typo'd `--devcie` did the + # same. `strict:` collects them so the run can refuse rather than do + # something other than what was asked. + unless invalid == [] do + Mix.raise(invalid_options_message(invalid)) + end # 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 @@ -410,6 +421,21 @@ defmodule Mix.Tasks.Mob.Deploy do end end + @doc """ + The error for options the task does not accept. + + Names them, because the failure this replaces was silent: the flag was + dropped and the deploy proceeded as if it had never been passed. + """ + @spec invalid_options_message([{String.t(), String.t() | nil}]) :: String.t() + def invalid_options_message(invalid) do + names = invalid |> Enum.map(&elem(&1, 0)) |> Enum.join(", ") + + "Unknown option(s): #{names}\n\n" <> + "Run `mix help mob.deploy` for the accepted options. " <> + "Short forms are not aliases except `-d` for `--device`." + end + @doc """ The machine-readable result of a finished deploy. diff --git a/test/mix/tasks/mob_deploy_beam_flags_test.exs b/test/mix/tasks/mob_deploy_beam_flags_test.exs index f619a2c..be032e5 100644 --- a/test/mix/tasks/mob_deploy_beam_flags_test.exs +++ b/test/mix/tasks/mob_deploy_beam_flags_test.exs @@ -463,4 +463,19 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do assert result |> Jason.encode!() |> Jason.decode!() == result end end + + describe "unknown options are refused, not discarded" do + # `mix mob.deploy -d ` deployed to every device instead of the one + # named, and said nothing: `-d` was never aliased here even though + # `mob.connect` has aliased it all along, and `switches:` silently drops + # what it does not recognise. Found while verifying MOB-151, when two + # native builds went to the wrong device for this reason. + test "the message names the offending options" do + message = Deploy.invalid_options_message([{"-d", nil}, {"--devcie", "x"}]) + + assert message =~ "-d" + assert message =~ "--devcie" + assert message =~ "Unknown option" + end + end end diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index d2690e7..7aab467 100644 --- a/test/mob_dev/wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -179,4 +179,33 @@ defmodule MobDev.WiringTest do ~r/emit_json\(opts, \[\], \[\], \[\], "Native build failed"\)\s*Mix\.raise/ end end + + describe "mob.deploy refuses what it does not recognise" do + # `-d` was never aliased here, and `switches:` drops unknown flags in + # silence — so `mix mob.deploy -d ` deployed to every device instead + # of the one named. Source assertions because the parse happens inside + # run/1, before anything testable is reached. + test "-d is aliased to --device, matching mob.connect" do + source = @deploy_task + + assert source =~ "aliases: [d: :device]" + end + + test "parsing is strict, so nothing is dropped in silence" do + # `switches:` puts an unrecognised flag in `invalid` and carries on; + # `strict:` is what makes the run able to refuse. Reverting this one word + # restores the silent-ignore. + source = @deploy_task + + assert source =~ "OptionParser.parse(args, strict: @switches, aliases: [d: :device])" + refute source =~ "OptionParser.parse(args, switches: @switches)" + end + + test "the run raises rather than proceeding" do + source = @deploy_task + + assert source =~ + ~r/unless invalid == \[\] do\s*Mix\.raise\(invalid_options_message\(invalid\)\)/ + end + end end