Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pilot][upload_to_testflight] add Mac support #19296

Merged
merged 2 commits into from Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion fastlane/lib/fastlane/actions/upload_to_testflight.rb
Expand Up @@ -13,6 +13,8 @@ def self.run(values)
unless distribute_only
values[:ipa] ||= Actions.lane_context[SharedValues::IPA_OUTPUT_PATH]
values[:ipa] = File.expand_path(values[:ipa]) if values[:ipa]
values[:pkg] ||= Actions.lane_context[SharedValues::PKG_OUTPUT_PATH]
values[:pkg] = File.expand_path(values[:pkg]) if values[:pkg]
end

# Only set :api_key from SharedValues if :api_key_path isn't set (conflicting options)
Expand Down Expand Up @@ -117,7 +119,7 @@ def self.authors
end

def self.is_supported?(platform)
[:ios].include?(platform)
[:ios, :mac, :tvos].include?(platform)
end
end
end
Expand Down
31 changes: 25 additions & 6 deletions fastlane_core/lib/fastlane_core/build_watcher.rb
Expand Up @@ -11,7 +11,7 @@ class BuildWatcher

class << self
# @return The build we waited for. This method will always return a build
def wait_for_build_processing_to_be_complete(app_id: nil, platform: nil, train_version: nil, app_version: nil, build_version: nil, poll_interval: 10, timeout_duration: nil, strict_build_watch: false, return_when_build_appears: false, return_spaceship_testflight_build: true, select_latest: false)
def wait_for_build_processing_to_be_complete(app_id: nil, platform: nil, train_version: nil, app_version: nil, build_version: nil, poll_interval: 10, timeout_duration: nil, strict_build_watch: false, return_when_build_appears: false, return_spaceship_testflight_build: true, select_latest: false, wait_for_build_beta_detail_processing: false)
# Warn about train_version being removed in the future
if train_version
UI.deprecated(":train_version is no longer a used argument on FastlaneCore::BuildWatcher. Please use :app_version instead.")
Expand Down Expand Up @@ -41,13 +41,13 @@ def wait_for_build_processing_to_be_complete(app_id: nil, platform: nil, train_v
showed_info = true
end

report_status(build: matched_build)
report_status(build: matched_build, wait_for_build_beta_detail_processing: wait_for_build_beta_detail_processing)

# Processing of builds by AppStoreConnect can be a very time consuming task and will
# block the worker running this task until it is completed. In some cases,
# having a build resource appear in AppStoreConnect (matched_build) may be enough (i.e. setting a changelog)
# so here we may choose to skip the full processing of the build if return_when_build_appears is true
if matched_build && (return_when_build_appears || matched_build.processed?)
if matched_build && (return_when_build_appears || processed?(build: matched_build, wait_for_build_beta_detail_processing: wait_for_build_beta_detail_processing))

if !app_version.nil? && app_version != app_version_queried
UI.important("App version is #{app_version} but build was found while querying #{app_version_queried}")
Expand Down Expand Up @@ -145,10 +145,29 @@ def alternate_version(version)
return nil
end

def report_status(build: nil)
if build && !build.processed?
def processed?(build: nil, wait_for_build_beta_detail_processing: false)
return false unless build

is_processed = build.processed?

# App Store Connect API has multiple build processing states
# builds have one processing status
# buildBetaDetails have two processing statues (internal and external testing)
#
# If set, this method will only return true if all three statuses are complete
if wait_for_build_beta_detail_processing
is_processed &&= build.build_beta_detail.processed?
end

return is_processed
end

def report_status(build: nil, wait_for_build_beta_detail_processing: false)
is_processed = processed?(build: build, wait_for_build_beta_detail_processing: wait_for_build_beta_detail_processing)

if build && !is_processed
UI.message("Waiting for App Store Connect to finish processing the new build (#{build.app_version} - #{build.version}) for #{build.platform}")
elsif build && build.processed?
elsif build && is_processed
UI.success("Successfully finished processing the build #{build.app_version} - #{build.version} for #{build.platform}")
else
UI.message("Waiting for the build to show up in the build list - this may take a few minutes (check your email for processing issues if this continues)")
Expand Down
17 changes: 14 additions & 3 deletions pilot/lib/pilot/build_manager.rb
Expand Up @@ -16,7 +16,7 @@ def upload(options)
should_login_in_start = options[:apple_id].nil?
start(options, should_login: should_login_in_start)

UI.user_error!("No ipa file given") unless config[:ipa]
UI.user_error!("No ipa or pkg file given") if config[:ipa].nil? && config[:pkg].nil?

check_for_changelog_or_whats_new!(options)

Expand All @@ -25,10 +25,17 @@ def upload(options)
dir = Dir.mktmpdir

platform = fetch_app_platform
package_path = FastlaneCore::IpaUploadPackageBuilder.new.generate(app_id: fetch_app_id,
if options[:ipa]
package_path = FastlaneCore::IpaUploadPackageBuilder.new.generate(app_id: fetch_app_id,
ipa_path: options[:ipa],
package_path: dir,
platform: platform)
else
package_path = FastlaneCore::PkgUploadPackageBuilder.new.generate(app_id: fetch_app_id,
pkg_path: options[:pkg],
package_path: dir,
platform: platform)
end

transporter = transporter_for_selected_team(options)
result = transporter.upload(package_path: package_path)
Expand Down Expand Up @@ -94,6 +101,9 @@ def wait_for_build_processing_to_be_complete(return_when_build_appears = false)
if config[:ipa]
app_version = FastlaneCore::IpaFileAnalyser.fetch_app_version(config[:ipa])
app_build = FastlaneCore::IpaFileAnalyser.fetch_app_build(config[:ipa])
elsif config[:pkg]
app_version = FastlaneCore::PkgFileAnalyser.fetch_app_version(config[:pkg])
app_build = FastlaneCore::PkgFileAnalyser.fetch_app_build(config[:pkg])
else
app_version = config[:app_version]
app_build = config[:build_number]
Expand All @@ -108,7 +118,8 @@ def wait_for_build_processing_to_be_complete(return_when_build_appears = false)
timeout_duration: config[:wait_processing_timeout_duration],
return_when_build_appears: return_when_build_appears,
return_spaceship_testflight_build: false,
select_latest: config[:distribute_only]
select_latest: config[:distribute_only],
wait_for_build_beta_detail_processing: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildWatcher is used for build uploads or in submit_for_review.
Could you explain why when return_when_build_appears = false we want to always wait for buildBetaDetails processing when we upload builds but when wesubmit_for_review we skip waiting for buildBetaDetails processing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucgrabowski By submit_for_review you mean using deliver, right? A not pilot? buildBetaDetails includes flags for "internal testing state" and "external testing state". Processing the binary for App Store submittal and processing the binary for testing distribution are apparently two different things in App Store Connect

Since pilot deals with testing, the App Store Connect API will give an error if the internal and external testing state are still "processing". Submitting a binary for review does not care about the testing state so these don't need to be watched. IDEALLY they should finish at the same time but they aren't designed to.

Hope that helps 😊

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshdholtz Many thanks for explanation 🚀
That really makes sense :)

)

