Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: testing dependent-destroy #52

Closed
wants to merge 1 commit into from
Closed

WIP: testing dependent-destroy #52

wants to merge 1 commit into from

Conversation

jameswnl
Copy link
Contributor

See the different behaviors of the 2 added specs.

@Fryguy
Copy link
Member

Fryguy commented Jan 10, 2018

I just don't get it...need to investigate more :/

@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2018

Checked commit https://github.com/jameswnl/manageiq-providers-ansible_tower/commit/686f742c136c8e7fa77e144d70199c077645bc25 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 2 offenses detected

spec/support/ansible_shared/provider.rb

@Ladas
Copy link
Contributor

Ladas commented Jan 10, 2018

the data are not stored properly

check

subject.automation_manager.reload.configured_systems

D, [2018-01-10T21:22:01.373714 #12596] DEBUG -- : ManageIQ::Providers::AutomationManager::ConfiguredSystem Load (0.3ms) SELECT "configured_systems".* FROM "configured_systems" WHERE "configured_systems"."type" IN ('ManageIQ::Providers::AutomationManager::ConfiguredSystem', 'ManageIQ::Providers::ExternalAutomationManager::ConfiguredSystem', 'ManageIQ::Providers::EmbeddedAutomationManager::ConfiguredSystem', 'ManageIQ::Providers::AnsibleTower::AutomationManager::ConfiguredSystem', 'ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfiguredSystem') AND "configured_systems"."manager_id" = $1 [["manager_id", 2952]]

The configured system needs to have valid STI type, otherwise the query will not find it. subject.automation_manager.configured_systems is not doing any query, it just shows cached data

@jameswnl
Copy link
Contributor Author

@Ladas see my fix here ManageIQ/manageiq#16798


it "thru automation_manager will remove all child objects" do
subject.automation_manager.configured_systems = [
FactoryGirl.create(:configured_system, :computer_system =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use different factory https://github.com/Ladas/manageiq/blob/2f0ef1a90758f2b5c37e93a0ed2f586a916dad64/spec/factories/configured_system.rb#L10

or the default factory should contain valid base class STI, in this case ManageIQ::Providers::AutomationManager::ConfiguredSystem


None of factories should be producing a record that can't be created in real life, e.g. ConfiguredSystem with type nil is not valid, refresh always set :type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ladas !

@jameswnl
Copy link
Contributor Author

thanks to @chrisarcand problem solved, closing this

@jameswnl jameswnl closed this Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants