Refactor pilot CommanderGenerator usage (#8287)

Carry the work and lessons learned from #8129
over to pilot

pilot does not currently have any option
conflicts, but this refactoring is still generally
more correct and safe.

pilot/lib/pilot/commands_generator.rb

@@ -8,8 +8,6 @@ module Pilot
   class CommandsGenerator
     include Commander::Methods
 
-    FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options)
-
     def self.start
       new.run
     end
@@ -53,6 +51,9 @@ def run
       command :upload do |c|
         c.syntax = "fastlane pilot upload"
         c.description = "Uploads a new binary to Apple TestFlight"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::BuildManager.new.upload(config)
@@ -62,6 +63,9 @@ def run
       command :distribute do |c|
         c.syntax = "fastlane pilot distribute"
         c.description = "Distribute a previously uploaded binary to Apple TestFlight"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::BuildManager.new.distribute(config)
@@ -71,6 +75,9 @@ def run
       command :builds do |c|
         c.syntax = "fastlane pilot builds"
         c.description = "Lists all builds for given application"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::BuildManager.new.list(config)
@@ -80,6 +87,9 @@ def run
       command :add do |c|
         c.syntax = "fastlane pilot add"
         c.description = "Adds new external tester(s) to a specific app (if given). This will also add an existing tester to an app."
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           handle_multiple('add_tester', args, options)
         end
@@ -88,6 +98,9 @@ def run
       command :list do |c|
         c.syntax = "fastlane pilot list"
         c.description = "Lists all registered testers, both internal and external"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::TesterManager.new.list_testers(config)
@@ -97,6 +110,9 @@ def run
       command :find do |c|
         c.syntax = "fastlane pilot find"
         c.description = "Find tester(s) (internal or external) by their email address"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           handle_multiple('find_tester', args, options)
         end
@@ -105,6 +121,9 @@ def run
       command :remove do |c|
         c.syntax = "fastlane pilot remove"
         c.description = "Remove external tester(s) by their email address"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           handle_multiple('remove_tester', args, options)
         end
@@ -113,6 +132,9 @@ def run
       command :export do |c|
         c.syntax = "fastlane pilot export"
         c.description = "Exports all external testers to a CSV file"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::TesterExporter.new.export_testers(config)
@@ -122,6 +144,9 @@ def run
       command :import do |c|
         c.syntax = "fastlane pilot import"
         c.description = "Create external testers from a CSV file"
+
+        FastlaneCore::CommanderGenerator.new.generate(Pilot::Options.available_options, command: c)
+
         c.action do |args, options|
           config = FastlaneCore::Configuration.create(Pilot::Options.available_options, convert_options(options))
           Pilot::TesterImporter.new.import_testers(config)

pilot/spec/commands_generator_spec.rb

