Skip to content

Commit

Permalink
Create or assert associations by id to avoid class mismatches errors
Browse files Browse the repository at this point in the history
Rails 7 changed the way migrations are run.  The migration class
constant is now removed and the file is reloaded between migrations.

This means tests should not use expect/allow on migration stub classes
as the constant stubbed is not the same constant the migration is run against.

Additionally, comparisons of objects created before the migration with ones
after will also not work.  In that case, you can assert by id or other
attributes instead of comparing objects.

Rails also checks association classes are correct and will also not match
expectations.  For example, instead of using host to create or query
the host association, use host_id instead.  This also works with has_manys
such as miq_product_feature_ids.

See https://github.com/rails/rails/pull 42198
  • Loading branch information
jrafanie committed May 29, 2024
1 parent 05bc0f4 commit 688323b
Show file tree
Hide file tree
Showing 30 changed files with 237 additions and 238 deletions.
4 changes: 2 additions & 2 deletions spec/migrations/20180316164826_update_switch_types_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
:type => "ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch")

host_stub.create!(:type => "ManageIQ::Providers::Vmware::InfraManager::HostEsx").tap do |host|
host.host_switches.create!(:host => host, :switch => dvswitch)
host.host_switches.create!(:host => host, :switch => host_switch)
host.host_switches.create!(:host_id => host.id, :switch_id => dvswitch.id)
host.host_switches.create!(:host_id => host.id, :switch_id => host_switch.id)
end

migrate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
let(:miq_approval_stub) { migration_stub :MiqApproval }

it 'only removes Host Provision request instances' do
miq_request_stub.create!(:type => 'MiqProvisionRequest', :miq_approvals => [miq_approval_stub.create!])
miq_request_stub.create!(:type => 'MiqHostProvisionRequest', :miq_approvals => [miq_approval_stub.create!])
miq_request_stub.create!(:type => 'ServiceTemplateProvisionRequest', :miq_approvals => [miq_approval_stub.create!])
miq_request_stub.create!(:type => 'MiqProvisionRequest', :miq_approval_ids => [miq_approval_stub.create!.id])
miq_request_stub.create!(:type => 'MiqHostProvisionRequest', :miq_approval_ids => [miq_approval_stub.create!.id])
miq_request_stub.create!(:type => 'ServiceTemplateProvisionRequest', :miq_approval_ids => [miq_approval_stub.create!.id])

miq_request_task_stub.create!(:type => 'MiqProvision')
miq_request_task_stub.create!(:type => 'MiqHostProvision')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
host = host_stub.create!
conversion_host = conversion_host_stub.create!(:resource => host)
task = task_stub.create!(
:type => 'ServiceTemplateTransformationPlanTask',
:conversion_host => conversion_host
:type => 'ServiceTemplateTransformationPlanTask',
:conversion_host_id => conversion_host.id
)

migrate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
let(:ems) { migration_stub(:ExtManagementSystem) }
let(:service) { migration_stub(:Service) }
let(:tenant) { migration_stub(:Tenant).create! }
let(:group) { migration_stub(:MiqGroup).create!(:tenant => tenant) }
let(:group) { migration_stub(:MiqGroup).create!(:tenant_id => tenant.id)}
let!(:user) { migration_stub(:User).create!(:userid => "admin", :miq_groups => [group], :current_group => group) }

migration_context :up do
it "sets owner, tenant, and group from the user if neither the ems and service exist" do
stack = orchestration_stack.create!(:ext_management_system => nil)
stack = orchestration_stack.create!(:ems_id => nil)
expect(orchestration_stack.count).to eq(1)

migrate
Expand All @@ -20,8 +20,8 @@
end

it "sets owner, tenant, and group from the ems if the ems exists and the service doesn't" do
ext_ms = ems.create!(:tenant => tenant)
stack = orchestration_stack.create!(:ext_management_system => ext_ms)
ext_ms = ems.create!(:tenant_id => tenant.id)
stack = orchestration_stack.create!(:ems_id => ext_ms.id)
expect(orchestration_stack.count).to eq(1)

migrate
Expand All @@ -42,7 +42,7 @@

it "sets owner, tenant, and group from the service if the service exists and ems doesn't" do
svc = service.create!(:tenant_id => tenant.id, :miq_group_id => group.id)
stack = orchestration_stack.create!(:direct_services => [svc])
stack = orchestration_stack.create!(:direct_service_ids => [svc.id])
expect(orchestration_stack.count).to eq(1)

