Skip to content

Commit

Permalink
Refs #36849 - Enable strict keyword argument matching
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren authored and ekohl committed Feb 1, 2024
1 parent 0e0c29b commit 4728839
Show file tree
Hide file tree
Showing 13 changed files with 41 additions and 31 deletions.
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

0 comments on commit 4728839

Please sign in to comment.