@@ -0,0 +1,163 @@
+require 'pilot/commands_generator'
+
+describe Pilot::CommandsGenerator do
+  let(:available_options) { Pilot::Options.available_options }
+
+  def expect_build_manager_call_with(method_sym, expected_options)
+    fake_build_manager = "build_manager"
+    expect(Pilot::BuildManager).to receive(:new).and_return(fake_build_manager)
+    expect(fake_build_manager).to receive(method_sym) do |actual_options|
+      expect(actual_options._values).to eq(expected_options._values)
+    end
+  end
+
+  {
+    upload: :upload,
+    distribute: :distribute,
+    builds: :list
+  }.each do |command, build_manager_method|
+    describe ":#{command} option handling" do
+      it "can use the username short flag from tool options" do
+        stub_commander_runner_args([command.to_s, '-u', 'me@it.com'])
+
+        expected_options = FastlaneCore::Configuration.create(available_options, { username: 'me@it.com' })
+        expect_build_manager_call_with(build_manager_method, expected_options)
+
+        Pilot::CommandsGenerator.start
+      end
+
+      it "can use the app_identifier flag from tool options" do
+        stub_commander_runner_args([command.to_s, '--app_identifier', 'your.awesome.App'])
+
+        expected_options = FastlaneCore::Configuration.create(available_options, { app_identifier: 'your.awesome.App' })
+        expect_build_manager_call_with(build_manager_method, expected_options)
+
+        Pilot::CommandsGenerator.start
+      end
+    end
+  end
+
+  def expect_tester_manager_call_with(method_sym, expected_options_sets)
+    expected_options_sets = [expected_options_sets] unless expected_options_sets.kind_of? Array
+
+    fake_tester_manager = "tester_manager"
+    expect(Pilot::TesterManager).to receive(:new).and_return(fake_tester_manager)
+    expected_options_sets.each do |expected_options|
+      expect(fake_tester_manager).to receive(method_sym) do |actual_options|
+        expect(actual_options._values).to eq(expected_options._values)
+      end
+    end
+  end
+
+  describe ":list option handling" do
+    it "can use the username short flag from tool options" do
+      stub_commander_runner_args(['list', '-u', 'me@it.com'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { username: 'me@it.com' })
+      expect_tester_manager_call_with(:list_testers, expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+
+    it "can use the app_identifier flag from tool options" do
+      stub_commander_runner_args(['list', '--app_identifier', 'your.awesome.App'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { app_identifier: 'your.awesome.App' })
+      expect_tester_manager_call_with(:list_testers, expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+  end
+
+  {
+    add: :add_tester,
+    remove: :remove_tester,
+    find: :find_tester
+  }.each do |command, tester_manager_method|
+    describe ":#{command} option handling" do
+      it "can use the email short flag from tool options" do
+        stub_commander_runner_args([command.to_s, '-e', 'you@that.com'])
+
+        expected_options = FastlaneCore::Configuration.create(available_options, { email: 'you@that.com' })
+        expect_tester_manager_call_with(tester_manager_method, expected_options)
+
+        Pilot::CommandsGenerator.start
+      end
+
+      it "can use the email flag from tool options" do
+        stub_commander_runner_args([command.to_s, '--email', 'you@that.com'])
+
+        expected_options = FastlaneCore::Configuration.create(available_options, { email: 'you@that.com' })
+        expect_tester_manager_call_with(tester_manager_method, expected_options)
+
+        Pilot::CommandsGenerator.start
+      end
+
+      it "can provide multiple emails as args" do
+        stub_commander_runner_args([command.to_s, 'you@that.com', 'another@that.com'])
+
+        expected_options1 = FastlaneCore::Configuration.create(available_options, { email: 'you@that.com' })
+        expected_options2 = FastlaneCore::Configuration.create(available_options, { email: 'another@that.com' })
+        expect_tester_manager_call_with(tester_manager_method, [expected_options1, expected_options2])
+
+        Pilot::CommandsGenerator.start
+      end
+    end
+  end
+
+  describe ":export option handling" do
+    def expect_tester_exporter_export_testers_with(expected_options)
+      fake_tester_exporter = "tester_exporter"
+      expect(Pilot::TesterExporter).to receive(:new).and_return(fake_tester_exporter)
+      expect(fake_tester_exporter).to receive(:export_testers) do |actual_options|
+        expect(actual_options._values).to eq(expected_options._values)
+      end
+    end
+
+    it "can use the testers_file_path short flag from tool options" do
+      stub_commander_runner_args(['export', '-c', 'file/path'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { testers_file_path: 'file/path' })
+      expect_tester_exporter_export_testers_with(expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+
+    it "can use the app_identifier flag from tool options" do
+      stub_commander_runner_args(['export', '--app_identifier', 'your.awesome.App'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { app_identifier: 'your.awesome.App' })
+      expect_tester_exporter_export_testers_with(expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+  end
+
+  describe ":import option handling" do
+    def expect_tester_importer_import_testers_with(expected_options)
+      fake_tester_importer = "tester_importer"
+      expect(Pilot::TesterImporter).to receive(:new).and_return(fake_tester_importer)
+      expect(fake_tester_importer).to receive(:import_testers) do |actual_options|
+        expect(actual_options._values).to eq(expected_options._values)
+      end
+    end
+
+    it "can use the testers_file_path short flag from tool options" do
+      stub_commander_runner_args(['import', '-c', 'file/path'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { testers_file_path: 'file/path' })
+      expect_tester_importer_import_testers_with(expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+
+    it "can use the app_identifier flag from tool options" do
+      stub_commander_runner_args(['import', '--app_identifier', 'your.awesome.App'])
+
+      expected_options = FastlaneCore::Configuration.create(available_options, { app_identifier: 'your.awesome.App' })
+      expect_tester_importer_import_testers_with(expected_options)
+
+      Pilot::CommandsGenerator.start
+    end
+  end
+end