From 02b6a9ed80da28ade693bc3e4db9221b27e524b9 Mon Sep 17 00:00:00 2001 From: Oleh Fedorenko Date: Wed, 10 Jan 2024 20:48:51 +0000 Subject: [PATCH 1/2] Fixes #36849 - Run GHA on Ruby 3.0 --- .github/matrix.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/matrix.json b/.github/matrix.json index 81e37ac5f94..f15a18dd692 100644 --- a/.github/matrix.json +++ b/.github/matrix.json @@ -1,5 +1,5 @@ { "postgresql": ["12"], - "ruby": ["2.7"], + "ruby": ["2.7", "3.0"], "node": ["14"] } From eefdccdf6db34c5419ceb4c1c605c319b7469a35 Mon Sep 17 00:00:00 2001 From: Oleh Fedorenko Date: Wed, 24 Jan 2024 13:14:36 +0000 Subject: [PATCH 2/2] Refs #36849 - Enable strict keyword argument matching --- .../api/v2/hosts_controller_test.rb | 24 +++++++++++-------- test/controllers/hosts_controller_test.rb | 6 +++-- test/helpers/application_helper_test.rb | 4 ++-- test/models/host_status/global_test.rb | 2 +- test/models/host_test.rb | 2 +- test/models/mail_notification_test.rb | 4 ++-- test/models/orchestration/tftp_test.rb | 4 ++-- test/models/subnet/ipv4_test.rb | 8 +++---- test/test_helper.rb | 4 ++++ .../api_hosts_controller_extension_test.rb | 2 +- test/unit/foreman_ansible/fact_parser_test.rb | 4 ++-- test/unit/menu_item_test.rb | 2 +- test/unit/proxy_api/dhcp_test.rb | 6 ++--- 13 files changed, 41 insertions(+), 31 deletions(-) diff --git a/test/controllers/api/v2/hosts_controller_test.rb b/test/controllers/api/v2/hosts_controller_test.rb index c0727459e16..af1f69a4ce4 100644 --- a/test/controllers/api/v2/hosts_controller_test.rb +++ b/test/controllers/api/v2/hosts_controller_test.rb @@ -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 @@ -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) } @@ -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 @@ -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) diff --git a/test/controllers/hosts_controller_test.rb b/test/controllers/hosts_controller_test.rb index 90b896c597e..2cd217dfb3a 100644 --- a/test/controllers/hosts_controller_test.rb +++ b/test/controllers/hosts_controller_test.rb @@ -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: { diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index bdda9aab17f..e58eb66f2c8 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -47,7 +47,7 @@ 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('test'.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 @@ -55,7 +55,7 @@ def test_generate_link_for test '#documentation_button forwards plugin manual options to #documentation_url' do expects(:icon_text).returns('http://nowhere.com') expects(:link_to).returns('test'.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 diff --git a/test/models/host_status/global_test.rb b/test/models/host_status/global_test.rb index 1777a0dae22..b8d62a814bc 100644 --- a/test/models/host_status/global_test.rb +++ b/test/models/host_status/global_test.rb @@ -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 diff --git a/test/models/host_test.rb b/test/models/host_test.rb index 8675666eb9f..5485276e4a1 100644 --- a/test/models/host_test.rb +++ b/test/models/host_test.rb @@ -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 diff --git a/test/models/mail_notification_test.rb b/test/models/mail_notification_test.rb index 1d70b3a1dce..7631f59e2fe 100644 --- a/test/models/mail_notification_test.rb +++ b/test/models/mail_notification_test.rb @@ -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 diff --git a/test/models/orchestration/tftp_test.rb b/test/models/orchestration/tftp_test.rb index 9fc77090007..f0baa984f59 100644 --- a/test/models/orchestration/tftp_test.rb +++ b/test/models/orchestration/tftp_test.rb @@ -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') diff --git a/test/models/subnet/ipv4_test.rb b/test/models/subnet/ipv4_test.rb index c390fa0b8bb..d82d690318f 100644 --- a/test/models/subnet/ipv4_test.rb +++ b/test/models/subnet/ipv4_test.rb @@ -162,14 +162,14 @@ 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 @@ -177,9 +177,9 @@ def setup 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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 24fbbbc57ca..f674603eddd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -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"] diff --git a/test/unit/concerns/api_hosts_controller_extension_test.rb b/test/unit/concerns/api_hosts_controller_extension_test.rb index f5fd3746daf..500551e08dc 100644 --- a/test/unit/concerns/api_hosts_controller_extension_test.rb +++ b/test/unit/concerns/api_hosts_controller_extension_test.rb @@ -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 diff --git a/test/unit/foreman_ansible/fact_parser_test.rb b/test/unit/foreman_ansible/fact_parser_test.rb index 40b6b7f25ab..6d90064f901 100644 --- a/test/unit/foreman_ansible/fact_parser_test.rb +++ b/test/unit/foreman_ansible/fact_parser_test.rb @@ -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 diff --git a/test/unit/menu_item_test.rb b/test/unit/menu_item_test.rb index 28a646aab79..7c5f9c0ec46 100644 --- a/test/unit/menu_item_test.rb +++ b/test/unit/menu_item_test.rb @@ -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 diff --git a/test/unit/proxy_api/dhcp_test.rb b/test/unit/proxy_api/dhcp_test.rb index afdf27eb80b..d9e34d306e5 100644 --- a/test/unit/proxy_api/dhcp_test.rb +++ b/test/unit/proxy_api/dhcp_test.rb @@ -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 @@ -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 @@ -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