unless latest_build.app_version == app_version && latest_build.version == app_build
Expand Down
4 changes: 3 additions & 1 deletion pilot/lib/pilot/manager.rb
Expand Up @@ -73,7 +73,8 @@ def fetch_app_id

def fetch_app_identifier
result = config[:app_identifier]
result ||= FastlaneCore::IpaFileAnalyser.fetch_app_identifier(config[:ipa])
result ||= FastlaneCore::IpaFileAnalyser.fetch_app_identifier(config[:ipa]) if config[:ipa]
result ||= FastlaneCore::PkgFileAnalyser.fetch_app_identifier(config[:pkg]) if config[:pkg]
result ||= UI.input("Please enter the app's bundle identifier: ")
UI.verbose("App identifier (#{result})")
return result
Expand All @@ -82,6 +83,7 @@ def fetch_app_identifier
def fetch_app_platform(required: true)
result = config[:app_platform]
result ||= FastlaneCore::IpaFileAnalyser.fetch_app_platform(config[:ipa]) if config[:ipa]
result ||= FastlaneCore::PkgFileAnalyser.fetch_app_platform(config[:pkg]) if config[:pkg]
if required
result ||= UI.input("Please enter the app's platform (appletvos, ios, osx): ")
UI.user_error!("App Platform must be ios, appletvos, or osx") unless ['ios', 'appletvos', 'osx'].include?(result)
Expand Down
21 changes: 20 additions & 1 deletion pilot/lib/pilot/options.rb
Expand Up @@ -48,7 +48,6 @@ def self.available_options
env_name: "PILOT_PLATFORM",
description: "The platform to use (optional)",
optional: true,
default_value: 'ios',
verify_block: proc do |value|
UI.user_error!("The platform can only be ios, appletvos, or osx") unless ['ios', 'appletvos', 'osx'].include?(value)
end),
Expand Down Expand Up @@ -82,6 +81,26 @@ def self.available_options
value = File.expand_path(value)
UI.user_error!("Could not find ipa file at path '#{value}'") unless File.exist?(value)
UI.user_error!("'#{value}' doesn't seem to be an ipa file") unless value.end_with?(".ipa")
end,
conflicting_options: [:pkg],
conflict_block: proc do |value|
UI.user_error!("You can't use 'ipa' and '#{value.key}' options in one run.")
end),
FastlaneCore::ConfigItem.new(key: :pkg,
short_option: "-P",
optional: true,
env_name: "PILOT_PKG",
description: "Path to your pkg file",
code_gen_sensitive: true,
default_value: Dir["*.pkg"].sort_by { |x| File.mtime(x) }.last,
default_value_dynamic: true,
verify_block: proc do |value|
UI.user_error!("Could not find pkg file at path '#{File.expand_path(value)}'") unless File.exist?(value)
UI.user_error!("'#{value}' doesn't seem to be a pkg file") unless value.end_with?(".pkg")
end,
conflicting_options: [:ipa],
conflict_block: proc do |value|
UI.user_error!("You can't use 'pkg' and '#{value.key}' options in one run.")
end),

