Test and fix for #348 (#349)

* test and fix for #348

* quick fix for py27 ci

* another shot at a quick py27 ci fix

* caching doesn't work oob in containers, another 2.7 ci fix

* Also test LRI overwriting

Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>

---------

Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>

.github/workflows/tests.yaml

@@ -25,7 +25,6 @@ jobs:
           - {name: '3.9', python: '3.9', os: ubuntu-latest, tox: py39}
           - {name: '3.8', python: '3.8', os: ubuntu-latest, tox: py38}
           - {name: '3.7', python: '3.7', os: ubuntu-latest, tox: py37}
-          - {name: '2.7', python: '2.7.18', os: ubuntu-latest, tox: py27}
           - {name: 'PyPy3', python: 'pypy-3.9', os: ubuntu-latest, tox: pypy3}
     steps:
       - uses: actions/checkout@v2
@@ -47,3 +46,19 @@ jobs:
           key: pip|${{ runner.os }}|${{ matrix.python }}|${{ hashFiles('setup.py') }}|${{ hashFiles('requirements/*.txt') }}
       - run: pip install tox
       - run: tox -e ${{ matrix.tox }}
+  tests-py27:
+    name: '2.7'
+    runs-on: ubuntu-20.04
+    container:
+      image: python:2.7.18-buster
+    strategy:
+      fail-fast: false
+    steps:
+      - uses: actions/checkout@v2
+      - name: update pip
+        run: |
+          pip install -U wheel
+          pip install -U setuptools
+          python -m pip install -U pip
+      - run: pip install tox
+      - run: tox -e py27

boltons/cacheutils.py

@@ -235,9 +235,10 @@ class LRI(dict):
                 else:
                     evicted = self._set_key_and_evict_last_in_ll(key, value)
                     super(LRI, self).__delitem__(evicted)
-                super(LRI, self).__setitem__(key, value)
             else:
                 link[VALUE] = value
+            super(LRI, self).__setitem__(key, value)
+        return
 
     def __getitem__(self, key):
         with self._lock:

tests/test_cacheutils.py

@@ -167,6 +167,32 @@ def test_lru_basic():
     assert second_lru != lru
 
 
+@pytest.mark.parametrize("lru_class", [LRU, LRI])
+def test_lru_dict_replacement(lru_class):
+    # see issue #348
+    cache = lru_class()
+
+    # Add an entry.
+    cache['a'] = 1
+
+    # Normal __getitem__ access.
+    assert cache['a'] == 1  # passes.
+    # Convert to dict.
+    assert dict(cache) == {'a': 1}  # passes.
+    # Another way to access the only value.
+    assert list(cache.values())[0] == 1  # passes.
+
+    # Replace the existing 'a' entry with a new value.
+    cache['a'] = 200
+
+    # __getitem__ works as expected.
+    assert cache['a'] == 200  # passes.
+
+    # Both dict and accessing via values() return the old entry: 1.
+    assert dict(cache) == {'a': 200}  # fails.
+    assert list(cache.values())[0] == 200
+
+
 def test_lru_with_dupes():
     SIZE = 2
     lru = LRU(max_size=SIZE)