Change in vdsm[master]: utils: add weakmethod helper

fromani at redhat.com fromani at redhat.com
Fri Jan 15 13:34:11 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: utils: add weakmethod helper
......................................................................


Patch Set 7:

(5 comments)

https://gerrit.ovirt.org/#/c/51865/7/tests/utilsTests.py
File tests/utilsTests.py:

Line 993:             pass
Line 994:         self.assertEqual(log.messages, [])
Line 995: 
Line 996: 
Line 997: class FakeVirDomain(object):
> This is not about VirDomain, so lets pick a more generic name like ObjectWi
Done
Line 998: 
Line 999:     def public(self, *args, **kw):
Line 1000:         return 'public', args, kw
Line 1001: 


Line 1006: @contextlib.contextmanager
Line 1007: def _debug_gc():
Line 1008:     flags = gc.get_debug()
Line 1009:     try:
Line 1010:         gc.set_debug(gc.DEBUG_UNCOLLECTABLE)
> We don't need this flag:
Done
Line 1011:         yield
Line 1012:     finally:
Line 1013:         gc.set_debug(flags)
Line 1014: 


Line 1021:     return wrapper
Line 1022: 
Line 1023: 
Line 1024: @expandPermutations
Line 1025: class WeakmethodTests(TestCaseBase):
> Please name tests TestXxx instead of XxxTest, so they work py.test without 
Done
Line 1026: 
Line 1027:     @permutations([
Line 1028:         # wrapper, leak_expected
Line 1029:         [_leaking_wrapper, True],


Line 1034:         # we need to check differences in gc.garbage
Line 1035:         # because gc.garbage is shared hidden state,
Line 1036:         # we don't want test to interfer with each other
Line 1037:         gc.collect()
Line 1038:         before = set(gc.garbage)
> Better clear gc.garbage before the test, or use the context manager to clea
done in the context manger _debug_gc
Line 1039: 
Line 1040:         with _debug_gc():
Line 1041:             obj = FakeVirDomain()
Line 1042:             obj.public = wrapper(obj.public)


Line 1043:             self.assertEquals(obj.public(), ("public", (), {}))
Line 1044:             del obj
Line 1045: 
Line 1046:         gc.collect()
Line 1047:         garbage = set(gc.garbage) - before
> We don't need this check. When leak is not expected, we can assert that:
Done with minimal differences with respect to what you suggested
Line 1048:         has_leak = FakeVirDomain in set(inst.__class__ for inst in garbage)
Line 1049:         self.assertEquals(has_leak, leak_expected)
Line 1050: 
Line 1051:     def test_raise_on_invalid_weakref(self):


-- 
To view, visit https://gerrit.ovirt.org/51865
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f26aa314e26142122e9f594275406cf7fbade98
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list