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

Updates to work with Ruby 2.4.0 #13104

Merged
merged 4 commits into from
May 1, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ gem "rails-controller-testing", :require => false
gem "rails-i18n", "~>5.x"
gem "recursive-open-struct", "~>1.0.0"
gem "responders", "~>2.0"
gem "ripper_ruby_parser", :require => false
gem "ruby-dbus" # For external auth
gem "ruby-progressbar", "~>1.7.0", :require => false
gem "ruby_parser", "~>3.8", :require => false
gem "rufus-scheduler", "~>3.1.3", :require => false
gem "rugged", "=0.25.0b10", :require => false
gem "secure_headers", "~>3.0.0"
Expand Down
6 changes: 3 additions & 3 deletions lib/extensions/descendant_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ def self.status(io = $stdout)
# and the name of its superclass), given a path to a ruby script file.
module Parser
def classes_in(filename)
require 'ruby_parser'
require 'ripper_ruby_parser'

content = File.read(filename)
parsed = RubyParser.for_current_ruby.parse(content)
parsed = RipperRubyParser::Parser.new.parse(content)

classes = collect_classes(parsed)

Expand All @@ -94,7 +94,7 @@ def classes_in(filename)
[search_combos, define_combos, flatten_name(name), flatten_name(sklass)]
end.compact

rescue Racc::ParseError
rescue RipperRubyParser::SyntaxError
puts "\nSyntax error in #{filename}\n\n"
raise
end
Expand Down
5 changes: 4 additions & 1 deletion spec/models/custom_attribute_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
end

it "returns the value type of Fixnum custom attributes" do
expect(int_custom_attribute.value_type).to eq(:fixnum)
# TODO: Ruby 2.4 unifies fixnum and bignum into integer, we shouldn't be
# returnings ruby types like this.
expected = RUBY_VERSION >= "2.4.0" ? :integer : :fixnum
expect(int_custom_attribute.value_type).to eq(expected)
Copy link
Member

Choose a reason for hiding this comment

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

To be safe @cben, others, I kept this test but made it return what ruby 2.4.0 is expecting. We'll have to address this as we move to ruby 2.4.0 more. This should be enough to get us compatible for now.

end
end
2 changes: 1 addition & 1 deletion spec/models/miq_server/rhn_mirror_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

expect(FileUtils).to receive(:mkdir_p).with("/repo/mirror")
expect(MiqApache::Conf).to receive(:create_conf_file).once.and_return(true)
expect(FileUtils).to receive(:rm).with("/etc/httpd/conf.d/manageiq-https-mirror.conf", :force => true)
expect(FileUtils).to receive(:rm).with("/etc/httpd/conf.d/manageiq-https-mirror.conf", hash_including(:force => true))
Copy link
Member

Choose a reason for hiding this comment

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

See commit, it's basically changed from an options hash to kwargs so the expectations on arguments is different now.

expect(MiqApache::Control).to receive(:restart).once
expect(LinuxAdmin::Yum).to receive(:download_packages).once.with("/repo/mirror", "cfme-appliance")
expect(Dir).to receive(:glob).with("/repo/mirror/**/*.rpm").and_return(rpm_file_list)
Expand Down
15 changes: 15 additions & 0 deletions spec/support/webmock_ruby24_bridge.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Ruby 2.4.0 removed the closed? check in the conditional in: s.close if !s.closed?
# Webmock was changed to add close to StubSocket along with another change.
# https://github.com/ruby/ruby/commit/f845a9ef76c0195254ded79c85c24332534f4057
# https://github.com/bblimke/webmock/commit/8f2176a1fa75374df55b87d782e08ded673a75b4
# WebMock 2.3.1+ fixed this.
# We should upgrade webmock but that requires some re-recording of cassettes (I think)
# and maybe other things.
if WebMock::VERSION < "2.3.1"
class StubSocket
def close
end
end
else
warn "Remove me: #{__FILE__}:#{__LINE__}. WebMock 2.3.1+ fixed the issue with ruby 2.4.0 by adding StubSocket#close."
Copy link
Member

Choose a reason for hiding this comment

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

Added a reminder in case we upgrade webmock and forget to remove this monkey patch.

Copy link
Member

Choose a reason for hiding this comment

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

you weren't tempted to upgrade webmock?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I was. I have no idea how to re-record all of the cassettes though. #14270

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, also, re-recording cassettes is a major bloat on a repo if everything is replaced completely. Might be better if we can figure out how to just update the yaml of the cassettes in a programmatic fashion with the needed changes for webmock to avoid needing to actually re-record the cassettes.

end