Fixed issue where the cache of a lazy_value used for a tuple of parameters (several argnames) was not considering the pytest context and thus was wrongly used across pytest nodes. Fixes #202

Author: Sylvain MARIE

pytest_cases/common_pytest_lazy_values.py

@@ -147,9 +147,8 @@ def copy_from(cls,
         """Creates a copy of this _LazyValue"""
         new_obj = cls(valuegetter=obj.valuegetter, id=obj._id, marks=obj._marks)
         # make sure the copy will not need to retrieve the result if already done
-        if obj.has_cached_value():
-            new_obj.cached_value_context = obj.cached_value_context
-            new_obj.cached_value = obj.cached_value
+        new_obj.cached_value_context = obj.cached_value_context
+        new_obj.cached_value = obj.cached_value
         return new_obj
 
     # noinspection PyMissingConstructor
@@ -225,18 +224,44 @@ def get(self, request_or_item):
         """
         node = get_test_node(request_or_item)
 
-        if self.cached_value_context is None or self.cached_value_context() is not node:
+        if not self.has_cached_value(node=node):
             # retrieve the value by calling the function
             self.cached_value = self.valuegetter()
             # remember the pytest context of the call with a weak reference to avoir gc issues
             self.cached_value_context = weakref.ref(node)
 
         return self.cached_value
 
-    def has_cached_value(self):
-        """Return True if there is a cached value in self.value, but with no guarantee that it corresponds to the
-        current request"""
-        return self.cached_value_context is not None
+    def has_cached_value(self, request_or_item=None, node=None, raise_if_no_context=True):
+        """Return True if there is a cached value in self.value correnponding to the given request
+
+        A degraded query "is there a cached value" (whatever the context) can be performed by not passing any
+        request, item or node, and switching `raise_if_no_context` to False.
+
+        :param request_or_item: the pytest request or item
+        :param node: the pytest node if it already known.
+        :param raise_if_no_context: a boolean indicating if an error should be raised if `request_or_item` and `node`
+            are both None. Default is `True`.
+        """
+        if node is None:
+            # can we get that context information from the request/item ?
+            if request_or_item is None:
+                if raise_if_no_context:
+                    raise ValueError("No request, item or node was provided: I can not tell if there is a "
+                                     "cached value for your context. Switch `raise_if_no_context=False` if"
+                                     " you wish to get a degraded answer.")
+                else:
+                    # degraded answer: just tell if the cache was populated at least once
+                    return self.cached_value_context is not None
+
+            # get node context information
+            node = get_test_node(request_or_item)
+
+        elif request_or_item is not None:
+            raise ValueError("Only one of `request_or_item` and `node` should be provided")
+
+        # True if there is a cached value context that is the same as the context of the request
+        return self.cached_value_context is not None and self.cached_value_context() is node
 
     def as_lazy_tuple(self, nb_params):
         return LazyTuple(self, nb_params)
@@ -280,7 +305,7 @@ def __repr__(self):
         """Override the inherited method to avoid infinite recursion"""
         vals_to_display = (
             ('item', self.item),  # item number first for easier debug
-            ('tuple', self.host.cached_value if self.host.has_cached_value() else self.host._lazyvalue),  # lazy value tuple or cached tuple
+            ('tuple', self.host.cached_value if self.host.has_cached_value(raise_if_no_context=False) else self.host._lazyvalue),  # lazy value tuple or cached tuple
         )
         return "%s(%s)" % (self.__class__.__name__, ", ".join("%s=%r" % (k, v) for k, v in vals_to_display))
 
@@ -348,8 +373,19 @@ def get(self, request_or_item):
         """
         return self._lazyvalue.get(request_or_item)
 
-    def has_cached_value(self):
-        return self._lazyvalue.has_cached_value()
+    def has_cached_value(self, request_or_item=None, node=None, raise_if_no_context=True):
+        """Return True if there is a cached value correnponding to the given request
+
+        A degraded query "is there a cached value" (whatever the context) can be performed by not passing any
+        request, item or node, and switching `raise_if_no_context` to False.
+
+        :param request_or_item: the pytest request or item
+        :param node: the pytest node if it already known.
+        :param raise_if_no_context: a boolean indicating if an error should be raised if `request_or_item` and `node`
+            are both None. Default is `True`.
+        """
+        return self._lazyvalue.has_cached_value(request_or_item=request_or_item, node=node,
+                                                raise_if_no_context=raise_if_no_context)
 
     @property
     def cached_value(self):
@@ -361,17 +397,16 @@ def __getitem__(self, item):
         a facade (a LazyTupleItem), so that pytest can store this item independently wherever needed, without
         yet calling the value getter.
         """
-        if self._lazyvalue.has_cached_value():
-            # this is never called by pytest, but keep it for debugging
-            return self._lazyvalue.cached_value[item]
-        elif item >= self.theoretical_size:
+        if item >= self.theoretical_size:
             raise IndexError(item)
         else:
-            # do not retrieve yet: return a facade
+            # note: do not use the cache here since we do not know the context.
+            # return a facade than will be able to use the cache of the tuple
             return LazyTupleItem(self, item)
 
     def force_getitem(self, item, request):
         """ Call the underlying value getter, then return self[i]. """
+        # Note: this will use the cache correctly if needed
         argvalue = self.get(request)
         try:
             return argvalue[item]

pytest_cases/tests/cases/issues/test_issue_202.py

@@ -0,0 +1,27 @@
+import pytest_cases
+
+
+def case_tup():
+    return [], {}  # This can be reproduced with any mutable object
+
+
+@pytest_cases.fixture(scope="function")
+@pytest_cases.parametrize_with_cases("obj1, obj2", cases=".")
+def tup(obj1, obj2):
+    return obj1, obj2
+
+
+def test_1(tup):
+    obj1, obj2 = tup
+
+    assert len(obj1) == 0
+
+    obj1.append(1)
+
+
+def test_2(tup):
+    obj1, obj2 = tup
+
+    assert len(obj1) == 0
+
+    obj1.append(1)

pytest_cases/tests/pytest_extension/parametrize_plus/test_lazy_value_low_level.py

@@ -62,19 +62,21 @@ def __init__(self):
 
     # now do the same test for lazy values used as a tuple of parameters
     new_lv = lazy_value(foo)
-    assert not new_lv.has_cached_value()
-    assert a.has_cached_value()
+    assert not new_lv.has_cached_value(fake_request)
+    assert a.has_cached_value(fake_request)
+    with pytest.raises(ValueError):
+        a.has_cached_value()
 
     for src in new_lv, a:
         # set the counter according to the state of the cache
-        _called = 0 if not src.has_cached_value() else 1
+        _called = 0 if not src.has_cached_value(fake_request) else 1
         at = src.as_lazy_tuple(2)
 
         # test that it is hashable
         assert hash(at) == hash((LazyTuple, src, 2))
 
         # test when the tuple is unpacked into several parameters
-        if not at.has_cached_value():
+        if not at.has_cached_value(fake_request):
             for i, a in enumerate(at):
                 # test that it is hashable
                 assert hash(a) == hash((LazyTupleItem, at, i))
@@ -83,8 +85,7 @@ def __init__(self):
                 assert mini_idval(a, 'a', 1) == 'foo[%s]' % i
                 assert ('LazyTupleItem(item=%s' % i) in repr(a)
         else:
-            # this does not happen in real usage but in case a plugin messes with this
-            assert tuple(at) == (1, 2)
+            assert tuple(at)[0], tuple(at)[1] == (1, 2)
 
         # test when the tuple is not unpacked -
         # note: this is not supposed to happen when @parametrize decorates a test function,
@@ -104,7 +105,9 @@ def __init__(self):
 
         # test that retrieving the tuple does not loose the id
         assert str(at) == 'foo'
-        assert at.has_cached_value()
+        assert at.has_cached_value(fake_request2)
+        assert not at.has_cached_value(fake_request)
+        assert at.has_cached_value(raise_if_no_context=False)
 
 
 def test_lv_clone():