From 0c2dddf5d6416430bef9d6a8be9e3a6010d5f491 Mon Sep 17 00:00:00 2001 From: Iulian Onofrei Date: Thu, 3 Nov 2022 17:04:18 +0200 Subject: [PATCH] Fix error-prone shell command status check 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. --- .../lib/fastlane_core/cert_checker.rb | 4 ++-- fastlane_core/spec/cert_checker_spec.rb | 20 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/fastlane_core/lib/fastlane_core/cert_checker.rb b/fastlane_core/lib/fastlane_core/cert_checker.rb index 0af9394d64f..bb78e8ee8cf 100644 --- a/fastlane_core/lib/fastlane_core/cert_checker.rb +++ b/fastlane_core/lib/fastlane_core/cert_checker.rb @@ -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 diff --git a/fastlane_core/spec/cert_checker_spec.rb b/fastlane_core/spec/cert_checker_spec.rb index 70564e3d3e1..864c116f605 100644 --- a/fastlane_core/spec/cert_checker_spec.rb +++ b/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']) @@ -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 @@ -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'])