Refuse unknown options in mob.deploy instead of ignoring them - #63
Merged
Conversation
`mix mob.deploy -d <udid>` deployed to every connected device rather than the one named, and said nothing. Two causes, both silent: `-d` was never aliased to `--device` in this task, though `mob.connect` has aliased it all along; and `OptionParser.parse` was called with `switches:`, which drops anything it does not recognise and carries on. A typo'd `--devcie` behaved the same way. Found while verifying MOB-151, where two native builds went to the physical iPhone instead of the simulator I had named, and I spent a while assuming the device auto-detection was at fault. This is the MOB-150 class exactly — a run doing something other than what was asked and reporting success — so it gets the same treatment: `strict:` parsing, the alias added for parity with `mob.connect`, and a `Mix.raise` that names the options rather than a generic usage dump. Device-verified: `--ios -d <sim-udid>` now pushes to one device where it previously pushed to all, and `--devcie foo` exits 1 with "Unknown option(s): --devcie". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
GenericJam added a commit
that referenced
this pull request
Sep 5, 2026
…ger works (#67) A post-hoc review of #63 — which I merged without one — found that switching `mix mob.deploy` to strict parsing broke the documented invocation. `OptionParser` will not consume a dash-prefixed argument as a `:string` value, so `--beam-flags "-S 4:4 -A 4"` parses as two unknown options. BEAM flags essentially all begin with a dash, so this is not an edge case: it is every spelling this repo prints, in seven places — the task's own moduledoc, five README recipes, and the "untuned variant" step of both battery benchmarks. Under the old lenient parsing the value was silently dropped and the deploy carried on with whatever `mob.exs` held, which is its own bug. Strict parsing turned that into a hard abort whose message named `--beam-flags` — a valid option — as unknown. So the fix for one silent failure produced a loud wrong one, and shipped because I skipped the review on a change I had filed mentally as small. `--flag value` is now rewritten to `--flag=value` when the value starts with a dash, for string-valued switches only. Doing it for every switch would swallow the flag after a boolean. Three more from the same review: The error conflated "unrecognised flag" with "recognised flag, unparseable value" and discarded the value, so `--schedulers abc` reported `--schedulers` as unknown and sent the reader to a help page that lists it. `mob.new_plugin` already handled this correctly; this now matches. Positional arguments were still silently dropped. `mix mob.deploy --native ABC123` — a natural fumble of `--device` — parsed cleanly and deployed to every device, which is the identical symptom to the bug #63 set out to fix, in the same task, left in place by it. The four tests #63 shipped were all source-text assertions and none ran the parser, so none could have caught any of this. There is now a table test over every invocation the docs actually print, which fails if a switch is renamed or a documented spelling stops working. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mix mob.deploy -d <udid>deployed to every connected device rather than the one named, and said nothing. Two silent causes:-dwas never aliased to--devicein this task (mob.connecthas aliased it all along), andOptionParser.parsewas called withswitches:, which drops anything unrecognised and carries on. A typo'd--devciebehaved the same way.Found while verifying MOB-151: two native builds went to the physical iPhone instead of the simulator I had named, and I spent a while assuming the device auto-detection was at fault.
Same class as MOB-150 — a run doing something other than what was asked and reporting success — so it gets the same treatment:
strict:parsing, the alias added for parity withmob.connect, and aMix.raisenaming the offending options rather than a generic usage dump.Device-verified:
--ios -d <sim-udid>now pushes to one device where it previously pushed to all;--devcie fooexits 1 withUnknown option(s): --devcie.2256 tests, credo and format clean.