Skip to content

Commit

Permalink
Fix error-prone shell command status check
Browse files Browse the repository at this point in the history
In the case of the tests, `$?` was not mocked, so it looks like it was
indeed just a coincidence that it worked. Meaning, some other shell
command that somehow was actually executed (so it wasn't stubbed)
succeeded, which caused the `$?.success?` to succeed too.
  • Loading branch information
revolter committed Nov 3, 2022
1 parent 1a27157 commit 0c2dddf
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
4 changes: 2 additions & 2 deletions fastlane_core/lib/fastlane_core/cert_checker.rb
Expand Up @@ -160,13 +160,13 @@ def self.install_wwdr_certificate(cert_alias)
import_command = "curl -f -o #{filename} #{url} && security import #{filename} #{keychain}"
UI.verbose("Installing WWDR Cert: #{import_command}")

stdout, stderr, _status = Open3.capture3(import_command)
stdout, stderr, status = Open3.capture3(import_command)
if FastlaneCore::Globals.verbose?
UI.command_output(stdout)
UI.command_output(stderr)
end

unless $?.success?
unless status.success?
UI.verbose("Failed to install WWDR Certificate, checking output to see why")
# Check the command output, WWDR might already exist
unless /The specified item already exists in the keychain./ =~ stderr
Expand Down
20 changes: 13 additions & 7 deletions fastlane_core/spec/cert_checker_spec.rb
@@ -1,5 +1,11 @@
describe FastlaneCore do
describe FastlaneCore::CertChecker do
let(:success_status) {
allow_any_instance_of(String).to receive(:success?).and_return(true)

""
}

describe '#installed_identies' do
it 'should print an error when no local code signing identities are found' do
allow(FastlaneCore::CertChecker).to receive(:installed_wwdr_certificates).and_return(['G1', 'G2', 'G3', 'G4', 'G5', 'G6'])
Expand Down Expand Up @@ -65,22 +71,22 @@
it 'should download the WWDR certificate from correct URL' do
allow(FastlaneCore::CertChecker).to receive(:wwdr_keychain).and_return('login.keychain')

expect(Open3).to receive(:capture3).with(include('https://developer.apple.com/certificationauthority/AppleWWDRCA.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://developer.apple.com/certificationauthority/AppleWWDRCA.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G1')

expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG2.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG2.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G2')

expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG3.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG3.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G3')

expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG4.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG4.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G4')

expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG5.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG5.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G5')

expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG6.cer')).and_return("")
expect(Open3).to receive(:capture3).with(include('https://www.apple.com/certificateauthority/AppleWWDRCAG6.cer')).and_return(["", "", success_status])
FastlaneCore::CertChecker.install_wwdr_certificate('G6')
end
end
Expand All @@ -105,7 +111,7 @@
cmd = %r{curl -f -o (([A-Z]\:)?\/.+) https://www\.apple\.com/certificateauthority/AppleWWDRCAG6\.cer && security import \1 -k #{Regexp.escape(keychain.shellescape)}}
require "open3"

expect(Open3).to receive(:capture3).with(cmd).and_return("")
expect(Open3).to receive(:capture3).with(cmd).and_return(["", "", success_status])
expect(FastlaneCore::CertChecker).to receive(:wwdr_keychain).and_return(keychain_name)

allow(FastlaneCore::CertChecker).to receive(:installed_wwdr_certificates).and_return(['G1', 'G2', 'G3', 'G4', 'G5'])
Expand Down

1 comment on commit 0c2dddf

@revolter
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joshdholtz, as you have a lot more experience with both Ruby and fastlane, could you please take a look at this change, in case you notice something wrong?

Please sign in to comment.