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

Honor test spec deployment target during validation. #9999

Merged
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
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Expand Up @@ -237,7 +237,7 @@ Lint/UselessAccessModifier:
# Offense count: 373
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/BlockLength:
Max: 1445
Max: 1476

# Offense count: 7099
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,10 @@ To install release candidates run `[sudo] gem install cocoapods --pre`

##### Bug Fixes

* Honor test spec deployment target during validation.
[Dimitris Koutsogiorgas](https://github.com/dnkoutso)
[#9999](https://github.com/CocoaPods/CocoaPods/pull/9999)

* Ensure that incremental installation is able to set target dependencies for a
test spec that uses a custom `app_host_name` that is in a project that is not
regenerated.
Expand Down
10 changes: 6 additions & 4 deletions lib/cocoapods/validator.rb
Expand Up @@ -556,6 +556,8 @@ def clean!
validation_dir.rmtree
end

# @return [String] The deployment targret of the library spec.
#
def deployment_target
deployment_target = spec.subspec_by_name(subspec_name).deployment_target(consumer.platform_name)
if consumer.platform_name == :ios && use_frameworks
Expand Down Expand Up @@ -711,7 +713,7 @@ def build_pod
UI.warn "Skipping compilation with `xcodebuild` because target contains no sources.\n".yellow
else
if analyze
output = xcodebuild('analyze', scheme, 'Release')
output = xcodebuild('analyze', scheme, 'Release', :deployment_target => deployment_target)
find_output = Executable.execute_command('find', [validation_dir, '-name', '*.html'], false)
if find_output != ''
message = 'Static Analysis failed.'
Expand All @@ -720,7 +722,7 @@ def build_pod
error('build_pod', message)
end
else
output = xcodebuild('build', scheme, configuration ? configuration : 'Release')
output = xcodebuild('build', scheme, configuration ? configuration : 'Release', :deployment_target => deployment_target)
end
parsed_output = parse_xcodebuild_output(output)
translate_output_to_linter_messages(parsed_output)
Expand Down Expand Up @@ -757,7 +759,7 @@ def test_pod
UI.warn "Skipping test spec `#{test_spec.name}` on platform `#{consumer.platform_name}` since it is not supported.\n".yellow
else
scheme = @installer.target_installation_results.first[pod_target.name].native_target_for_spec(test_spec)
output = xcodebuild('test', scheme, 'Debug')
output = xcodebuild('test', scheme, 'Debug', :deployment_target => test_spec.deployment_target(consumer.platform_name))
parsed_output = parse_xcodebuild_output(output)
translate_output_to_linter_messages(parsed_output)
end
Expand Down Expand Up @@ -1048,7 +1050,7 @@ def parse_xcodebuild_output(output)
# @return [String] Executes xcodebuild in the current working directory and
# returns its output (both STDOUT and STDERR).
#
def xcodebuild(action, scheme, configuration)
def xcodebuild(action, scheme, configuration, deployment_target:)
Copy link
Contributor

@gyfelton gyfelton Aug 21, 2020

Choose a reason for hiding this comment

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

i see this is a new param, but the implementation does not seem to use it (or where this deployment_target refer to down in line 1059/1061? @dnkoutso

require 'fourflusher'
command = %W(clean #{action} -workspace #{File.join(validation_dir, 'App.xcworkspace')} -scheme #{scheme} -configuration #{configuration})
case consumer.platform_name
Expand Down
Expand Up @@ -4,8 +4,8 @@ module Pod
class Installer
class Xcode
class PodsProjectGenerator
describe PodTargetInstaller do # rubocop:disable Metrics/BlockLength
describe 'In General' do # rubocop:disable Metrics/BlockLength
describe PodTargetInstaller do
describe 'In General' do
before do
@banana_spec = fixture_spec('banana-lib/BananaLib.podspec')
@project = Pod::Project.new(config.sandbox.project_path)
Expand Down
33 changes: 33 additions & 0 deletions spec/unit/validator_spec.rb
Expand Up @@ -600,6 +600,39 @@ def podspec_path(name = 'JSONKit', version = '1.4')
validator.results_message.strip.should.be.empty
end

it 'uses the deployment target of the current test spec' do
Copy link
Member

Choose a reason for hiding this comment

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

it's not super clear to me what in here is validating that the correct deployment target is being used for xcodebuild ?

Copy link
Member

Choose a reason for hiding this comment

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

it pulls a destination from Fourflusher once requesting iOS 8 for building the library and once requesting iOS 9 to run the test. at least that was my understanding. maybe would be better to add an expectation that xcodebuild is provided the correct deployment targets instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeap. Those tests are super convoluted and hard to follow. I verified without my change this test fails.

require 'fourflusher'
Validator.any_instance.unstub(:xcodebuild)
validator = Validator.new(podspec_path, config.sources_manager.master.map(&:url))
validator.use_frameworks = true
validator.instance_variable_set(:@results, [])
validator.stubs(:validate_url)
validator.stubs(:validate_screenshots)
validator.stubs(:check_file_patterns)
validator.stubs(:install_pod)
validator.stubs(:add_app_project_import)
%i(prepare resolve_dependencies download_dependencies write_lockfiles).each do |m|
Installer.any_instance.stubs(m)
end
Installer.any_instance.stubs(:aggregate_targets).returns([])
Installer.any_instance.stubs(:pod_targets).returns([])
validator.spec.ios.deployment_target = '8.0'
test_spec = Specification.new(validator.spec, 'testspec', true) do |s|
s.ios.deployment_target = '9.0'
end
validator.spec.stubs(:subspecs).returns([test_spec])
pod_target = stub('JSONKit-PodTarget')
pod_target.stubs(:name).returns('JSONKit')
validator.stubs(:validation_pod_target).returns(pod_target)
target_installation_result = stub('JSONKitTargetInstallationResult')
target_installation_result.stubs(:native_target_for_spec).with(test_spec).returns('Testspec-Target')
Installer.any_instance.stubs(:target_installation_results).returns([{ 'JSONKit' => target_installation_result }])
Fourflusher::SimControl.any_instance.expects(:destination).with(:oldest, 'iOS', '8.0').returns(['-destination', 'id=XXX'])
Fourflusher::SimControl.any_instance.expects(:destination).with(:oldest, 'iOS', '9.0').returns(['-destination', 'id=XXX'])

validator.validate
end

describe '#podfile_from_spec' do
before do
@validator = Validator.new(podspec_path, config.sources_manager.master.map(&:url))
Expand Down