On Wed, May 30, 2012 at 11:11:35AM -0500, Ryan Harper wrote: > * Dan Kenigsberg <dan...@redhat.com> [2012-05-30 10:43]: > > On Tue, May 29, 2012 at 09:07:25PM -0500, Ryan Harper wrote: > > > * dan...@redhat.com <dan...@redhat.com> [2012-05-29 09:29]: > > > > Dan Kenigsberg has posted comments on this change. > > > > > > > > Change subject: tests: fix gluster storage exception test > > > > ...................................................................... > > > > > > > > > > > > Patch Set 1: Looks good to me, approved > > > > > > > > Thanks Ryan. Though Python has the lovely > > > > > > > > 4100 <= obj.code <= 4800 > > > > > > Thanks, but I'm well aware of the python syntax. > > > > > > I've been requested in the past[1] to be more verbose in the asserts so I > > > figured I'd break it out into to separate asserts. Is there a general > > > guideline we're to follow when it comes to the test cases? I'd be happy > > > to follow. > > > > > > 1. http://gerrit.ovirt.org/#patch,unified,4323,3,tests/netinfoTests.py > > > > The point is that when your current assertion explode, we still do not > > get infomation on the actual value of obj.code which broke it - just > > as with the pythonic syntactic sugar above. I suppose assertGreater() > > would have been more helpful - but I couldn't say no for a quick fix to > > a broken unit test! > > the assertTrue with range check doesn't either: > > Traceback (most recent call last): > File "/root/work/git/clean/vdsm/tests/main.py", line 72, in test_collisions > self.assertTrue(5000 <= obj.code <= 6000) > AssertionError: False is not true > > But the assertGreater does: > > ====================================================================== > FAIL: test_collisions (main.TestGlusterExceptions) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/root/work/git/clean/vdsm/tests/main.py", line 72, in test_collisions > self.assertGreater(obj.code, 5000) > AssertionError: 4115 not greater than 5000 > > Would you be interested in conversion of the assertTrue/False to > Greater/Lesser > etc in tests/*.py ?
It's not something I'd fight for, but yes, that's an improvement. _______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-devel