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

Automate - Refactoring infra vm check_powered_off method. #108

Merged

Conversation

billfitzgerald0120
Copy link
Contributor

Refactoring the check_powered_off method for the infrastructure Vm. This PR is based on the issue below.

Created tests.

ManageIQ/manageiq#12038

@miq-bot add_label refactoring, fine/yes

@billfitzgerald0120 billfitzgerald0120 force-pushed the refactor_i_check_powered_off branch from 48ef476 to ac6f674 Compare May 4, 2017 14:27
@billfitzgerald0120
Copy link
Contributor Author

@tinaafitz
Please Review

@tinaafitz
Copy link
Member

@miq-bot remove_label fine/yes

@tinaafitz
Copy link
Member

@miq-bot add_label fine/no

@miq-bot miq-bot added fine/no and removed fine/yes labels May 4, 2017
@tinaafitz
Copy link
Member

@billfitzgerald0120 Looks good.
@mkanoor Please review.


context "no vm" do
let(:vm) { nil }
it_behaves_like "#ae_result nil"
Copy link
Contributor

Choose a reason for hiding this comment

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

@billfitzgerald0120 Should this be raising an error if the @handle.root['vm'] is nil as opposed to checking ae_result = nil


def main
# Get vm from root object
vm = @handle.root['vm']
Copy link
Contributor

Choose a reason for hiding this comment

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

@billfitzgerald0120
Can we write a function that returns the vm and raises an error if the vm is missing @handle.root[vm] is nil

Updated test to check error raised.
@miq-bot
Copy link
Member

miq-bot commented May 8, 2017

Checked commits billfitzgerald0120/manageiq-content@ac6f674~...2638af6 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@billfitzgerald0120
Copy link
Contributor Author

@mkanoor
Please Review

shared_examples_for "#vm power state" do
it "check" do
vm.update_attributes(:raw_power_state => raw_power_state)
svc_model_vm
Copy link
Contributor

Choose a reason for hiding this comment

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

@billfitzgerald0120
What does this line do? Is it trying to reload the service vm object. Should this be happening after the method has run?

end

let(:root_hash) do
{ 'vm' => MiqAeMethodService::MiqAeServiceVm.find(vm.id) }
Copy link
Contributor

Choose a reason for hiding this comment

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

@billfitzgerald0120
This could be just

{'vm' => svc_model_vm }

@mkanoor mkanoor merged commit 677ec5e into ManageIQ:master Jun 29, 2017
@mkanoor mkanoor added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 29, 2017
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.

4 participants