Fix requirements cloning for :testspecs
#401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
justinseanmartin
force-pushed
the
master
branch
from
ca804ac
to
2e2c2cc
Compare
dnkoutso
approved these changes
Aug 29, 2017
|
We discovered this locally, verified it works as expected. |
justinseanmartin
force-pushed
the
master
branch
from
2e2c2cc
to
d9502fa
Compare
segiddins
approved these changes
Aug 29, 2017
dnkoutso
reviewed
Aug 29, 2017
| @@ -921,7 +921,8 @@ def parse_subspecs(name, requirements) | |||
| end if subspecs | |||
|
|
|||
| test_specs.each do |ss| | |||
| store_pod("#{name}/#{ss}", *requirements.dup) | |||
| requirements_copy = requirements.map(&:dup) | |||
| store_pod("#{name}/#{ss}", *requirements_copy) | |||
There was a problem hiding this comment.
this was basically returning a shallow clone of the requirements and when using test specs we are actually adding a pod not overriding like it happens for when using :subspecs.
The shallow clone caused the store_pod to remove the configuration when parsing the test spec and then the parent had 0 configuration causing an inconsistency between the parent configurations and the test spec configuration.
dnkoutso
reviewed
Aug 29, 2017
| @@ -409,6 +409,15 @@ module Pod | |||
| @child.all_whitelisted_configurations.sort.should == %w(Debug Release) | |||
| end | |||
|
|
|||
| it 'whitelistes pod configurations with testspecs' do | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
馃寛
Found an issue where the requirements weren't being deep copied when using the
:testspecsdirective in combination with the:configurationsdirective. It would lead to an error message showing up with:This addresses the issue by deep copying the requirements Hash contained within the Array instead of just copying the array containing those requirements.