From 9bc8dc9c474fa347968469899ea36cd624e4f3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Lipi=C5=84ski?= Date: Mon, 4 Jan 2021 20:03:40 +0100 Subject: [PATCH 1/4] WIP: porting tests to py3 --- src/diagnose/test_fixtures.py | 1 + tests/test_instruments.py | 7 ++----- tests/test_probes.py | 31 +++++++++++++++++-------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/diagnose/test_fixtures.py b/src/diagnose/test_fixtures.py index ffb420b..0afd6d7 100644 --- a/src/diagnose/test_fixtures.py +++ b/src/diagnose/test_fixtures.py @@ -1,4 +1,5 @@ import functools +from six.moves import xrange import diagnose diff --git a/tests/test_instruments.py b/tests/test_instruments.py index 862169b..a038383 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -1,16 +1,13 @@ import datetime -from cStringIO import StringIO import sys - from mock import call, patch +from six.moves import cStringIO, StringIO import diagnose from diagnose import probes from diagnose.test_fixtures import a_func, Thing - from . import ProbeTestCase - registry = {} @@ -211,7 +208,7 @@ def test_replace_instrument(self): spec["target"] = target2 mgr.apply() assert ( - probes.active_probes[target2].instruments.values()[0].name + list(probes.active_probes[target2].instruments.values())[0].name == "a_func" ) # The old target MUST be removed from the probes diff --git a/tests/test_probes.py b/tests/test_probes.py index bfc9f83..3b72e8f 100644 --- a/tests/test_probes.py +++ b/tests/test_probes.py @@ -1,11 +1,11 @@ -from collections import defaultdict import datetime import gc import sys import time import types - +from collections import defaultdict from mock import patch +from six.moves import xrange try: from mock import _patch as MockPatch @@ -42,7 +42,7 @@ def test_return_event_result(self): assert result == "" # The probe MUST have logged an entry - assert p.instruments.values()[0].results == [""] + assert list(p.instruments.values())[0].results == [""] def test_return_event_elapsed(self): with self.probe( @@ -55,7 +55,7 @@ def test_return_event_elapsed(self): assert result == "" # The probe MUST have logged an entry - assert p.instruments.values()[0].results[0] < elapsed + assert list(p.instruments.values())[0].results[0] < elapsed def test_return_event_locals(self): with self.probe( @@ -66,7 +66,7 @@ def test_return_event_locals(self): assert result == "" # The probe MUST have logged an entry - assert p.instruments.values()[0].results == [ + assert list(p.instruments.values())[0].results == [ [ "arg", "args", @@ -132,14 +132,14 @@ def test_call_event_args(self): assert result == "" # The probe MUST have logged an entry - assert p.instruments.values()[0].results == [(t, "ok")] + assert list(p.instruments.values())[0].results == [(t, "ok")] def test_call_event_elapsed(self): with self.probe( "test", "do", "diagnose.test_fixtures.Thing.do", "elapsed", event="call" ) as p: errs = [] - p.instruments.values()[0].handle_error = lambda probe: errs.append( + list(p.instruments.values())[0].handle_error = lambda probe: errs.append( sys.exc_info()[1].args[0] if sys.exc_info()[1].args else "" ) result = Thing().do("ok") @@ -147,7 +147,7 @@ def test_call_event_elapsed(self): assert result == "" # The probe MUST NOT have logged an entry... - assert p.instruments.values()[0].results == [] + assert list(p.instruments.values())[0].results == [] # ...but the instrument MUST have handled the error: assert errs == ["name 'elapsed' is not defined"] @@ -164,7 +164,7 @@ def test_call_event_locals(self): assert result == "" # The probe MUST have logged an entry - assert p.instruments.values()[0].results == [ + assert list(p.instruments.values())[0].results == [ ["arg", "args", "frame", "kwargs", "now", "self", "start"] ] @@ -315,6 +315,9 @@ def test_probe_bad_mock(self): p = probes.attach_to("diagnose.test_fixtures.Thing.notamethod") with self.assertRaises(AttributeError) as exc: p.start() + print(exc.exception) + print(exc.exception.args) + print(exc.exception) assert ( exc.exception.args[0] == "diagnose.test_fixtures.Thing does not have the attribute 'notamethod'" @@ -418,7 +421,7 @@ def test_function_registries(self): assert funcs["orig"]("ahem") == "aha!" # The probe MUST have logged an entry - i = p.instruments.values()[0] + i = list(p.instruments.values())[0] assert i.results == ["aha!"] i.log = [] @@ -440,7 +443,7 @@ def test_patch_staticmethod(self): "test", "quantile", "diagnose.test_fixtures.Thing.static", "result" ) as p: assert Thing().static() == 15 - assert p.instruments.values()[0].results == [15] + assert list(p.instruments.values())[0].results == [15] def test_patch_wrapped_function_end_event(self): probe = probes.attach_to("diagnose.test_fixtures.Thing.add5") @@ -471,7 +474,7 @@ def test_patch_property(self): "test", "quantile", "diagnose.test_fixtures.Thing.exists", "result" ) as p: assert Thing().exists is True - assert p.instruments.values()[0].results == [True] + assert list(p.instruments.values())[0].results == [True] assert Thing.exists is not old_prop assert Thing().exists is True @@ -493,10 +496,10 @@ def only_some_users(probe, instrument, *args, **kwargs): custom={"valid_ids": [1, 2, 3]}, ) as p: assert Thing().do("ok", user_id=2) == "" - assert p.instruments.values()[0].results == [""] + assert list(p.instruments.values())[0].results == [""] assert Thing().do("not ok", user_id=10004) == "" - assert p.instruments.values()[0].results == [""] + assert list(p.instruments.values())[0].results == [""] class TestHardcodedProbes(ProbeTestCase): From 6129b458a6de9a57f22c54e907e8044de0f6139b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Lipi=C5=84ski?= Date: Mon, 4 Jan 2021 21:57:08 +0100 Subject: [PATCH 2/4] Add porting tests to py3 --- tests/test_probes.py | 56 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/tests/test_probes.py b/tests/test_probes.py index 3b72e8f..cbeaebf 100644 --- a/tests/test_probes.py +++ b/tests/test_probes.py @@ -4,6 +4,8 @@ import time import types from collections import defaultdict + +import six from mock import patch from six.moves import xrange @@ -261,7 +263,7 @@ def test_slowest_line(self): ) assert hard_work(0, 10000) == 1000 assert [tags for tags, value in i.log] == [ - ["source:34: summary = len([x for x in output if x % 10 == 0])\n"] + ["source:35: summary = len([x for x in output if x % 10 == 0])\n"] ] assert [type(value) for tags, value in i.log] == [float] finally: @@ -315,12 +317,15 @@ def test_probe_bad_mock(self): p = probes.attach_to("diagnose.test_fixtures.Thing.notamethod") with self.assertRaises(AttributeError) as exc: p.start() - print(exc.exception) - print(exc.exception.args) - print(exc.exception) + + if six.PY2: + expected_message = "diagnose.test_fixtures.Thing does not have the attribute 'notamethod'" + else: + expected_message = " does not have the attribute 'notamethod'" + assert ( exc.exception.args[0] - == "diagnose.test_fixtures.Thing does not have the attribute 'notamethod'" + == expected_message ) def test_target_copies(self): @@ -380,20 +385,32 @@ class Entity(object): # The next problem is that, while our patch is live, # if t2 goes out of its original scope, we've still got # a reference to it in our mock patch. - self.assertEqual( - owner_types(func_2), - { + if six.PY2: + expected_result = { types.ModuleType: 2, Entity: 2, probes.WeakMethodPatch: 3, MockPatch: 1, probes.DictPatch: 1, - }, - ) - del t2 + } + else: + expected_result = defaultdict(int, { + types.FunctionType: 1, + types.ModuleType: 2, + MockPatch: 1, + diagnose.probes.WeakMethodPatch: 4, + diagnose.probes.DictPatch: 1, + Entity: 2 + }) + self.assertEqual( owner_types(func_2), - { + expected_result, + ) + del t2 + + if six.PY2: + expected_result = { types.ModuleType: 2, # The number of Entity references MUST decrease by 1. Entity: 1, @@ -401,7 +418,20 @@ class Entity(object): probes.WeakMethodPatch: 2, MockPatch: 1, probes.DictPatch: 1, - }, + } + else: + expected_result = defaultdict(int, { + types.FunctionType: 1, + types.ModuleType: 2, + MockPatch: 1, + diagnose.probes.WeakMethodPatch: 3, + diagnose.probes.DictPatch: 1, + Entity: 1 + }) + + self.assertEqual( + owner_types(func_2), + expected_result, ) finally: probe.stop() From d275814379daea3d3292636ce5df29d2e5cfb7d2 Mon Sep 17 00:00:00 2001 From: Robert Brewer Date: Tue, 5 Jan 2021 11:55:16 -0800 Subject: [PATCH 3/4] probes: avoid patching probe_wrapper.__wrapped__ with itself in py3 --- src/diagnose/probes.py | 12 +++++- tests/test_probes.py | 85 ++++++++++++++++++------------------------ 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/diagnose/probes.py b/src/diagnose/probes.py index 3c57b1d..a5238dc 100644 --- a/src/diagnose/probes.py +++ b/src/diagnose/probes.py @@ -284,6 +284,11 @@ def probe_wrapper(*args, **kwargs): for parent in gc.get_referrers(ref): if parent is _resolved_target or parent is primary_patch: continue + if parent is probe_wrapper: + # In Python 3.2+, `@functools.wraps(base)` above sets + # `wrapper.__wrapped__ = wrapped`. We don't want to + # patch that with itself! + continue if getattr(parent, "__dict__", None) is ref: # An attribute of a module or class or instance. @@ -451,7 +456,7 @@ class WeakMethodPatch(object): then replaces the given attribute of that object with the new value. On stop/__exit__, replaces the same attribute with the previous value. - Used by FunctionPatch to replace references to functions which appear in + Used by FunctionProbe to replace references to functions which appear in modules, classes, or other objects. Weak references are used internally so that, if the object is removed from that module etc (has no more strong references), then the patch is automatically abandoned. @@ -462,6 +467,9 @@ def __init__(self, getter, attribute, new): self.attribute = attribute self.new = new + def __repr__(self): + return "%s(%s, %s, %s)" % (self.__class__.__name__, self.getter, self.attribute, self.new) + def get_original(self): target = self.getter() name = self.attribute @@ -529,7 +537,7 @@ class DictPatch(object): identified by the given key with a new object. On stop/__exit__, replaces the same key with the previous object. - Used by FunctionPatch to replace references to functions which appear + Used by FunctionProbe to replace references to functions which appear in any dictionary, such as a function registry. """ diff --git a/tests/test_probes.py b/tests/test_probes.py index cbeaebf..1e60c5a 100644 --- a/tests/test_probes.py +++ b/tests/test_probes.py @@ -309,7 +309,7 @@ def owner_types(obj): if getattr(parent, "__dict__", None) is ref: num_instances[type(parent)] += 1 break - return num_instances + return dict(num_instances) class TestTargets(ProbeTestCase): @@ -350,6 +350,17 @@ class Entity(object): registry["in_a_dict"] = func_2 self.assertTrue(registry["in_a_dict"] is old_local_func_2) + # Before attaching the probe, there should be some references to func_2, + # but not our patch objects. + expected_result = { + types.ModuleType: 2, + Entity: 2 + } + self.assertEqual( + owner_types(func_2), + expected_result, + ) + probe = probes.attach_to("diagnose.test_fixtures.func_2") try: probe.start() @@ -385,54 +396,32 @@ class Entity(object): # The next problem is that, while our patch is live, # if t2 goes out of its original scope, we've still got # a reference to it in our mock patch. - if six.PY2: - expected_result = { - types.ModuleType: 2, - Entity: 2, - probes.WeakMethodPatch: 3, - MockPatch: 1, - probes.DictPatch: 1, - } - else: - expected_result = defaultdict(int, { - types.FunctionType: 1, - types.ModuleType: 2, - MockPatch: 1, - diagnose.probes.WeakMethodPatch: 4, - diagnose.probes.DictPatch: 1, - Entity: 2 - }) - - self.assertEqual( - owner_types(func_2), - expected_result, - ) + expected_result = { + # These referred to func_2 before our probe was attached... + types.ModuleType: 2, + Entity: 2, + # ...and these are added by attaching the probe: + # a) the target that we passed to probes.attach_to() + MockPatch: 1, + # b) 3 "methods": t.add13, t2.add13, and test_probes.func_2 + probes.WeakMethodPatch: 3, + # c) the registry dict. + probes.DictPatch: 1, + } + assert owner_types(func_2) == expected_result + + # Delete one of our references. del t2 - - if six.PY2: - expected_result = { - types.ModuleType: 2, - # The number of Entity references MUST decrease by 1. - Entity: 1, - # The number of WeakMethodPatch references MUST decrease by 1. - probes.WeakMethodPatch: 2, - MockPatch: 1, - probes.DictPatch: 1, - } - else: - expected_result = defaultdict(int, { - types.FunctionType: 1, - types.ModuleType: 2, - MockPatch: 1, - diagnose.probes.WeakMethodPatch: 3, - diagnose.probes.DictPatch: 1, - Entity: 1 - }) - - self.assertEqual( - owner_types(func_2), - expected_result, - ) + expected_result = { + types.ModuleType: 2, + # The number of Entity references MUST decrease by 1. + Entity: 1, + MockPatch: 1, + # The number of WeakMethodPatch references MUST decrease by 1. + probes.WeakMethodPatch: 2, + probes.DictPatch: 1, + } + assert owner_types(func_2) == expected_result finally: probe.stop() From eeb52a66c09ee796485effcdc16d212b43cc4e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Lipi=C5=84ski?= Date: Thu, 7 Jan 2021 11:27:29 +0100 Subject: [PATCH 4/4] Clean code --- tests/test_instruments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_instruments.py b/tests/test_instruments.py index a038383..b808892 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -208,7 +208,7 @@ def test_replace_instrument(self): spec["target"] = target2 mgr.apply() assert ( - list(probes.active_probes[target2].instruments.values())[0].name + next(iter(probes.active_probes[target2].instruments.values())).name == "a_func" ) # The old target MUST be removed from the probes