migrate
Expand All @@ -52,9 +52,9 @@
end

it "sets owner, tenant, and group from the service if the service and ems exists" do
ext_ms = ems.create!(:tenant => tenant)
ext_ms = ems.create!(:tenant_id => tenant.id)
svc = service.create!(:tenant_id => tenant.id, :miq_group_id => group.id)
stack = orchestration_stack.create!(:direct_services => [svc], :ext_management_system => ext_ms)
stack = orchestration_stack.create!(:direct_service_ids => [svc.id], :ems_id => ext_ms.id)
expect(orchestration_stack.count).to eq(1)

migrate
Expand Down
47 changes: 23 additions & 24 deletions spec/migrations/20190201173316_add_missing_ems_id_to_switch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,41 +44,40 @@ class Switch < ActiveRecord::Base

# Hosts
host_esx = host_stub.create!(:type => "ManageIQ::Providers::Vmware::InfraManager::HostEsx",
:ext_management_system => vmware_ems)
:ems_id => vmware_ems.id)
host_esx_archived = host_stub.create!(:type => "ManageIQ::Providers::Vmware::InfraManager::HostEsx",
:ext_management_system => nil)
:ems_id => nil)

host_redhat = host_stub.create!(:type => "ManageIQ::Providers::Redhat::InfraManager::Host",
:ext_management_system => redhat_ems)
:ems_id => redhat_ems.id)

# Switches
dvswitch = switch_stub.create!(:name => "DVS", :uid_ems => "dvswitch-1", :shared => true,
:type => "ManageIQ::Providers::Vmware::InfraManager::DistributedVirtualSwitch")
dvswitch_without_assoc = switch_stub.create!(:name => "DVS", :uid_ems => "dvswitch-3", :shared => true,
:host => host_esx,
:ext_management_system => vmware_ems,
:type => "ManageIQ::Providers::Vmware::InfraManager::DistributedVirtualSwitch")
dvswitch_archived = switch_stub.create!(:name => "DVS", :uid_ems => "dvswitch-1", :shared => true,
:ext_management_system => vmware_ems,
:type => "ManageIQ::Providers::Vmware::InfraManager::DistributedVirtualSwitch")
dvswitch_without_assoc = switch_stub.create!(:name => "DVS", :uid_ems => "dvswitch-3", :shared => true,
:host_id => host_esx.id,
:ems_id => vmware_ems.id,
:type => "ManageIQ::Providers::Vmware::InfraManager::DistributedVirtualSwitch")
dvswitch_archived = switch_stub.create!(:name => "DVS", :uid_ems => "dvswitch-1", :shared => true,
:ems_id => vmware_ems.id,
:type => "ManageIQ::Providers::Vmware::InfraManager::DistributedVirtualSwitch")
host_switch = switch_stub.create!(:name => "vSwitch0", :uid_ems => "vSwitch0", :shared => false,
:type => "ManageIQ::Providers::Vmware::InfraManager::HostVirtualSwitch")
host_switch_archived = switch_stub.create!(:name => "vSwitch0", :uid_ems => "vSwitch0",
:type => "ManageIQ::Providers::Vmware::InfraManager::HostVirtualSwitch")
host_switch_archived = switch_stub.create!(:name => "vSwitch0", :uid_ems => "vSwitch0",
:type => "ManageIQ::Providers::Vmware::InfraManager::HostVirtualSwitch")
redhat_switch = switch_stub.create!(:name => "vSwitch0", :uid_ems => "vSwitch0")
physical_switch = switch_stub.create!(:name => "Physical Switch", :uid_ems => "switch-1",
:type => "ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch",
:ext_management_system => lenovo_ems)
physical_switch1 = switch_stub.create!(:name => "Physical Switch", :uid_ems => "switch-1",
:type => "ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch",
:ext_management_system => nil)

