Allon Mureinik has posted comments on this change.

Change subject: core: DiskImageDAO - introduce getAllSnapshotsForLeaf
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

Some gaps in the DAO test, but other than that, looks awesome.

http://gerrit.ovirt.org/#/c/31500/1/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDAOTest.java
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDAOTest.java:

Line 149:         assertTrue(result.isEmpty());
Line 150:     }
Line 151: 
Line 152:     @Test
Line 153:     public void testGetAllSnapshotsForLeaf() {
Please add a test that when called with an illegal Guid (e.g., Guid.newGuid()) 
it returns an empty result set.
Line 154:         List<DiskImage> images = 
dao.getAllSnapshotsForLeaf(FixturesTool.IMAGE_ID);
Line 155: 
Line 156:         assertFalse(images.isEmpty());
Line 157:     }


Line 151: 
Line 152:     @Test
Line 153:     public void testGetAllSnapshotsForLeaf() {
Line 154:         List<DiskImage> images = 
dao.getAllSnapshotsForLeaf(FixturesTool.IMAGE_ID);
Line 155: 
I'd add an assert that the template is not returned.
Line 156:         assertFalse(images.isEmpty());
Line 157:     }


-- 
To view, visit http://gerrit.ovirt.org/31500
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6068be44fd5e5c47eed2064d87f506d3e3be444
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@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

Reply via email to