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

Use shared tests from manageiq-providers-ansible_tower #15594

Merged
merged 3 commits into from
Jul 20, 2017

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jul 18, 2017

Depends on #15598
The spec files in ManageIQ can conflict and aren't updated as they are in the provider repo.

Based on ManageIQ/manageiq-providers-ansible_tower#10 and #15593

@bdunne
Copy link
Member Author

bdunne commented Jul 18, 2017

Test failures are fixed by #15598

@durandom
Copy link
Member

imho app/models/manageiq/providers/embedded_ansible should move to the tower provider as well.

This would make hacks like this and this obsolete

@Fryguy
Copy link
Member

Fryguy commented Jul 19, 2017

Embedded Ansible is the core team’s concern, not the Ansible Tower team's concern, so I am 👎 on merging the two. However, I feel the cassettes and shared examples should definitely live in the Tower repo. Embedded depends on Tower, not the other way around.

@@ -31,6 +31,9 @@ def image_path(path, options = {})
Dir[Rails.root.join("spec/shared/**/*.rb")].each { |f| require f }
# include the manageiq-gems-pending matchers
Dir[ManageIQ::Gems::Pending.root.join("spec/support/custom_matchers/*.rb")].each { |f| require f }
Vmdb::Plugins.instance.vmdb_plugins.each do |plugin|
Dir[plugin.root.join("spec/support/**/*.rb")].each { |f| require f }
end
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this. The core shouldn't load all of the plugin's support dirs because we don't run their tests, except in the specific case of the shared examples for Tower.


after do
VCR.configure { |c| c.cassette_library_dir = @old_library }
end
Copy link
Member

Choose a reason for hiding this comment

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

I think you can pass the cassette_dir as part of the use_cassette call, and then you don't need the before/after dance.

# To be extracted with embedded_ansible
require ManageIQ::Providers::AnsibleTower::Engine.root.join("spec/support/vcr_helper.rb").to_s
Dir[ManageIQ::Providers::AnsibleTower::Engine.root.join("spec/support/ansible_shared/**/*.rb")].each { |f| require f }

Copy link
Member

Choose a reason for hiding this comment

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

I am 👍 for this as it's a very prescriptive change, and is clearly labeled to be moved when embedded_ansible is extracted.

@bdunne bdunne changed the title [WIP] Use shared tests from manageiq-providers-ansible_tower Use shared tests from manageiq-providers-ansible_tower Jul 19, 2017
@bdunne bdunne removed the wip label Jul 19, 2017
@miq-bot
Copy link
Member

miq-bot commented Jul 19, 2017

Checked commits bdunne/manageiq@9efea43~...8fea1b3 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy Fryguy merged commit 24cc255 into ManageIQ:master Jul 20, 2017
@Fryguy Fryguy added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 20, 2017
@bdunne bdunne deleted the use_shared_tests branch July 20, 2017 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants