[pilot] Do not use spaceship if both skip_waiting_for_build_processing and apple_id are set (fastlane#14456)

* make pilot not use spaceship if both skip_waiting_for_build_processing and apple_id are set

* make rubocop happy

* use options instead of config (assumption: they are actually identical)

* still have to fill @config somehow when start is not called (UGLY!)

* prepare test a bit

* 2 tests that make sure that the spaceship login method is only called when the 2 options are _not_ set

* make rubocop happy

* add missing options param

* add missing options param

* add some docs

* A few small improvements

* rename and clarify specs

* add explicit expect and !expect
clean and resort allows

* rubocop

* correct order

* Removed the login_if_needed method and replaced with better code and comments


Co-authored-by: Josh Holtz <josh@rokkincat.com>

Author: janpio

fastlane/lib/fastlane/actions/docs/upload_to_testflight.md

@@ -225,3 +225,7 @@ _pilot_ uses the [CredentialsManager](https://github.com/fastlane/fastlane/tree/
 
 ## Provider Short Name
 If you are on multiple App Store Connect teams, iTunes Transporter may need a provider short name to know where to upload your binary. _pilot_ will try to use the long name of the selected team to detect the provider short name. To override the detected value with an explicit one, use the `itc_provider` option.
+
+## Use an Application Specific Password to upload
+
+_pilot_/`upload_to_testflight` can use an [Application Specific Password via the `FASTLANE_APPLE_APPLICATION_SPECIFIC_PASSWORD` envirionment variable](https://docs.fastlane.tools/best-practices/continuous-integration/#application-specific-passwords) to upload a binary if both the `skip_waiting_for_build_processing` and `apple_id` options are set. (If any of those are not set, it will use the normal Apple login process that might require 2FA authentication.)

pilot/lib/pilot/build_manager.rb

@@ -10,7 +10,10 @@
 module Pilot
   class BuildManager < Manager
     def upload(options)
-      start(options)
+      # Only need to login before upload if no apple_id was given
+      # 'login' will be deferred until before waiting for build processing
+      should_login_in_start = options[:apple_id].nil?
+      start(options, should_login: should_login_in_start)
 
       options[:changelog] = self.class.sanitize_changelog(options[:changelog]) if options[:changelog]
 
@@ -24,18 +27,18 @@ def upload(options)
         end
       end
 
-      UI.success("Ready to upload new build to TestFlight (App: #{app.apple_id})...")
+      UI.success("Ready to upload new build to TestFlight (App: #{fetch_apple_id})...")
 
       dir = Dir.mktmpdir
 
       platform = fetch_app_platform
-      package_path = FastlaneCore::IpaUploadPackageBuilder.new.generate(app_id: app.apple_id,
-                                                                      ipa_path: config[:ipa],
+      package_path = FastlaneCore::IpaUploadPackageBuilder.new.generate(app_id: fetch_apple_id,
+                                                                      ipa_path: options[:ipa],
                                                                   package_path: dir,
                                                                       platform: platform)
 
       transporter = transporter_for_selected_team(options)
-      result = transporter.upload(app.apple_id, package_path)
+      result = transporter.upload(fetch_apple_id, package_path)
 
       unless result
         UI.user_error!("Error uploading ipa file, for more information see above")
@@ -49,6 +52,9 @@ def upload(options)
         return
       end
 
+      # Calling login again here is needed if login was not called during 'start'
+      login unless should_login_in_start
+
       UI.message("If you want to skip waiting for the processing to be finished, use the `skip_waiting_for_build_processing` option")
       latest_build = wait_for_build_processing_to_be_complete
       distribute(options, build: latest_build)
@@ -58,7 +64,7 @@ def wait_for_build_processing_to_be_complete
       platform = fetch_app_platform
       app_version = FastlaneCore::IpaFileAnalyser.fetch_app_version(config[:ipa])
       app_build = FastlaneCore::IpaFileAnalyser.fetch_app_build(config[:ipa])
-      latest_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: app.apple_id, platform: platform, train_version: app_version, build_version: app_build, poll_interval: config[:wait_processing_interval])
+      latest_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: app.apple_id, platform: platform, train_version: app_version, build_version: app_build, poll_interval: config[:wait_processing_interval], strict_build_watch: config[:wait_for_uploaded_build])
 
       unless latest_build.train_version == app_version && latest_build.build_version == app_build
         UI.important("Uploaded app #{app_version} - #{app_build}, but received build #{latest_build.train_version} - #{latest_build.build_version}.")
@@ -220,7 +226,8 @@ def should_update_localized_build_information?(options)
     # If there are fewer than two teams, don't infer the provider.
     def transporter_for_selected_team(options)
       generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider])
-      return generic_transporter unless options[:itc_provider].nil? && Spaceship::Tunes.client.teams.count > 1
+      return generic_transporter if options[:itc_provider] || Spaceship::Tunes.client.nil?
+      return generic_transporter unless Spaceship::Tunes.client.teams.count > 1
 
       begin
         team = Spaceship::Tunes.client.teams.find { |t| t['contentProvider']['contentProviderId'].to_s == Spaceship::Tunes.client.team_id }

