Force a lookup in EmsFolder #11632
Force a lookup in EmsFolder #11632
Conversation
|
@miq_bot add_label providers/vmware/infra, provisioning |
|
@miq-bot assign @gmcculloug |
|
cc - @agrare |
|
Nice, can you add a test case for this? Maybe pass in |
| folder_id = get_option(:placement_folder_name) | ||
| return EmsFolder.find_by(:id => folder_id) if folder_id | ||
| ems_folder = EmsFolder.find_by(:id => get_option(:placement_folder_name)) | ||
| return ems_folder unless ems_folder.nil? |
There was a problem hiding this comment.
Suggest using: return ems_folder if ems_folder
|
Would this be caused by passing an invalid folder ID? |
|
@agrare - Yes - (although there isn't definite proof yet that it has happened.) In that case a |
syncrou
force-pushed
the
find_folder_only_if_it_is_real
branch
3 times, most recently
from
ea4707c
to
6e4c880
Compare
| @@ -0,0 +1,51 @@ | |||
| describe ManageIQ::Providers::Vmware::InfraManager::Provision::Cloning do | |||
| context "#dest_folder" do | |||
| before(:each) do | |||
There was a problem hiding this comment.
Drop the (:each) as that is the default.
| describe ManageIQ::Providers::Vmware::InfraManager::Provision::Cloning do | ||
| context "#dest_folder" do | ||
| before(:each) do | ||
| @os = OperatingSystem.new(:product_name => 'Microsoft Windows') |
There was a problem hiding this comment.
Is there a reason you are not using FactoryGirl here?
There was a problem hiding this comment.
Didn't know about that factory. Fixed.
If anything comes back in :placement_folder_name we always do a find_by as opposed to assuming that the data passed back is always good. This resolves us from returning false positives https://www.pivotaltracker.com/story/show/131473779
syncrou
force-pushed
the
find_folder_only_if_it_is_real
branch
from
6e4c880
to
000e316
Compare
|
Checked commits syncrou/manageiq@88b41aa~...000e316 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 spec/models/manageiq/providers/vmware/infra_manager/provision/cloning_spec.rb
|
Force a lookup in EmsFolder (cherry picked from commit fc9a588)
|
Euwe Backport details: $ git log
commit 3dd9feed520606c48d1f56af734a15ae50f28e84
Author: Greg McCullough <gmccullo@redhat.com>
Date: Wed Oct 5 15:48:58 2016 -0400
Merge pull request #11632 from syncrou/find_folder_only_if_it_is_real
Force a lookup in EmsFolder
(cherry picked from commit fc9a588dc65a768f83ac2af4e7dd3f16267403df) |
This resolves an issue where we might return a false positive.
Links