Change in vdsm[master]: tests: lib: add tests for utils.weakmethod

nsoffer at redhat.com nsoffer at redhat.com
Sat Jan 23 16:29:14 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: tests: lib: add tests for utils.weakmethod
......................................................................


Patch Set 2:

(10 comments)

See the draft at https://gerrit.ovirt.org/52643 for possible way to clean up the tests.

https://gerrit.ovirt.org/#/c/52408/2/tests/utilsTests.py
File tests/utilsTests.py:

Line 846:     def public(self, *args, **kw):
Line 847:         return 'public', args, kw
Line 848: 
Line 849:     def __del__(self):
Line 850:         print('__del__', self.__class__.__name__)
Need to import print_function from __future__
Line 851: 
Line 852: 
Line 853: @contextlib.contextmanager
Line 854: def _debug_gc():


Line 850:         print('__del__', self.__class__.__name__)
Line 851: 
Line 852: 
Line 853: @contextlib.contextmanager
Line 854: def _debug_gc():
Lets disable gc during the tests.

Also we can simplify by eliminating this and using setUp() and tearDown()
Line 855:     saved_flags = gc.get_debug()
Line 856:     try:
Line 857:         gc.set_debug(0)
Line 858:         yield


Line 870:         """
Line 871:         for idx, _ in enumerate(gc.garbage):
Line 872:             if type(gc.garbage[idx]) == ObjectWithDel:
Line 873:                 gc.garbage[idx].public = None  # break the cycle
Line 874:                 del gc.garbage[idx]  # need to do manually
This loop is correct only for the first removal. The next removal will remove the wrong object since the list is reduced by one item on each removal.

    >>> lst = [1, 2, 3, 4]
    >>> for i, item in enumerate(lst): del lst[i]
    ... 
    >>> lst
    [2, 4]

Also, gc.garbage may change in the middle of the loop, possibly raising.

We can do:

    for obj in gc.garbage[:]:
        if type(obj) is ObjectWithDel:
            obj.public = None
            gc.garbage.remove(obj)
Line 875: 
Line 876:         gc.collect()
Line 877:         self.assertNotIn(ObjectWithDel,
Line 878:                          (type(obj) for obj in gc.garbage))


Line 874:                 del gc.garbage[idx]  # need to do manually
Line 875: 
Line 876:         gc.collect()
Line 877:         self.assertNotIn(ObjectWithDel,
Line 878:                          (type(obj) for obj in gc.garbage))
First in one line
Line 879: 
Line 880:     def test_with_reference_cycle(self):
Line 881: 
Line 882:         def _leaking_wrapper(meth):


Line 877:         self.assertNotIn(ObjectWithDel,
Line 878:                          (type(obj) for obj in gc.garbage))
Line 879: 
Line 880:     def test_with_reference_cycle(self):
Line 881: 
Unneeded blank line (pep8 does not enforce this)
Line 882:         def _leaking_wrapper(meth):
Line 883: 
Line 884:             def wrapper(*args, **kwargs):
Line 885:                 return meth(*args, **kwargs)


Line 879: 
Line 880:     def test_with_reference_cycle(self):
Line 881: 
Line 882:         def _leaking_wrapper(meth):
Line 883: 
Unneeded blank line (pep8 does not enforce this)
Line 884:             def wrapper(*args, **kwargs):
Line 885:                 return meth(*args, **kwargs)
Line 886: 
Line 887:             return wrapper


Line 882:         def _leaking_wrapper(meth):
Line 883: 
Line 884:             def wrapper(*args, **kwargs):
Line 885:                 return meth(*args, **kwargs)
Line 886: 
Unneeded blank line (pep8 does not enforce this)
Line 887:             return wrapper
Line 888: 
Line 889:         garbage_before = set(gc.garbage)
Line 890:         with _debug_gc():


Line 895: 
Line 896:         gc.collect()
Line 897:         garbage = set(gc.garbage) - garbage_before
Line 898:         self.assertIn(ObjectWithDel,
Line 899:                       (type(obj) for obj in garbage))
Fits in one line
Line 900: 
Line 901:     def test_without_reference_cycle(self):
Line 902: 
Line 903:         garbage_before = set(gc.garbage)


Line 899:                       (type(obj) for obj in garbage))
Line 900: 
Line 901:     def test_without_reference_cycle(self):
Line 902: 
Line 903:         garbage_before = set(gc.garbage)
This may fail with:

    TypeError: unhashable type: 'dict'

We must work with garbage as is, there is no chance that it will contain ObjectWithDel before the test, but to ensure that we can check and skip the test.
Line 904:         with _debug_gc():
Line 905:             obj = ObjectWithDel()
Line 906:             obj.public = utils.weakmethod(obj.public)
Line 907:             self.assertEquals(obj.public(), ("public", (), {}))


Line 909: 
Line 910:         gc.collect()
Line 911:         garbage = set(gc.garbage) - garbage_before
Line 912:         self.assertNotIn(ObjectWithDel,
Line 913:                          (type(obj) for obj in garbage))
Fits in one line
Line 914: 
Line 915:     def test_raise_on_invalid_weakref(self):
Line 916:         obj = ObjectWithDel()
Line 917:         method = utils.weakmethod(obj.public)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f0b8642f3fafef28a76ee9cfaa481a73ddaf6aa
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list