Add rake script to export/import policies and policy profiles #15256
Conversation
|
@gmcculloug any feedback on this? |
| } | ||
| end | ||
|
|
||
| before(:each) do |
There was a problem hiding this comment.
before do is actually the same as before(:each) do, so we can drop the (:each).
| @export_dir = Dir.mktmpdir('miq_exp_dir') | ||
| end | ||
|
|
||
| after(:each) do |
There was a problem hiding this comment.
Same thing here with the (:each)
| before(:each) do | ||
| FactoryGirl.create(:miq_policy, policy_create_attrs) | ||
| FactoryGirl.create(:miq_policy_read_only, policy2_create_attrs) | ||
| @export_dir = Dir.mktmpdir('miq_exp_dir') |
There was a problem hiding this comment.
Extract this to a let instead of an instance variable.
| FileUtils.remove_entry(@export_dir) | ||
| end | ||
|
|
||
| it 'should export user policies to a given directory' do |
There was a problem hiding this comment.
Minor change, but I believe we've stopped prefixing the descriptions with "should", so this would be better written as something like:
it 'exports user policies to a given directory'
| expect(Dir[File.join(@export_dir, '**', '*')].count { |file| File.file?(file) }).to eq(1) | ||
| end | ||
|
|
||
| it 'should export all policies to a given directory' do |
There was a problem hiding this comment.
Same here with removing the 'should'
| } | ||
| end | ||
|
|
||
| before(:each) do |
There was a problem hiding this comment.
Same as the previous file, the (:each) is redundant.
| @export_dir = Dir.mktmpdir('miq_exp_dir') | ||
| end | ||
|
|
||
| after(:each) do |
| FileUtils.remove_entry @export_dir | ||
| end | ||
|
|
||
| it 'should export user policy sets to a given directory' do |
There was a problem hiding this comment.
Again with the description dropping the 'should' and adjusting the wording accordingly
| expect(YAML.safe_load(file_contents)).to eq(policy_set_export_attrs) | ||
| end | ||
|
|
||
| it 'should export all policy sets to a given directory' do |
| expect do | ||
| TaskHelpers::Imports::PolicySets.new.import(options) | ||
| end.to output.to_stderr | ||
| end |
There was a problem hiding this comment.
I would say to give this file the same treatment as the other import file.
| expect do | ||
| TaskHelpers::Imports::Policies.new.import(options) | ||
| end.to output.to_stderr | ||
| end |
There was a problem hiding this comment.
Hmm, when I was doing the review I had a comment here but it disappeared... I'll try again.
There was a problem hiding this comment.
When there are different options/parameters called to the method under test, we'd like to try to structure the tests somewhat as follows:
describe "#import" do
let(:options) { {:source => source} }
describe "when the source is a directory" do
let(:source) { data_dir }
it "imports all .yaml files within the specified directory" do
end
end
describe "when the source is a valid policy file" do
let(:source) { "#{data_dir}/#{policy_file}" }
it "imports all .yaml files within the specified directory" do
end
end
describe "when the source is an invalid policy file" do
let(:source) { "#{data_dir}/#{bad_policy_file}" }
it "outputs a message to stderr" do
end
end
end
|
@branic Hey Brant, just wanted to make sure that you were good with the changes I suggested, please let me know if you need any help or have any questions. Thanks. |
|
@eclarizio I think I'm good with the changes suggested. I haven't had time to look at them in detail yet. I'm hoping to be able to do that and make the updates in the next week or so. |
|
@eclarizio I finally was able to make the changes to the spec files. |
|
@eclarizio Are there any other changes you would like me to make on this PR? |
| # expect do | ||
| # TaskHelpers::Imports::PolicySets.new.import(options) | ||
| # end.to output.to_stderr | ||
| # end |
There was a problem hiding this comment.
@branic Any reason to keep these commented out tests? They should probably be removed then this PR should be all set.
|
@gmcculloug No reason to keep them. I just forgot to delete them before committing. They have now been removed. |
|
Some comments on commits https://github.com/branic/manageiq/compare/fe6e0b90cd67acebec7d4fb7297c6ad6dcd3dd61~...59bbd0fd60616584c4c2499cc42651fd0c00e0cb lib/task_helpers/imports/policies.rb
lib/task_helpers/imports/policy_sets.rb
|
|
Checked commits https://github.com/branic/manageiq/compare/fe6e0b90cd67acebec7d4fb7297c6ad6dcd3dd61~...59bbd0fd60616584c4c2499cc42651fd0c00e0cb with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 **
|
These rake scripts and classes provide functionality for exporting/importing of the following ManageIQ object types:
This PR uses the framework that was implemented for PR #14126 to export/import other ManageIQ object types.
These scripts are based on the CFME RH Consulting Scripts and are used by Red Hat consultants to enable storing customizations in Git and maintaining customizations between environments (e.g. dev/qa/prod) for an SDLC lifecycle.