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

nsoffer at redhat.com nsoffer at redhat.com
Fri Jan 15 14:09:26 UTC 2016


Nir Soffer has posted comments on this change.

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


Patch Set 8:

(2 comments)

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

Line 1004: 
Line 1005: 
Line 1006: @contextlib.contextmanager
Line 1007: def _debug_gc():
Line 1008:     flags = gc.get_debug()
If you submit another version, it would be nice to call this saved_flags.
Line 1009:     saved_garbage = gc.garbage[:]
Line 1010:     del gc.garbage[:]
Line 1011:     try:
Line 1012:         gc.set_debug(0)


Line 1043:             gc.collect()
Line 1044:             has_leak = (
Line 1045:                 len(gc.garbage) == 1 and
Line 1046:                 ObjectWithDel == type(gc.garbage[0])
Line 1047:             )
This is not correct and less clear that it should be, and you are leaving the leaked object around, so it may sneak later into another test. We want to get rid of it, which can be done by braking the cycle, and removing it from gc.garbage (we already remove it).

How about:

    If leak_expected:
        assert len(gc.garbage) == 1
        assert ObjectWithDel == type(gc.garbage[0])
        gc.garbage[0].public = None  # break the cycle
    else:
        assert gc.garbage == []
Line 1048:             self.assertEquals(has_leak, leak_expected)
Line 1049: 
Line 1050:     def test_raise_on_invalid_weakref(self):
Line 1051:         obj = ObjectWithDel()


-- 
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: 8
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