Dan Kenigsberg has posted comments on this change.

Change subject: itmap unit tests
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(5 inline comments)

....................................................
File tests/miscTests.py
Line 201:         def dummy(arg):
Line 202:             time.sleep(1)
Line 203:             return arg
Line 204:         data = set([1, 2, 3, 4])
Line 205:         ret = set(list(misc.itmap(dummy, data, 3)))
why's the list() conversion?

p.s. forzenset() fits better to this use case.
Line 206:         self.assertFalse(ret != data)
Line 207: 
Line 208:     def testHowLong(self):
Line 209:         def dummy(arg):


Line 202:             time.sleep(1)
Line 203:             return arg
Line 204:         data = set([1, 2, 3, 4])
Line 205:         ret = set(list(misc.itmap(dummy, data, 3)))
Line 206:         self.assertFalse(ret != data)
why not assertEquals (which is less convoluted and gives more info on failure)?
Line 207: 
Line 208:     def testHowLong(self):
Line 209:         def dummy(arg):
Line 210:             #  no sleep. (testing how long it takes to initiate the 
processes)


Line 208:     def testHowLong(self):
Line 209:         def dummy(arg):
Line 210:             #  no sleep. (testing how long it takes to initiate the 
processes)
Line 211:             return arg
Line 212:         import time
in-function imports are frowned upon.
Line 213:         data = (1, 2, 3, 4)
Line 214:         currentTime = time.time()
Line 215:         misc.itmap(dummy, data, 3)  # returns when all processes 
finished
Line 216:         afterTime = time.time()


Line 214:         currentTime = time.time()
Line 215:         misc.itmap(dummy, data, 3)  # returns when all processes 
finished
Line 216:         afterTime = time.time()
Line 217:         # Check if it took more than 2 sec
Line 218:         self.assertFalse(currentTime + 2 < afterTime)
assertLess is nicer.
Line 219: 
Line 220:     def testUnlimited(self):
Line 221:         def dummy(arg):
Line 222:             return arg


Line 219: 
Line 220:     def testUnlimited(self):
Line 221:         def dummy(arg):
Line 222:             return arg
Line 223:         # till we change constants we can run 10 processes 
simultaniously
which constants are these? how about using

 data = set(xrange(CONSTANT))
Line 224:         # this test runs all threads together and returns
Line 225:         data = set([1, 2, 3, 4, 5, 6, 7, 8, 9])
Line 226:         ret = set(list(misc.itmap(dummy, data, 
misc.UNLIMITED_THREADS)))
Line 227:         self.assertFalse(ret != data)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I26e258a035f49a8ecd57e992b9a68b9475a58839
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Hunt Xu <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to