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

Fix modifying of frozen strings via config files and tests #8562

Merged
merged 4 commits into from
Mar 20, 2017
Merged

Conversation

KrauseFx
Copy link
Member

Fixes #8548

@KrauseFx
Copy link
Member Author

KrauseFx commented Mar 17, 2017

Oh wow, interesting, so turns out in

Ruby 2.3.0

irb(main):003:0> false.dup
NoMethodError: undefined method `dupe' for false:FalseClass

while in Ruby 2.4.0

irb(main):003:0> false.dup
=> false

Good thing the unit tests got this before we merged the change

@KrauseFx
Copy link
Member Author

Alright, updated to work with all versions of Ruby 👍

@giginet
Copy link
Collaborator

giginet commented Mar 17, 2017

By the way in current CI environments, the tests run only on Ruby 2.3.1.
Why don't we run specs on the multiple versions?
(We should run specs on Ruby2.3.x, 2.4.x and 2.0.0(Apple system default))

@hjanuschka
Copy link
Collaborator

we had that topic a few times i think right now circle does not support matrix runs, @dantoml still right?

expect(config[:app_identifier]).to eq(app_identifier)
config[:app_identifier].gsub!("yolo", "yoLiveTwice")
expect(config[:app_identifier]).to eq("com.krausefx.yoLiveTwice")
ENV.delete("SOMETHING_RANDOM_APP_IDENTIFIER")
Copy link
Contributor

Choose a reason for hiding this comment

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

For unit tests we have a helper, with_env_values that will ensure that we don't forget to delete the ENV value at the end. Let's keep using that 👍

with_env_values('SOMETHING_RANDOM_IOS_VERSION' -> ios_version) do
  config = FastlaneCore::Configuration.create(options, {})
  config.load_configuration_file('ConfigFileEnv')
  expect(config[:ios_version]).to eq(ios_version)
  config[:ios_version].gsub!(".1", ".2")
  expect(config[:ios_version]).to eq("9.2")
end

expect(config[:ios_version]).to eq(ios_version)
config[:ios_version].gsub!(".1", ".2")
expect(config[:ios_version]).to eq("9.2")
ENV.delete("SOMETHING_RANDOM_IOS_VERSION")
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use with_env_values(hash, &block)

@endocrimes
Copy link
Collaborator

@hjanuschka still correct

@KrauseFx KrauseFx merged commit 80471bd into master Mar 20, 2017
@KrauseFx KrauseFx deleted the fix-frozen branch March 20, 2017 18:44
@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.21.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Jun 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants