[WIP] Factory change removing abstract Vm

[kbrock]

Does it make sense to not use Vm/VmOrTemplate abstract classes for out tests?

Maybe we need to push/flush out dummy providers first?

Why

I was working on #23604 and was getting weird errors around a Vm in the database. It seemed odd to me that we would do this.

TODO:

• Prevent creating DummyProvider entries in production (done)
• add dummy provider to gems, and add factories
• point factory :vm_infra to Dummy::Vm
• change specs to point :vm_or_template and :vm to :vm_dummy
• change factory :vm* to only allow leafs. (Thinking we want to allow :vm_infra). This is similar to ems factories

[Fryguy]

@agrare Thoughts here?

[Fryguy]

we have a separate factory vm_vmware which already does this.

[kbrock]

I was thinking that vm, vm_or_template, and vm_vmware would be interchangeable. They instantiate a vm.

Or maybe they pick a random child? (though we need consistency with that and the corresponding ems)

I do like when tests link to "any vm or template".
But I don't think we should instantiate that class.

I couldn't declare the parent of vm to be vm_vmware since we needed it to still flow up to vm_or_template.

[agrare]

VmOrTemplate is a real class, not something abstract.

I don't think we should change factory :vm_or_template to return Vmware::InfraManager::Vm, I think we should change any specs that use FactoryBot.build(:vm_or_template) if that isn't what they really mean.

[kbrock]

Well, the tests don't care.
But that doesn't mean we should store :type => 'VmOrTemplate' in the database.

[kbrock]

@agrare Yea, so thinking we want to put in a Providers::Dummy::Vm here or something.

Thinking of doing this across all STI models - especially Host and Ems.
Just want your opinion before taking this on.

Does Dummy have both an Infra and a Container aspect? (Do we need Cloud too?)

Interestingly, this was the main issue with #23604 - once we stated that Vm is abstract-ish, the factories started blowing things up.

[kbrock]

Out of band with @agrare

• Don't like vm pointing to vmware specific
• Don't like using :vm_or_template in specs (33 of them)
• Don't like using :vm in specs (98 of them)
• Maybe :vm_infra is ok. and maybe it should point to a :vm_dummy.
• Like using vm_infra and stub_supports

moved TODO to top comment

[kbrock]

WIP: have a plan. probably a few PRs. After that, this should be good

[Fryguy]

• Don't like using :vm_or_template in specs (33 of them)
• Don't like using :vm in specs (98 of them)

These don't bother me as much if the operation being tested is generic enough that we don't need a subclass. For example, things like verifying column length validations - we don't need a specific VM to verify something like that.

[miq-bot]

Checked commit kbrock@d1d8c1e with ruby 3.1.7, rubocop 1.56.3, haml-lint 0.64.0, and yamllint
3 files checked, 1 offense detected

spec/factories/vm_or_template.rb

• ❗ - Line 21, Col 86 - Layout/ExtraSpacing - Unnecessary spacing detected.

[miq-bot]

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).