Dan Kenigsberg has posted comments on this change.
Change subject: getAllVolumes() unit test. ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
.................................................... File tests/getAllVolumesTests.py Line 27: sys.path.append('/usr/share/vdsm') no playing with sys.path inside modules, please.
unit tests should not assume that vdsm is installed.
Line 33: class TestBlockGetAllVolumes(TestCaseBase): I'd rather have a more testable code to begin with but that's fine too.
Line 70: # Danger Will Robinson! Danger! right! eval is evil. why not edit glob.out and read it line by line?
.................................................... File tests/Makefile.am Line 39: getAllVolumesTests.py make loves tabs
-- To view, visit http://gerrit.ovirt.org/4592 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic7cfb77f7675a58858507acf2b708317eb30a43d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com