fixes #19131 - upgrade minitest to latest 5.x

- override minitest's `_` expectation method with the gettext method
- rename `location` let helper, conflicted with a minitest method name
- fix `assert_equal nil, [..]` deprecation warnings, prefer `assert nil`

Contains fixes for tests that leak data or behavioral changes between
tests, as the ordering of test cases is now randomised:

- remove Host/Nic class modifications in orchestration concern test
- move NotificationBlueprint seeding into test transaction, preventing
  DB truncation in integration tests from removing the records
- ensure DBCleaner.start is called when in transaction mode to prevent
  records leaking out of integration tests
- sort records for reliable comparisons in TaxonomyTest
- move IPs of BMC NIC factory objects to 1.0.0.0/8 to avoid duplicates
- remove duplicate SettingTest cache clear, missing rescue

bundler.d/test.rb

@@ -1,7 +1,7 @@
 group :test do
   gem 'mocha', '~> 1.1'
   gem 'single_test', '~> 0.6'
-  gem 'minitest', '~> 5.1.0'
+  gem 'minitest', '~> 5.1'
   gem 'minitest-optional_retry', '~> 0.0', :require => false
   gem 'minitest-spec-rails', '~> 5.3'
   gem 'ci_reporter_minitest', :require => false

test/active_support_test_case_helper.rb

@@ -234,4 +234,11 @@ def fake_rest_client_response(data)
       raise "unknown RestClient::Response.create version (#{RestClient.version}, arity #{RestClient::Response.method(:create).arity})"
     end
   end
+
+  # Minitest provides a "_" expects syntax which overrides the gettext "_" method
+  # Override the minitest method and call the original instead for compatibility
+  # with the app's runtime environment.
+  def _(*args)
+    Object.instance_method(:_).bind(self).call(*args)
+  end
 end

test/controllers/api/v2/hosts_controller_test.rb

@@ -853,11 +853,13 @@ def setup
       assert_response :created
       body = ActiveSupport::JSON.decode(@response.body)
       assert_not_nil body['id']
-      host = Host.find_by_id(body['id'])
-      assert_equal 'dhcp75-197.virt.bos.redhat.com', host.name
-      assert_equal @domain, host.domain
-      assert_equal '00:50:56:a9:00:28', host.mac
-      assert_equal true, host.build
+      as_admin do
+        host = Host.find_by_id(body['id'])
+        assert_equal 'dhcp75-197.virt.bos.redhat.com', host.name
+        assert_equal domain, host.domain
+        assert_equal '00:50:56:a9:00:28', host.mac
+        assert_equal true, host.build
+      end
     end
 
     test 'should not import if associated host exists' do

test/controllers/api/v2/users_controller_test.rb

@@ -41,7 +41,7 @@ def setup
     show_response = ActiveSupport::JSON.decode(@response.body)
 
     assert_equal taxonomies(:location1).id, show_response['default_location']['id']
-    assert_equal nil, show_response['default_organization']
+    assert_nil show_response['default_organization']
   end
 
   test "effective_admin is true if group admin is enabled" do

test/controllers/hostgroups_controller_test.rb

@@ -253,7 +253,7 @@ def setup_user(operation, type = 'hostgroups')
       child.reload
       as_admin do
         assert_equal "original", child.parameters["z"]
-        assert_equal nil, child.parameters["x"]
+        assert_nil child.parameters["x"]
       end
     end
   end

test/controllers/hosts_controller_test.rb

@@ -566,7 +566,7 @@ def setup_multiple_compute_resource
       assert_empty flash[:error]
 
       @hosts.each do |host|
-        assert_equal nil, host.reload.puppet_ca_proxy
+        assert_nil host.reload.puppet_ca_proxy
       end
     end
 
@@ -582,7 +582,7 @@ def setup_multiple_compute_resource
       assert_empty flash[:error]
 
       @hosts.each do |host|
-        assert_equal nil, host.reload.puppet_ca_proxy
+        assert_nil host.reload.puppet_ca_proxy
       end
     end
   end
@@ -627,7 +627,7 @@ def setup_multiple_compute_resource
       assert_empty flash[:error]
 
       @hosts.each do |host|
-        assert_equal nil, host.reload.puppet_ca_proxy
+        assert_nil host.reload.puppet_ca_proxy
       end
     end
   end