# app review info
Expand Down
173 changes: 131 additions & 42 deletions pilot/spec/manager_spec.rb
Expand Up @@ -13,6 +13,7 @@
let(:fake_app) { "fake app" }
let(:fake_app_identifier) { "fake app_identifier" }
let(:fake_ipa) { "fake ipa" }
let(:fake_pkg) { "fake pkg" }

describe "what happens on 'start'" do
context "when 'config' variable is already set" do
Expand Down Expand Up @@ -392,6 +393,23 @@
end
end

context "when config does not has 'app_identifier' but has 'pkg' path variable" do
before(:each) do
fake_manager.instance_variable_set(:@config, { pkg: fake_pkg })

allow(FastlaneCore::PkgFileAnalyser)
.to receive(:fetch_app_identifier)
.with(fake_pkg)
.and_return(fake_app_identifier)
end

it "uses the FastlaneCore::PkgFileAnalyser with 'pkg' path to find the 'app_identifier' value" do
fetch_app_identifier_result = fake_manager.fetch_app_identifier

expect(fetch_app_identifier_result).to eq(fake_app_identifier)
end
end

context "when FastlaneCore::IpaFileAnalyser failed to fetch the 'app_identifier' variable" do
before(:each) do
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })
Expand All @@ -413,76 +431,147 @@
end

describe "what happens on fetching the 'app_platform'" do
let(:fake_app_platform) { "ios" }

context "when config has 'app_platform' variable" do
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { app_platform: fake_app_platform })
context "ios" do
let(:fake_app_platform) { "ios" }

before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { app_platform: fake_app_platform })
end

it "uses the config 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end

it "uses the config 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform
context "osx" do
let(:fake_app_platform) { "osx" }

before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { app_platform: fake_app_platform })
end

it "uses the config 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end
end

context "when config does not has 'app_platform' but has 'ipa' path variable" do
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })
context "when config does not have 'app_platform'" do
context "but has 'ipa' path variable" do
let(:fake_app_platform) { "ios" }

allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(fake_app_platform)
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })

allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(fake_app_platform)
end

it "uses the FastlaneCore::IpaFileAnalyser with 'ipa' path to find the 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end

it "uses the FastlaneCore::IpaFileAnalyser with 'ipa' path to find the 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform
context "but has 'pkg' path variable" do
let(:fake_app_platform) { "osx" }

before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { pkg: fake_pkg })

expect(fetch_app_platform_result).to eq(fake_app_platform)
allow(FastlaneCore::PkgFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_pkg)
.and_return(fake_app_platform)
end

it "uses the FastlaneCore::PkgFileAnalyser with 'pkg' path to find the 'app_platform' value" do
fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end
end

context "when FastlaneCore::IpaFileAnalyser failed to fetch the 'app_platform' variable" do
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })
context "ios" do
let(:fake_app_platform) { "ios" }

allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(nil)
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })

allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(nil)
end

it "asks user to enter the app's platform manually" do
expect(UI).to receive(:input).with("Please enter the app's platform (appletvos, ios, osx): ").and_return(fake_app_platform)

fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end

it "asks user to enter the app's platform manually" do
expect(UI).to receive(:input).with("Please enter the app's platform (appletvos, ios, osx): ").and_return(fake_app_platform)
context "osx" do
let(:fake_app_platform) { "osx" }

fetch_app_platform_result = fake_manager.fetch_app_platform
before(:each) do
expect(UI).to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { pkg: fake_pkg })

expect(fetch_app_platform_result).to eq(fake_app_platform)
allow(FastlaneCore::PkgFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_pkg)
.and_return(nil)
end

it "asks user to enter the app's platform manually" do
expect(UI).to receive(:input).with("Please enter the app's platform (appletvos, ios, osx): ").and_return(fake_app_platform)

fetch_app_platform_result = fake_manager.fetch_app_platform

expect(fetch_app_platform_result).to eq(fake_app_platform)
end
end
end

context "when FastlaneCore::IpaFileAnalyser failed to fetch the 'app_platform' variable and its not required to enter manually" do
before(:each) do
expect(UI).not_to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })
context "ios" do
let(:fake_app_platform) { "ios" }

allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(nil)
end
before(:each) do
expect(UI).not_to receive(:verbose).with("App Platform (#{fake_app_platform})")
fake_manager.instance_variable_set(:@config, { ipa: fake_ipa })

it "does not ask user to enter the app's platform manually" do
expect(UI).not_to receive(:input).with("Please enter the app's platform (appletvos, ios, osx): ")
allow(FastlaneCore::IpaFileAnalyser)
.to receive(:fetch_app_platform)
.with(fake_ipa)
.and_return(nil)
end

fetch_app_platform_result = fake_manager.fetch_app_platform(required: false)
it "does not ask user to enter the app's platform manually" do
expect(UI).not_to receive(:input).with("Please enter the app's platform (appletvos, ios, osx): ")

expect(fetch_app_platform_result).to eq(nil)
fetch_app_platform_result = fake_manager.fetch_app_platform(required: false)

expect(fetch_app_platform_result).to eq(nil)
end
end
end

Expand Down