Allon Mureinik has posted comments on this change.
Change subject: core: validate that lun has a valid LunType
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(5 inline comments)
A couple of general issues:
1. Convention - start tests with testXYZ, not checkXYZ
2. assertTrue/False/Whatever should be used with the optional message parameter
(i.e., instead of assertTrue(shouldWork()), use assertTrue("calling the
shouldWork method in this scenario failed", shouldWork())
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
Line 520: storage_server_connections connection = new
storage_server_connections();
Line 521: connection.setiqn("a");
Line 522: connection.setconnection("0.0.0.0");
Line 523: connection.setport("1234");
Line 524: ArrayList<storage_server_connections> connections = new
ArrayList<storage_server_connections>();
define as List, not ArrayList
Line 525: connections.add(connection);
Line 526: lun.setLunConnections(connections);
Line 527: disk.setLun(lun);
Line 528: return disk;
Line 521: connection.setiqn("a");
Line 522: connection.setconnection("0.0.0.0");
Line 523: connection.setport("1234");
Line 524: ArrayList<storage_server_connections> connections = new
ArrayList<storage_server_connections>();
Line 525: connections.add(connection);
why not just use a singletonList?
Line 526: lun.setLunConnections(connections);
Line 527: disk.setLun(lun);
Line 528: return disk;
Line 529: }
Line 558: disk.getLun().getLunConnections().get(0).setiqn(null);
Line 559: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 560:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
Line 561:
Line 562: clearCanDoActionMessages();
why do you need this? why not just another test?
Line 563:
Line 564: disk.getLun().getLunConnections().get(0).setiqn("");
Line 565: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 566:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
Line 575: disk.getLun().getLunConnections().get(0).setconnection(null);
Line 576: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 577:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
Line 578:
Line 579: clearCanDoActionMessages();
why do you need this? why not just another test?
Line 580:
Line 581: disk.getLun().getLunConnections().get(0).setconnection("");
Line 582: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 583:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
Line 592: disk.getLun().getLunConnections().get(0).setport(null);
Line 593: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 594:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
Line 595:
Line 596: clearCanDoActionMessages();
why do you need this? why not just another test?
Line 597:
Line 598: disk.getLun().getLunConnections().get(0).setport("");
Line 599: assertFalse(command.checkIfLunDiskCanBeAdded());
Line 600:
assertTrue(verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS));
--
To view, visit http://gerrit.ovirt.org/8437
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I302b399fd93419fb1f621fd315b50d5971ae9c11
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@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: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
