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

Fixes #36849 - Run GHA on Ruby 3.0 #9989

Merged
merged 2 commits into from
Feb 1, 2024
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 .github/matrix.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"postgresql": ["12"],
"ruby": ["2.7"],
"ruby": ["2.7", "3.0"],
Copy link
Member

Choose a reason for hiding this comment

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

Can we separate this out to its own commit? I'd like to merge all the preparation work before we pull the trigger.

"node": ["14"]
}
24 changes: 14 additions & 10 deletions test/controllers/api/v2/hosts_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,13 @@ def expected_compute_attributes(compute_attrs, index)
compute_attrs.vm_interfaces[index].update("from_profile" => compute_attrs.compute_profile.name)
end

def expect_attribute_modifier(modifier_class, args)
def expect_attribute_modifier(modifier_class, **args)
modifier = mock(modifier_class.name)
modifier_class.expects(:new).with(*args).returns(modifier)
if args.any? # TODO: The else statement can be removed once we fully switched to Ruby 3
modifier_class.expects(:new).with(**args).returns(modifier)
else
modifier_class.expects(:new).returns(modifier)
end
Host.any_instance.expects(:apply_compute_profile).with(modifier)
end

Expand Down Expand Up @@ -365,22 +369,22 @@ def last_record

test "create applies attribute modifiers on the new host" do
disable_orchestration
expect_attribute_modifier(ComputeAttributeMerge, [])
expect_attribute_modifier(InterfaceMerge, [{:merge_compute_attributes => true}])
expect_attribute_modifier(ComputeAttributeMerge)
expect_attribute_modifier(InterfaceMerge, :merge_compute_attributes => true)
post :create, params: { :host => valid_attrs }
end

test "update applies attribute modifiers on the host" do
disable_orchestration
expect_attribute_modifier(ComputeAttributeMerge, [])
expect_attribute_modifier(InterfaceMerge, [{:merge_compute_attributes => true}])
expect_attribute_modifier(ComputeAttributeMerge)
expect_attribute_modifier(InterfaceMerge, :merge_compute_attributes => true)
put :update, params: { :id => @host.to_param, :host => valid_attrs }
end

test "update applies attribute modifiers on the host when compute profile is changed" do
disable_orchestration
expect_attribute_modifier(ComputeAttributeMerge, [])
expect_attribute_modifier(InterfaceMerge, [{:merge_compute_attributes => true}])
expect_attribute_modifier(ComputeAttributeMerge)
expect_attribute_modifier(InterfaceMerge, :merge_compute_attributes => true)

compute_attrs = compute_attributes(:with_interfaces)
put :update, params: { :id => @host.to_param, :host => basic_attrs_with_profile(compute_attrs) }
Expand Down Expand Up @@ -832,7 +836,7 @@ def initialize_proxy_ops
end

test "boot call to interface" do
ProxyAPI::BMC.any_instance.stubs(:boot).with(:function => 'bootdevice', :device => 'bios').
ProxyAPI::BMC.any_instance.stubs(:boot).with({ :function => 'bootdevice', :device => 'bios' }).
returns({ "action" => "bios", "result" => true } .to_json)
put :boot, params: { :id => @bmchost.to_param, :device => 'bios' }
assert_response :success
Expand Down Expand Up @@ -860,7 +864,7 @@ def initialize_proxy_ops

test 'responds correctly for non-admin user if BMC is available' do
ProxyAPI::BMC.any_instance.stubs(:boot).
with(:function => 'bootdevice', :device => 'bios').
with({ :function => 'bootdevice', :device => 'bios' }).
returns({ "action" => "bios", "result" => true } .to_json)
put :boot, params: { :id => @bmchost.to_param, :device => 'bios' },
session: set_session_user.merge(:user => @one.id)
Expand Down
6 changes: 4 additions & 2 deletions test/controllers/hosts_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1423,8 +1423,10 @@ class Host::Test < Host::Base; end
end

Host::Managed.any_instance.expects('compute_attributes=').with(
'scsi_controllers' => scsi_controllers,
'volumes_attributes' => { '0' => volume_attributes }
{
'scsi_controllers' => scsi_controllers,
'volumes_attributes' => { '0' => volume_attributes },
}
)

