-
Notifications
You must be signed in to change notification settings - Fork 141
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
Post body changes for validate_vms_resource
to include service template_id
#486
Post body changes for validate_vms_resource
to include service template_id
#486
Conversation
this is the edit ServiceTemplate record use case where we need to specify the id of the ServiceTemplate record
@miq-bot add_label hammer/yes |
@bdunne Can you review this? ManageIQ/manageiq#18065 was just merged after which I restarted Travis here. |
transformation_mapping = | ||
FactoryGirl.create(:transformation_mapping, | ||
:transformation_mapping_items => [TransformationMappingItem.new(:source => source_ems, :destination => destination_ems)]) | ||
vm = FactoryGirl.create(:vm_vmware, :name => 'test_vm', :ems_cluster => source_ems, :ext_management_system => FactoryGirl.create(:ext_management_system)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't set name
as there's a sequencer for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should one of the clusters above (assuming it's supposed to be a cluster) be connected to this ext_management_system
?
transformation_mapping = | ||
FactoryGirl.create(:transformation_mapping, | ||
:transformation_mapping_items => [TransformationMappingItem.new(:source => source_ems, :destination => destination_ems)]) | ||
vm = FactoryGirl.create(:vm_vmware, :name => 'test_vm', :ems_cluster => source_ems, :ext_management_system => FactoryGirl.create(:ext_management_system)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
context "can validate vms with csv data and service_template_id are specified" do | ||
it "vm belongs to the service_template record" do | ||
api_basic_authorize(action_identifier(:transformation_mappings, :validate_vms, :resource_actions, :post)) | ||
source_ems = FactoryGirl.create(:ems_cluster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be an EMS or a cluster? Please adjust the factory or variable name accordingly.
|
||
it "vm does not belong to the service_template record" do | ||
api_basic_authorize(action_identifier(:transformation_mappings, :validate_vms, :resource_actions, :post)) | ||
source_ems = FactoryGirl.create(:ems_cluster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Checked commits AparnaKarve/manageiq-api@9936e46~...569bc98 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@bdunne Addressed your feedback for improving readability in the tests. |
…_service_template_id Post body changes for `validate_vms_resource ` to include service template_id (cherry picked from commit 93b07cd)
Hammer backport details:
|
Dependent on ManageIQ/manageiq#18065
Fixes ManageIQ/manageiq-v2v#677
(Tests are expected to fail until ManageIQ/manageiq#18065 is merged)