Allon Mureinik has posted comments on this change.
Change subject: core: Re-enable AddDiskToVmCommandTest
......................................................................
Patch Set 1:
disagree.
many of these methods that wrap out external resources do hide implementation
details from the "higher" methods.
E.g.,
performImagesCheckS(vm) makes sense in the context of
checkIfImageDiskCanBeAdded(vm).
On the other hand, seeing this block in the middle of the method is hardly
readable:
List emptyList = Collections.emptyList()
ImagesHandler.PerformImagesChecks(vm,
getReturnValue().getCanDoActionMessages(),
vm.getstorage_pool_id(),
getStorageDomainId().getValue(),
false,
false,
false,
false,
true,
false,
false,
true,
emptyList)
Regarding the usefulness of the test itself - I agree it's hardly full, but it
does test logic here, specifically of the canDoAction.
--
To view, visit http://gerrit.ovirt.org/5392
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b061a6f381f4e19d6666b24140b0583a244a63d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