put :update, params: {
Expand Down
4 changes: 2 additions & 2 deletions test/helpers/application_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ def test_generate_link_for
test '#documentation_button forwards options to #documentation_url' do
expects(:icon_text).returns('http://nowhere.com')
expects(:link_to).returns('<a>test</a>'.html_safe)
expects(:documentation_url).with('2.2PluginSection', { :root_url => 'http://www.theforeman.org/my_plugin/v0.1/index.html#' })
expects(:documentation_url).with('2.2PluginSection', :root_url => 'http://www.theforeman.org/my_plugin/v0.1/index.html#')

documentation_button '2.2PluginSection', :root_url => 'http://www.theforeman.org/my_plugin/v0.1/index.html#'
end

test '#documentation_button forwards plugin manual options to #documentation_url' do
expects(:icon_text).returns('http://nowhere.com')
expects(:link_to).returns('<a>test</a>'.html_safe)
expects(:documentation_url).with('2.2PluginSection', { type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2' })
expects(:documentation_url).with('2.2PluginSection', type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2')

documentation_button '2.2PluginSection', type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2'
end
Expand Down
2 changes: 1 addition & 1 deletion test/models/host_status/global_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def setup
test '.build(statuses, :last_reports => [reports]) uses reports cache for configuration statuses' do
status = HostStatus::ConfigurationStatus.new
report = Report.new(:host => Host.last)
status.expects(:relevant?).with(:last_reports => [report]).returns(true)
status.expects(:relevant?).with({ :last_reports => [report] }).returns(true)
status.expects(:to_global).returns(:result)
global = HostStatus::Global.build([status], :last_reports => [report])
assert_equal :result, global.status
Expand Down
2 changes: 1 addition & 1 deletion test/models/host_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3242,7 +3242,7 @@ def to_managed!
bmc_proxy_stub = stub('bmc_proxy')
@host.expects(:bmc_available?).returns(true)
@host.expects(:bmc_proxy).returns(bmc_proxy_stub)
bmc_proxy_stub.expects(:boot).with(:function => 'bootdevice', :device => 'bios').returns(true)
bmc_proxy_stub.expects(:boot).with({ :function => 'bootdevice', :device => 'bios' }).returns(true)
assert @host.ipmi_boot('bios')
end
end
Expand Down
4 changes: 2 additions & 2 deletions test/models/mail_notification_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class MailNotificationTest < ActiveSupport::TestCase
mailer = FactoryBot.create(:mail_notification)
mail = mock('mail')
mail.expects(:deliver_now).twice
HostMailer.expects(:test_mail).with(:foo, :user => users[0]).returns(mail)
HostMailer.expects(:test_mail).with(:foo, :user => users[1]).returns(mail)
HostMailer.expects(:test_mail).with(:foo, { :user => users[0] }).returns(mail)
HostMailer.expects(:test_mail).with(:foo, { :user => users[1] }).returns(mail)
mailer.deliver(:foo, :users => users)
end

Expand Down
4 changes: 2 additions & 2 deletions test/models/orchestration/tftp_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,12 @@ class TFTPOrchestrationTest < ActiveSupport::TestCase
ProxyAPI::TFTP.any_instance.expects(:set).with(
'PXEGrub2',
'00:53:67:ab:dd:00',
{:pxeconfig => 'Template'}
:pxeconfig => 'Template'
).once
ProxyAPI::TFTP.any_instance.expects(:set).with(
'PXEGrub2',
'00:53:67:ab:dd:01',
{:pxeconfig => 'Template'}
:pxeconfig => 'Template'
).once
host.provision_interface.stubs(:generate_pxe_template).returns('Template')
host.provision_interface.send(:setTFTP, 'PXEGrub2')
Expand Down
8 changes: 4 additions & 4 deletions test/models/subnet/ipv4_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,24 +162,24 @@ def setup
stubs(:subnets => [{ 'network' => '192.168.11.0',
'netmask' => '255.255.255.0',
'options' => dhcp_options }])
Subnet.expects(:new).with(:network => "192.168.11.0",
Subnet.expects(:new).with({:network => "192.168.11.0",
:mask => "255.255.255.0",
:gateway => "192.168.11.1",
:dns_primary => "192.168.11.1",
:dns_secondary => "8.8.8.8",
:from => "192.168.11.0",
:to => "192.168.11.200",
:dhcp => @mock_proxy)
:dhcp => @mock_proxy})
Subnet::Ipv4.import(@mock_proxy)
end

test 'imports subnets without options' do
ProxyAPI::DHCP.any_instance.
stubs(:subnets => [{ 'network' => '192.168.11.0',
'netmask' => '255.255.255.0' }])
Subnet.expects(:new).with(:network => "192.168.11.0",
Subnet.expects(:new).with({:network => "192.168.11.0",
:mask => "255.255.255.0",
:dhcp => @mock_proxy)
:dhcp => @mock_proxy})
Subnet::Ipv4.import(@mock_proxy)
end
end
Expand Down
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
end
end

Mocha.configure do |config|
config.strict_keyword_argument_matching = true
end

# Use our custom test runner, and register a fake plugin to skip a specific test
Foreman::Plugin.register :skip_test do
tests_to_skip "CustomRunnerTest" => ["custom runner is working"]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/concerns/api_hosts_controller_extension_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ApiHostsControllerExtensionTest < ActiveSupport::TestCase

test 'adds permissions_check filter when using #check_permissions_for dsl' do
@klass.stubs :foo
@klass.expects(:before_action).with(:permissions_check, {:only => [:foo]})
@klass.expects(:before_action).with(:permissions_check, :only => [:foo])
@klass.check_permissions_for [:foo]
end

Expand Down
4 changes: 2 additions & 2 deletions test/unit/foreman_ansible/fact_parser_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class FactParserTest < ActiveSupport::TestCase
_, minor_fact = @facts_parser.
facts['ansible_distribution_version'].split('.')
Operatingsystem.expects(:where).
with(:name => @facts_parser.facts['ansible_distribution'],
:major => major_fact, :minor => minor_fact || '').
with({ :name => @facts_parser.facts['ansible_distribution'],
:major => major_fact, :minor => minor_fact || '' }).
returns(sample_mock)
sample_mock.expects(:first)
@facts_parser.operatingsystem
Expand Down
2 changes: 1 addition & 1 deletion test/unit/menu_item_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_menu_item_should_use_url_parameter_when_available

def test_menu_item_uses_url_hash_by_default
item = Menu::Item.new(:test_good_menu, :url_hash => {:controller => 'test', :action => 'index'}, :after => :me)
ActionDispatch::Routing::RouteSet.any_instance.expects(:url_for).with(:controller => 'test', :action => 'index', :only_path => true).returns('/url')
ActionDispatch::Routing::RouteSet.any_instance.expects(:url_for).with({ :controller => 'test', :action => 'index', :only_path => true }).returns('/url')
assert_equal '/url', item.url
end

Expand Down
6 changes: 3 additions & 3 deletions test/unit/proxy_api/dhcp_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class ProxyApiDhcpTest < ActiveSupport::TestCase
subnet_mock.expects(:network).returns('192.168.0.0')
subnet_mock.expects(:from).returns(nil)

proxy_dhcp.expects(:get).with('192.168.0.0/unused_ip', {query: ""}).returns(fake_rest_client_response({:ip => '192.168.0.50'}))
proxy_dhcp.expects(:get).with('192.168.0.0/unused_ip', query: "").returns(fake_rest_client_response({:ip => '192.168.0.50'}))
assert_equal({'ip' => '192.168.0.50'}, proxy_dhcp.unused_ip(subnet_mock))
end

Expand All @@ -87,7 +87,7 @@ class ProxyApiDhcpTest < ActiveSupport::TestCase
subnet_mock.expects(:from).at_least_once.returns(from_mock)
subnet_mock.expects(:to).at_least_once.returns(to_mock)

proxy_dhcp.expects(:get).with("192.168.0.0/unused_ip", { query: "from=192.168.0.50&to=192.168.0.150"}).returns(fake_rest_client_response({:ip => '192.168.0.50'}))
proxy_dhcp.expects(:get).with("192.168.0.0/unused_ip", query: "from=192.168.0.50&to=192.168.0.150").returns(fake_rest_client_response({:ip => '192.168.0.50'}))
assert_equal({'ip' => '192.168.0.50'}, proxy_dhcp.unused_ip(subnet_mock))
end

Expand All @@ -96,7 +96,7 @@ class ProxyApiDhcpTest < ActiveSupport::TestCase
subnet_mock.expects(:network).returns('192.168.0.0')
subnet_mock.expects(:from).returns(nil)

proxy_dhcp.expects(:get).with('192.168.0.0/unused_ip', { query: "mac=00:11:22:33:44:55"}).returns(fake_rest_client_response({:ip => '192.168.0.50'}))
proxy_dhcp.expects(:get).with('192.168.0.0/unused_ip', query: "mac=00:11:22:33:44:55").returns(fake_rest_client_response({:ip => '192.168.0.50'}))
assert_equal({'ip' => '192.168.0.50'}, proxy_dhcp.unused_ip(subnet_mock, '00:11:22:33:44:55'))
end
end
Loading