pilot/lib/pilot/manager.rb

@@ -10,10 +10,10 @@
 
 module Pilot
   class Manager
-    def start(options)
+    def start(options, should_login: true)
       return if @config # to not login multiple times
       @config = options
-      login
+      login if should_login
     end
 
     def login
@@ -27,7 +27,7 @@ def login
 
     # The app object we're currently using
     def app
-      @apple_id ||= fetch_app_id
+      @apple_id ||= fetch_apple_id
 
       @app ||= Spaceship::Tunes::Application.find(@apple_id)
       unless @app
@@ -42,19 +42,20 @@ def app
     # Config Related
     ################
 
-    def fetch_app_id
+    def fetch_apple_id
+      @apple_id ||= config[:apple_id]
       return @apple_id if @apple_id
       config[:app_identifier] = fetch_app_identifier
 
       if config[:app_identifier]
         @app ||= Spaceship::Tunes::Application.find(config[:app_identifier])
         UI.user_error!("Couldn't find app '#{config[:app_identifier]}' on the account of '#{config[:username]}' on App Store Connect") unless @app
-        app_id ||= @app.apple_id
+        apple_id ||= @app.apple_id
       end
 
-      app_id ||= UI.input("Could not automatically find the app ID, please enter it here (e.g. 956814360): ")
+      apple_id ||= UI.input("Could not automatically find the app ID, please enter it here (e.g. 956814360): ")
 
-      return app_id
+      return apple_id
     end
 
     def fetch_app_identifier

pilot/spec/build_manager_spec.rb

@@ -11,6 +11,7 @@
       expect(changelog).to eq("1234")
     end
   end
+
   describe ".sanitize_changelog" do
     it "removes emoji" do
       changelog = "I'm 🦇B🏧an!"
@@ -24,7 +25,8 @@
       expect(changelog).to eq(File.read("./pilot/spec/fixtures/build_manager/changelog_long_truncated"))
     end
   end
-  describe "distribute submits the build for review" do
+
+  describe "#distribute submits the build for review" do
     let(:mock_base_client) { "fake testflight base client" }
     let(:mock_base_api_client) { "fake api base client" }
     let(:fake_build_manager) { Pilot::BuildManager.new }
@@ -240,4 +242,65 @@
       end
     end
   end
+
+  describe "#upload" do
+    describe "uses Manager.login (which does spaceship login)" do
+      let(:fake_build_manager) { Pilot::BuildManager.new }
+      let(:upload_options) do
+        {
+          apple_id: 'mock_apple_id',
+          skip_waiting_for_build_processing: true,
+          ipa: 'foo'
+        }
+      end
+
+      before(:each) do
+        allow(fake_build_manager).to receive(:fetch_app_platform).and_return('ios')
+
+        fake_ipauploadpackagebuilder = double
+        allow(fake_ipauploadpackagebuilder).to receive(:generate).and_return(true)
+        allow(FastlaneCore::IpaUploadPackageBuilder).to receive(:new).and_return(fake_ipauploadpackagebuilder)
+
+        fake_itunestransporter = double
+        allow(fake_itunestransporter).to receive(:upload).and_return(true)
+        allow(FastlaneCore::ItunesTransporter).to receive(:new).and_return(fake_itunestransporter)
+      end
+
+      it "NOT when skip_waiting_for_build_processing and apple_id are set" do
+        # should not execute Manager.login (which does spaceship login)
+        expect(fake_build_manager).not_to(receive(:login))
+
+        fake_build_manager.upload(upload_options)
+      end
+
+      it "when skip_waiting_for_build_processing and apple_id are not set" do
+        # remove options that make login unnecessary
+        upload_options.delete(:apple_id)
+        upload_options.delete(:skip_waiting_for_build_processing)
+
+        # allow Manager.login method this time
+        expect(fake_build_manager).to receive(:login).at_least(:once)
+
+        # other stuff required to let `upload` work:
+
+        allow(FastlaneCore::IpaFileAnalyser).to receive(:fetch_app_identifier).and_return("com.fastlane")
+        allow(fake_build_manager).to receive(:fetch_apple_id).and_return(123)
+        allow(FastlaneCore::IpaFileAnalyser).to receive(:fetch_app_version)
+        allow(FastlaneCore::IpaFileAnalyser).to receive(:fetch_app_build)
+
+        fake_app = double
+        allow(fake_app).to receive(:apple_id).and_return(123)
+        allow(fake_build_manager).to receive(:app).and_return(fake_app)
+
+        fake_build = double
+        allow(fake_build).to receive(:train_version)
+        allow(fake_build).to receive(:build_version)
+        allow(FastlaneCore::BuildWatcher).to receive(:wait_for_build_processing_to_be_complete).and_return(fake_build)
+
+        allow(fake_build_manager).to receive(:distribute)
+
+        fake_build_manager.upload(upload_options)
+      end
+    end
+  end
 end