test/controllers/locations_controller_test.rb

@@ -60,8 +60,8 @@ class LocationsControllerTest < ActionController::TestCase
       delete :destroy, {:id => location.id}, set_session_user.merge(:location_id => location.id)
     end
 
-    assert_equal Location.current, nil
-    assert_equal session[:location_id], nil
+    assert_nil Location.current
+    assert_nil session[:location_id]
   end
 
   test "should save location on session expiry" do
@@ -178,8 +178,8 @@ class LocationsControllerTest < ActionController::TestCase
   test "should clear out Location.current" do
     @request.env['HTTP_REFERER'] = root_url
     get :clear, {}, set_session_user
-    assert_equal Location.current, nil
-    assert_equal session[:location_id], nil
+    assert_nil Location.current
+    assert_nil session[:location_id]
     assert_redirected_to root_url
   end
 

test/controllers/notification_recipients_controller_test.rb

@@ -1,4 +1,5 @@
 require 'test_helper'
+require 'notifications_test_helper'
 
 class NotificationRecipientsControllerTest < ActionController::TestCase
   setup do
@@ -66,38 +67,42 @@ class NotificationRecipientsControllerTest < ActionController::TestCase
     assert_empty response['notifications']
   end
 
-  test "notification when host is destroyed" do
-    host = FactoryGirl.create(:host)
-    assert host.destroy
-    get :index, { :format => 'json' }, set_session_user
-    assert_response :success
-    response = ActiveSupport::JSON.decode(@response.body)
-    assert_equal 1, response['notifications'].size
-    assert_equal "#{host} has been deleted successfully", response['notifications'][0]["text"]
-  end
-
-  test "notification when host is built" do
-    host = FactoryGirl.create(:host, owner: User.current)
-    assert host.update_attribute(:build, true)
-    assert host.built
-    get :index, { :format => 'json' }, set_session_user
-    assert_response :success
-    response = ActiveSupport::JSON.decode(@response.body)
-    assert_equal 1, response['notifications'].size
-    assert_equal "#{host} has been provisioned successfully", response['notifications'][0]["text"]
-  end
-
-  test "notification when host has no owner" do
-    host = FactoryGirl.create(:host, :managed)
-    User.current = nil
-    assert host.update_attributes(owner_id: nil, owner_type: nil, build: true)
-    assert_nil host.owner
-    assert host.built
-    get :index, { :format => 'json' }, set_session_user
-    assert_response :success
-    response = ActiveSupport::JSON.decode(@response.body)
-    assert_equal 1, response['notifications'].size
-    assert_equal "#{host} has no owner set", response['notifications'][0]["text"]
+  context "with seeded notification types" do
+    include NotificationBlueprintSeeds
+
+    test "notification when host is destroyed" do
+      host = FactoryGirl.create(:host)
+      assert host.destroy
+      get :index, { :format => 'json' }, set_session_user
+      assert_response :success
+      response = ActiveSupport::JSON.decode(@response.body)
+      assert_equal 1, response['notifications'].size
+      assert_equal "#{host} has been deleted successfully", response['notifications'][0]["text"]
+    end
+
+    test "notification when host is built" do
+      host = FactoryGirl.create(:host, owner: User.current)
+      assert host.update_attribute(:build, true)
+      assert host.built
+      get :index, { :format => 'json' }, set_session_user
+      assert_response :success
+      response = ActiveSupport::JSON.decode(@response.body)
+      assert_equal 1, response['notifications'].size
+      assert_equal "#{host} has been provisioned successfully", response['notifications'][0]["text"]
+    end
+
+    test "notification when host has no owner" do
+      host = FactoryGirl.create(:host, :managed)
+      User.current = nil
+      assert host.update_attributes(owner_id: nil, owner_type: nil, build: true)
+      assert_nil host.owner
+      assert host.built
+      get :index, { :format => 'json' }, set_session_user
+      assert_response :success
+      response = ActiveSupport::JSON.decode(@response.body)
+      assert_equal 1, response['notifications'].size
+      assert_equal "#{host} has no owner set", response['notifications'][0]["text"]
+    end
   end
 
   test 'group mark as read' do

test/controllers/organizations_controller_test.rb

