Allon Mureinik has posted comments on this change. Change subject: core: Removed useless code from test ......................................................................
Patch Set 3: Code-Review-1 (4 comments) This patch seems to leave unused variables in the code, which is probably not the intention of this patch. Giving -1 for visibility - they should either be removed, or explained why they are needed. http://gerrit.ovirt.org/#/c/35458/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java: Line 191: } Line 192: Line 193: @Test Line 194: public void canDoActionFailedUpdateReadOnly() { Line 195: VmDevice device = createVmDevice(diskImageGuid, vmId); So can't we completely remove this device now? It seems unsued Line 196: Line 197: // make sure that device is plugged Line 198: assertEquals(true, device.getIsPlugged()); Line 199: Line 413: } Line 414: }); Line 415: Line 416: initializeCommand(parameters); Line 417: VmDevice device = createVmDevice(diskImageGuid, vmId); Can we remove this statement now? The device seems unused Line 418: command.executeVmCommand(); Line 419: Line 420: // verify that device address was cleared exactly once Line 421: verify(vmDeviceDAO, times(1)).clearDeviceAddress(device.getDeviceId()); Line 428: parameters.getDiskInfo().setReadOnly(true); Line 429: Line 430: when(diskDao.get(diskImageGuid)).thenReturn(createDiskImage()); Line 431: Line 432: VmDevice device = createVmDevice(diskImageGuid, vmId); This device seems unused. Can it be removed? Line 433: initializeCommand(parameters); Line 434: command.executeVmCommand(); Line 435: Line 436: device.setIsReadOnly(true); Line 473: Line 474: when(diskDao.get(diskImageGuid)).thenReturn(createDiskImage()); Line 475: Line 476: // Disk is already attached to VM as a read write Line 477: VmDevice device = createVmDevice(diskImageGuid, vmId); This device seems to be unused. Can it be removed? Line 478: Line 479: // To be sure that readOnly property is not changed Line 480: assertEquals(device.getIsReadOnly(), parameters.getDiskInfo().getReadOnly()); Line 481: -- To view, visit http://gerrit.ovirt.org/35458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If1580f66cc5a41583abbc936308946ea3b70939f Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