physical_switch = switch_stub.create!(:name => "Physical Switch", :uid_ems => "switch-1",
:type => "ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch",
:ems_id => lenovo_ems.id)
physical_switch1 = switch_stub.create!(:name => "Physical Switch", :uid_ems => "switch-1",
:type => "ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch",
:ems_id => nil)
# Host -> switches mapping
host_esx.host_switches.create!(:host => host_esx, :switch => dvswitch)
host_esx.host_switches.create!(:host => host_esx, :switch => host_switch)
host_esx_archived.host_switches.create!(:host => host_esx_archived, :switch => host_switch_archived)
host_esx_archived.host_switches.create!(:host => host_esx_archived, :switch => dvswitch_archived)
host_redhat.host_switches.create!(:host => host_redhat, :switch => redhat_switch)
host_esx.host_switches.create!(:host_id => host_esx.id, :switch_id => dvswitch.id)
host_esx.host_switches.create!(:host_id => host_esx.id, :switch_id => host_switch.id)
host_esx_archived.host_switches.create!(:host_id => host_esx_archived.id, :switch_id => host_switch_archived.id)
host_esx_archived.host_switches.create!(:host_id => host_esx_archived.id, :switch_id => dvswitch_archived.id)
host_redhat.host_switches.create!(:host_id => host_redhat.id, :switch_id => redhat_switch.id)

migrate

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