@@ -72,8 +72,8 @@ class OrganizationsControllerTest < ActionController::TestCase
       delete :destroy, {:id => organization.id}, set_session_user.merge(:organization_id => organization.id)
     end
 
-    assert_equal Organization.current, nil
-    assert_equal session[:organization_id], nil
+    assert_nil Organization.current
+    assert_nil session[:organization_id]
   end
 
   test "should save organization on session expiry" do
@@ -190,8 +190,8 @@ class OrganizationsControllerTest < ActionController::TestCase
   test "should clear out Organization.current" do
     @request.env['HTTP_REFERER'] = root_url
     get :clear, {}, set_session_user
-    assert_equal Organization.current, nil
-    assert_equal session[:organization_id], nil
+    assert_nil Organization.current
+    assert_nil session[:organization_id]
     assert_redirected_to root_url
   end
 

test/factories/host_related.rb

@@ -82,7 +82,7 @@ def set_nic_attributes(host, attributes, evaluator)
   factory :nic_bmc, :class => Nic::BMC, :parent => :nic_managed do
     type 'Nic::BMC'
     sequence(:mac) { |n| "00:43:56:cd:" + n.to_s(16).rjust(4, '0').insert(2, ':') }
-    sequence(:ip) { |n| IPAddr.new((subnet.present? ? subnet.ipaddr.to_i : 255) + n, Socket::AF_INET).to_s }
+    sequence(:ip) { |n| IPAddr.new((subnet.present? ? subnet.ipaddr.to_i : 256 * 256 * 256) + n, Socket::AF_INET).to_s }
     provider 'IPMI'
     username 'admin'
     password 'admin'

test/fixtures/hostgroups.yml

@@ -36,6 +36,7 @@ parent:
   environment: global_puppetmaster
   operatingsystem: centos5_3
   medium: one
+  ptable: autopart
   puppet_proxy: puppetmaster
   puppet_ca_proxy: puppetmaster
   domain: mydomain

test/fixtures/templates.yml

@@ -93,3 +93,10 @@ locked:
   operatingsystems: centos5_3
   locked: true
   type: ProvisioningTemplate
+
+autopart:
+  name: Example partition table
+  template: autopart
+  operatingsystems: centos5_3
+  locked: true
+  type: Ptable

test/integration_test_helper.rb

@@ -9,10 +9,6 @@
 require 'database_cleaner'
 require 'active_support_test_case_helper'
 require 'minitest-optional_retry'
