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

Support more Tower credential types #15558

Closed
wants to merge 4 commits into from

Conversation

jameswnl
Copy link
Contributor

@jameswnl jameswnl commented Jul 12, 2017

Get the other Tower supported credential types into Embedded_ansible as well.

Plus using the shared examples and cassettes from manageiq-providers-ansible-towr and removing the duplicated set in manageiq

@@ -113,7 +117,7 @@ def image_path(path, options = {})
end

VCR.configure do |c|
c.cassette_library_dir = Rails.root.join('spec/vcr_cassettes')
c.cassette_library_dir = File.join(external_tower_gem_dir, 'spec/vcr_cassettes')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@durandom @Fryguy This is the part I am not sure about.
This is pulling in only cassettes from the external tower repo's. Haven't been able to find ways to make vcr load from multiple cassettes paths because cassette_library_dir can be only 1. Need some advice here.

Good thing is: currently it's ok because manageiq specs don't use any other cassettes. And once we split the embedded code, it would be even less to worry about.

Copy link
Member

Choose a reason for hiding this comment

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

another reason why the specs shouldnt rely on an external gem.
maybe you can wrap the it_behaves_like block with a VCR block? Not sure if thats possible and supported though, something like

VCR.with_cassette_library_dir('..') do
  it_behaves_like '...'
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 like my solution better... https://github.com/ManageIQ/manageiq/pull/15594/files#diff-88d3c9989ff613ced10a9fe635834148R2
This change breaks everything else that uses VCR in ManageIQ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes 👍
I was looking for answer like yours. Will incorporate.

@jameswnl
Copy link
Contributor Author

@miq-bot add_labels enhancement, fine/yes

@@ -32,6 +32,10 @@ def image_path(path, options = {})
# include the manageiq-gems-pending matchers
Dir[ManageIQ::Gems::Pending.root.join("spec/support/custom_matchers/*.rb")].each { |f| require f }

# For embedded ansible to use external ansible tower's specs
external_tower_gem_dir = Gem::Specification.find_by_name("manageiq-providers-ansible_tower").gem_dir
Dir[File.join(external_tower_gem_dir, 'spec/support/**/*.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.

somehow it feels wrong to include shared examples from an external gem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so duplicate it?

Copy link
Member

Choose a reason for hiding this comment

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

@Fryguy @blomquisg how about moving the embedded ansible specs and code to the tower provider gem? I mean for the foreseeable future embedded will be very much like a tower provider

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, probably a better compromise and much easier to maintain.

@@ -113,7 +117,7 @@ def image_path(path, options = {})
end

VCR.configure do |c|
c.cassette_library_dir = Rails.root.join('spec/vcr_cassettes')
c.cassette_library_dir = File.join(external_tower_gem_dir, 'spec/vcr_cassettes')
Copy link
Member

Choose a reason for hiding this comment

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

another reason why the specs shouldnt rely on an external gem.
maybe you can wrap the it_behaves_like block with a VCR block? Not sure if thats possible and supported though, something like

VCR.with_cassette_library_dir('..') do
  it_behaves_like '...'
end

@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2017

This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2017

Checked commits jameswnl/manageiq@8914aaf~...8fe5587 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 1 offense detected

spec/spec_helper.rb

@Fryguy
Copy link
Member

Fryguy commented Jul 19, 2017

I merged the various "fixes" in the separate PR #15598 since they are unrelated this PR.

@miq-bot
Copy link
Member

miq-bot commented Jul 19, 2017

This pull request is not mergeable. Please rebase and repush.

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.

6 participants