From c55ad6fa6415fe94c2b4eb54a1ece98c49ae8f0e Mon Sep 17 00:00:00 2001 From: Alexandre Conrad-Dormoy Date: Wed, 16 Apr 2025 22:36:38 -0700 Subject: [PATCH 1/4] Revert "Revert "chore: test memoize"" This reverts commit 2936370afd32ef42aeaf9f857249f8b5cf385553. --- pycobertura/utils.py | 32 +++++++++++- tests/test_utils.py | 121 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 tests/test_utils.py diff --git a/pycobertura/utils.py b/pycobertura/utils.py index 492dc2bd..f5e80054 100644 --- a/pycobertura/utils.py +++ b/pycobertura/utils.py @@ -3,6 +3,7 @@ import re import fnmatch from functools import partial +import inspect # Add import from typing import List, Tuple, Union @@ -40,22 +41,51 @@ def add_to(self, arg): def __init__(self, func): self.target_func = func + # Store signature for later use + self._sig = inspect.signature(self.target_func) + def __get__(self, obj, objtype=None): if obj is None: return self.target_func + # Use partial to pre-bind the instance `obj` return partial(self, obj) def __call__(self, *args, **kw): + # `args[0]` is the instance pre-bound by `__get__` using `partial` target_self = args[0] + # The actual arguments passed to the method start from args[1:] + method_args = args[1:] + try: cache = target_self.__cache except AttributeError: cache = target_self.__cache = {} - key = (self.target_func, args[1:], frozenset(kw.items())) + + # Bind arguments to the function signature to normalize them + try: + # We pass target_self explicitly here because the signature includes 'self' + # but it's not part of the *args received by __call__ in this context + # (it was bound by __get__ and is args[0]) + bound_args = self._sig.bind(target_self, *method_args, **kw) + except TypeError: + # Handle cases where binding fails (e.g., incorrect arguments passed) + # In such cases, don't cache, just call the original function + # which will likely raise the TypeError again. + return self.target_func(*args, **kw) + + bound_args.apply_defaults() # Apply defaults for consistent keys + + # Create a hashable key from the bound arguments *excluding* 'self' + # Use tuple(bound_args.arguments.items()) which includes param names and values in order + # Skip the first item, which corresponds to 'self' + key_items = tuple(item for item in bound_args.arguments.items() if item[0] != 'self') + key = (self.target_func, key_items) + try: res = cache[key] except KeyError: + # Pass the original *args and **kw to the target function res = cache[key] = self.target_func(*args, **kw) return res diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 00000000..abf6b355 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,121 @@ +import pytest + +from pycobertura.utils import memoize + + +class TestMemoize: + def test_memoize_caches_method_results(self): + """Tests that the memoize decorator caches method results.""" + + class MyClass: + def __init__(self): + self.call_count = 0 + + @memoize + def my_method(self, arg1, arg2): + self.call_count += 1 + return arg1 + arg2 + + instance = MyClass() + + # First call - should execute the method + result1 = instance.my_method(1, 2) + assert result1 == 3 + assert instance.call_count == 1 + + # Second call with same args - should return cached result + result2 = instance.my_method(1, 2) + assert result2 == 3 + assert instance.call_count == 1 # Count should not increase + + # Call with different args - should execute the method again + result3 = instance.my_method(3, 4) + assert result3 == 7 + assert instance.call_count == 2 # Count should increase + + # Call again with the first set of args - should return cached result + result4 = instance.my_method(1, 2) + assert result4 == 3 + assert instance.call_count == 2 # Count should not increase + + # Call again with the second set of args - should return cached result + result5 = instance.my_method(3, 4) + assert result5 == 7 + assert instance.call_count == 2 # Count should not increase + + def test_memoize_different_instances(self): + """Tests that memoization is instance-specific.""" + + class MyClass: + def __init__(self): + self.call_count = 0 + + @memoize + def my_method(self, arg): + self.call_count += 1 + return arg * 2 + + instance1 = MyClass() + instance2 = MyClass() + + # Call on instance1 + result1_1 = instance1.my_method(5) + assert result1_1 == 10 + assert instance1.call_count == 1 + assert instance2.call_count == 0 + + # Call on instance2 with the same arg + result2_1 = instance2.my_method(5) + assert result2_1 == 10 + assert instance1.call_count == 1 + assert instance2.call_count == 1 # instance2 count increases + + # Call on instance1 again + result1_2 = instance1.my_method(5) + assert result1_2 == 10 + assert instance1.call_count == 1 # instance1 count stays the same + assert instance2.call_count == 1 + + def test_memoize_with_kwargs(self): + """Tests that memoization works with keyword arguments.""" + + class MyClass: + def __init__(self): + self.call_count = 0 + + @memoize + def my_method(self, a, b=0): + self.call_count += 1 + return a + b + + instance = MyClass() + + # Call with positional args + res1 = instance.my_method(1, b=2) + assert res1 == 3 + assert instance.call_count == 1 + + # Call with same args using kwargs + res2 = instance.my_method(a=1, b=2) + assert res2 == 3 + assert instance.call_count == 1 # Should be cached + + # Call with different kwargs order + res3 = instance.my_method(b=2, a=1) + assert res3 == 3 + assert instance.call_count == 1 # Should be cached + + # Call with different value for kwarg + res4 = instance.my_method(a=1, b=3) + assert res4 == 4 + assert instance.call_count == 2 # New call + + # Call using default value + res5 = instance.my_method(5) + assert res5 == 5 + assert instance.call_count == 3 # New call + + # Call again using default value + res6 = instance.my_method(5, b=0) + assert res6 == 5 + assert instance.call_count == 3 # Should be cached From 0dd30b2a1db332da32ab3be53fd599926b8dd6e4 Mon Sep 17 00:00:00 2001 From: Alexandre Conrad-Dormoy Date: Wed, 16 Apr 2025 22:38:06 -0700 Subject: [PATCH 2/4] black --- pycobertura/utils.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pycobertura/utils.py b/pycobertura/utils.py index f5e80054..9c348a5c 100644 --- a/pycobertura/utils.py +++ b/pycobertura/utils.py @@ -44,7 +44,6 @@ def __init__(self, func): # Store signature for later use self._sig = inspect.signature(self.target_func) - def __get__(self, obj, objtype=None): if obj is None: return self.target_func @@ -69,17 +68,19 @@ def __call__(self, *args, **kw): # (it was bound by __get__ and is args[0]) bound_args = self._sig.bind(target_self, *method_args, **kw) except TypeError: - # Handle cases where binding fails (e.g., incorrect arguments passed) - # In such cases, don't cache, just call the original function - # which will likely raise the TypeError again. - return self.target_func(*args, **kw) + # Handle cases where binding fails (e.g., incorrect arguments passed) + # In such cases, don't cache, just call the original function + # which will likely raise the TypeError again. + return self.target_func(*args, **kw) - bound_args.apply_defaults() # Apply defaults for consistent keys + bound_args.apply_defaults() # Apply defaults for consistent keys # Create a hashable key from the bound arguments *excluding* 'self' # Use tuple(bound_args.arguments.items()) which includes param names and values in order # Skip the first item, which corresponds to 'self' - key_items = tuple(item for item in bound_args.arguments.items() if item[0] != 'self') + key_items = tuple( + item for item in bound_args.arguments.items() if item[0] != "self" + ) key = (self.target_func, key_items) try: From 61cdcd9b0dde18a5b7bb1e7cc0f331e63c36635b Mon Sep 17 00:00:00 2001 From: Alexandre Conrad-Dormoy Date: Wed, 16 Apr 2025 22:39:56 -0700 Subject: [PATCH 3/4] pep8 --- pycobertura/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pycobertura/utils.py b/pycobertura/utils.py index 9c348a5c..0c1e25f8 100644 --- a/pycobertura/utils.py +++ b/pycobertura/utils.py @@ -76,8 +76,8 @@ def __call__(self, *args, **kw): bound_args.apply_defaults() # Apply defaults for consistent keys # Create a hashable key from the bound arguments *excluding* 'self' - # Use tuple(bound_args.arguments.items()) which includes param names and values in order - # Skip the first item, which corresponds to 'self' + # Use tuple(bound_args.arguments.items()) which includes param names + # and values in order Skip the first item, which corresponds to 'self' key_items = tuple( item for item in bound_args.arguments.items() if item[0] != "self" ) From dddb13f0acc791fd2adbcb713ef508bd7a603d4a Mon Sep 17 00:00:00 2001 From: Alexandre Conrad-Dormoy Date: Wed, 16 Apr 2025 22:47:03 -0700 Subject: [PATCH 4/4] Download master artifact --- .github/workflows/build-and-test-pycobertura.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/build-and-test-pycobertura.yml b/.github/workflows/build-and-test-pycobertura.yml index 79f8e473..14dcb1b4 100644 --- a/.github/workflows/build-and-test-pycobertura.yml +++ b/.github/workflows/build-and-test-pycobertura.yml @@ -31,6 +31,21 @@ jobs: - name: Test with tox run: tox + - name: Download coverage artifact from master (${{ matrix.python-version }}) + # Only run on PRs targeting master + if: github.event_name == 'pull_request' && github.base_ref == 'master' + uses: actions/download-artifact@v4 + with: + # Construct the specific artifact name using the target commit SHA + name: cobertura-coverage-${{ github.event.pull_request.base.sha }}-${{ matrix.python-version }} + # Specify the workflow, branch, and commit to find the artifact from the correct run + workflow: ${{ github.workflow }} # Assumes this workflow generated the artifact + branch: master + commit: ${{ github.event.pull_request.base.sha }} + # Specify where to download the artifact (optional, defaults to current dir) + path: base-coverage/ + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Upload coverage artifact if: ${{ github.event_name == 'push' }} uses: actions/upload-artifact@v4