-# load notification blueprint seeds
-require File.join(Rails.root,'db','seeds.d','17-notification_blueprints.rb')
-
-DatabaseCleaner.strategy = :transaction
 
 Capybara.register_driver :poltergeist do |app|
   opts = {
@@ -100,7 +96,7 @@ def wait_for_ajax
     end
   end
 
-  setup :login_admin
+  setup :start_database_cleaner, :login_admin
 
   teardown do
     DatabaseCleaner.clean       # Truncate the database
@@ -111,6 +107,15 @@ def wait_for_ajax
 
   private
 
+  def start_database_cleaner
+    DatabaseCleaner.strategy = database_cleaner_strategy
+    DatabaseCleaner.start
+  end
+
+  def database_cleaner_strategy
+    :transaction
+  end
+
   def login_admin
     SSO.register_method(TestSSO)
     set_request_user(:admin)
@@ -134,9 +139,11 @@ def with_controller_caching(*controller_klasses)
 end
 
 class IntegrationTestWithJavascript < ActionDispatch::IntegrationTest
+  def database_cleaner_strategy
+    :truncation
+  end
+
   def login_admin
-    DatabaseCleaner.strategy = :truncation
-    DatabaseCleaner.start
     Capybara.current_driver = Capybara.javascript_driver
     super
   end

test/models/compute_resources/libvirt_test.rb

@@ -43,7 +43,7 @@ class LibvirtTest < ActiveSupport::TestCase
       cr.stubs(:find_vm_by_uuid).returns(vm)
 
       attrs = cr.vm_compute_attributes_for('abc')
-      assert_equal nil, attrs[:memory]
+      assert_nil attrs[:memory]
     end
   end
 

test/models/host_test.rb

@@ -124,7 +124,7 @@ class HostTest < ActiveSupport::TestCase
 
   test "fetches nil vm compute attributes for bare metal" do
     host = FactoryGirl.create(:host)
-    assert_equal host.vm_compute_attributes, nil
+    assert_nil host.vm_compute_attributes
   end
 
   test "can authorize Host::Managed as non-admin user" do
@@ -415,7 +415,7 @@ def teardown
       assert host.import_facts(raw['facts'])
       host = Host.find_by_name('sinn1636.fail')
       assert_equal '10.35.27.2', host.interfaces.find_by_identifier('br180').ip
-      assert_equal nil, host.primary_interface.ip # eth0 does not have ip address among facts
+      assert_nil host.primary_interface.ip # eth0 does not have ip address among facts
     end
 
     test 'should update certname when host is found by hostname and certname is provided' do
@@ -1717,7 +1717,7 @@ def teardown
       end
 
       test "hosts should be able to retrieve their token if one exists" do
-        h = FactoryGirl.create(:host, :managed)
+        h = FactoryGirl.create(:host, :managed, build: true)
         assert_equal Token.first, h.token
       end
 
@@ -1765,12 +1765,12 @@ def teardown
         assert_equal Token.all.size, 0
       end
 
-      test "token should return false when tokens are disabled or invalid" do
+      test "token should be nil when tokens are disabled or invalid" do
         h = FactoryGirl.create(:host, :managed)
-        assert_equal h.token, nil
+        assert_nil h.token
         Setting[:token_duration] = 30
         h.reload
-        assert_equal h.token, nil
+        assert_nil h.token
       end
     end
 

test/models/hostgroup_test.rb

@@ -150,9 +150,10 @@ class HostgroupTest < ActiveSupport::TestCase
   end
 
   test "inherited id value equals field id value if no ancestry" do
-    hostgroup = hostgroups(:common)
+    hostgroup = hostgroups(:parent)
     [:compute_profile_id, :environment_id, :domain_id, :puppet_proxy_id, :puppet_ca_proxy_id,
      :operatingsystem_id, :architecture_id, :medium_id, :ptable_id, :subnet_id, :subnet6_id].each do |field|
+      refute_nil hostgroup.send(field), "missing #{field}"
       assert_equal hostgroup.send(field), hostgroup.send("inherited_#{field}")
     end
   end
@@ -163,6 +164,7 @@ class HostgroupTest < ActiveSupport::TestCase
     # environment_id is not included in the array below since child value is not null
     [:compute_profile_id, :domain_id, :puppet_proxy_id, :puppet_ca_proxy_id,
      :operatingsystem_id, :architecture_id, :medium_id, :ptable_id, :subnet_id, :subnet6_id].each do |field|
+      refute_nil parent.send(field), "missing #{field}"
       assert_equal parent.send(field), child.send("inherited_#{field}")
     end
   end
@@ -182,6 +184,7 @@ class HostgroupTest < ActiveSupport::TestCase
     # environment is not included in the array below since child value is not null
     [:compute_profile, :domain, :puppet_proxy, :puppet_ca_proxy,
      :operatingsystem, :architecture, :medium, :ptable, :subnet, :subnet6].each do |field|
+      refute_nil parent.send(field), "missing #{field}"
       assert_equal parent.send(field), child.send(field)
     end
   end

test/models/medium_test.rb

@@ -103,6 +103,6 @@ class MediumTest < ActiveSupport::TestCase
 
   test "blank os family is saved as nil" do
     medium = Medium.new :name => "dummy", :path => "http://hello", :os_family => ""
-    assert_equal nil, medium.os_family
+    assert_nil medium.os_family
   end
 end

test/models/nic_test.rb

@@ -390,15 +390,15 @@ class DisallowedTestNic < Nic::Base
     end
 
     test "type_by_name returns nil for an unknown name" do
-      assert_equal nil, Nic::Base.type_by_name("UNKNOWN_NAME")
+      assert_nil Nic::Base.type_by_name("UNKNOWN_NAME")
     end
 
     test "type_by_name finds the class" do
       assert_equal HumanizedTestNic, Nic::Base.type_by_name("custom")
     end
 
     test "type_by_name returns nil for classes that aren't allowed" do
-      assert_equal nil, Nic::Base.type_by_name("DisallowedTestNic")
+      assert_nil Nic::Base.type_by_name("DisallowedTestNic")
     end
 
     test 'fqdn_changed? should be true if name changes' do