migration_context :up do
it "fixes options[:subject] classes that don't exist" do
notification1 = notification_stub.create!(:notification_type => notification_type_stub.create!)
notification1 = notification_stub.create!(:notification_type_id => notification_type_stub.create!.id)
notification1.update(
:options =>
<<~OPTIONS
Expand Down Expand Up @@ -35,7 +35,7 @@
end

it "Sets options[:subject] where the record no longer exists" do
notification1 = notification_stub.create!(:notification_type => notification_type_stub.create!)
notification1 = notification_stub.create!(:notification_type_id => notification_type_stub.create!.id)
notification1.update(
:options =>
<<~OPTIONS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
end

resource_pools = emss.map do |ems|
resource_pool_stub.create!(:ext_management_system => ems)
resource_pool_stub.create!(:ems_id => ems.id)
end

migrate
Expand All @@ -23,7 +23,7 @@

it "doesn't migrate resource pools from other providers" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AnotherManager::InfraManager")
respool = resource_pool_stub.create!(:ext_management_system => ems)
respool = resource_pool_stub.create!(:ems_id => ems.id)

migrate

Expand All @@ -38,7 +38,7 @@
end

resource_pools = emss.map do |ems|
resource_pool_stub.create!(:ext_management_system => ems, :type => "#{ems.type}::ResourcePool")
resource_pool_stub.create!(:ems_id => ems.id, :type => "#{ems.type}::ResourcePool")
end

migrate
Expand All @@ -50,7 +50,7 @@

it "doesn't migrate resource pools from other providers" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AnotherManager::InfraManager")
respool = resource_pool_stub.create!(:ext_management_system => ems)
respool = resource_pool_stub.create!(:ems_id => ems.id)

migrate

Expand Down
14 changes: 7 additions & 7 deletions spec/migrations/20191118191722_subclass_ems_folders_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
ext_management_system_stub.create!(:type => "ManageIQ::Providers::#{vendor}::InfraManager")
end

folders = emss.map { |ems| ems_folder_stub.create!(:ext_management_system => ems) }
datacenters = emss.map { |ems| ems_folder_stub.create!(:ext_management_system => ems, :type => "Datacenter") }
storage_clusters = emss.map { |ems| ems_folder_stub.create!(:ext_management_system => ems, :type => "StorageCluster") }
folders = emss.map { |ems| ems_folder_stub.create!(:ems_id => ems.id) }
datacenters = emss.map { |ems| ems_folder_stub.create!(:ems_id => ems.id, :type => "Datacenter") }
storage_clusters = emss.map { |ems| ems_folder_stub.create!(:ems_id => ems.id, :type => "StorageCluster") }

migrate

Expand All @@ -24,7 +24,7 @@
it "doesn't migrate an unrelated folder" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AutomationManager")
inventory_group = ems_folder_stub.create!(
:ext_management_system => ems,
:ems_id => ems.id,
:type => "ManageIQ::Providers::AutomationManager::InventoryGroup"
)

Expand All @@ -41,10 +41,10 @@
end

folders = emss.map do |ems|
ems_folder_stub.create!(:ext_management_system => ems, :type => "#{ems.type}::Folder")
ems_folder_stub.create!(:ems_id => ems.id, :type => "#{ems.type}::Folder")
end
datacenters = emss.map do |ems|
ems_folder_stub.create!(:ext_management_system => ems, :type => "#{ems.type}::Datacenter")
ems_folder_stub.create!(:ems_id => ems.id, :type => "#{ems.type}::Datacenter")
end

migrate
Expand All @@ -56,7 +56,7 @@
it "doesn't migrate an unrelated folder" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AutomationManager")
inventory_group = ems_folder_stub.create!(
:ext_management_system => ems,
:ems_id => ems.id,
:type => "ManageIQ::Providers::AutomationManager::InventoryGroup"
)

Expand Down
8 changes: 4 additions & 4 deletions spec/migrations/20191210163518_subclass_storages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
end

storages = emss.map do |ems|
storage_stub.create!(:ext_management_system => ems)
storage_stub.create!(:ems_id => ems.id)
end

migrate
Expand All @@ -23,7 +23,7 @@

it "doesn't migrate storages from other providers" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AnotherManager::InfraManager")
storage = storage_stub.create!(:ext_management_system => ems)
storage = storage_stub.create!(:ems_id => ems.id)

migrate

Expand All @@ -38,7 +38,7 @@
end

storages = emss.map do |ems|
storage_stub.create!(:ext_management_system => ems, :type => "#{ems.type}::Storage")
storage_stub.create!(:ems_id => ems.id, :type => "#{ems.type}::Storage")
end

migrate
Expand All @@ -50,7 +50,7 @@

it "doesn't migrate storages from other providers" do
ems = ext_management_system_stub.create!(:type => "ManageIQ::Providers::AnotherManager::InfraManager")
storage = storage_stub.create!(:ext_management_system => ems)
storage = storage_stub.create!(:ems_id => ems.id)

migrate

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
migration_context :up do
it "Fixes the STI class of Google Cloud Volumes" do
gce = ems_stub.create!(:type => "ManageIQ::Providers::Google::CloudManager")
volume = volume_stub.create!(:ext_management_system => gce, :type => nil)
volume = volume_stub.create!(:ems_id => gce.id, :type => nil)

migrate

Expand All @@ -17,7 +17,7 @@
it "Doesn't impact non-Google Cloud volumes" do
osp = ems_stub.create!(:type => "ManageIQ::Providers::Openstack::CloudManager")
volume = volume_stub.create!(
:ext_management_system => osp,
:ems_id => osp.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolume"
)

Expand All @@ -31,7 +31,7 @@
it "Fixes the STI class of Google Cloud Volumes" do
gce = ems_stub.create!(:type => "ManageIQ::Providers::Google::CloudManager")
volume = volume_stub.create!(
:ext_management_system => gce,
:ems_id => gce.id,
:type => "ManageIQ::Providers::Google::CloudManager::CloudVolume"
)

Expand All @@ -43,7 +43,7 @@
it "Doesn't impact non-Google Cloud volumes" do
osp = ems_stub.create!(:type => "ManageIQ::Providers::Openstack::CloudManager")
volume = volume_stub.create!(
:ext_management_system => osp,
:ems_id => osp.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolume"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@
cinder = ems_stub.create!(:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager", :parent_ems_id => osp.id)

cloud_volume = cloud_volume_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolume"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolume"
)
cloud_volume_snapshot = cloud_volume_snapshot_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeSnapshot"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeSnapshot"
)
cloud_volume_backup = cloud_volume_backup_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeBackup"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeBackup"
)
cloud_volume_type = cloud_volume_type_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeType"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::CloudManager::CloudVolumeType"
)

migrate
Expand All @@ -44,20 +44,20 @@
cinder = ems_stub.create!(:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager", :parent_ems_id => osp.id)

cloud_volume = cloud_volume_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolume"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolume"
)
cloud_volume_snapshot = cloud_volume_snapshot_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeSnapshot"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeSnapshot"
)
cloud_volume_backup = cloud_volume_backup_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeBackup"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeBackup"
)
cloud_volume_type = cloud_volume_type_stub.create!(
:ext_management_system => cinder,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeType"
:ems_id => cinder.id,
:type => "ManageIQ::Providers::Openstack::StorageManager::CinderManager::CloudVolumeType"
)

migrate
Expand Down
Loading

0 comments on commit 688323b

Please sign in to comment.