Skip to content

Conversation

@s-t-e-v-e-n-k
Copy link

nose has been unmaintained upstream for years now, and so this project
should move with the times. To that end, switch to either using bare
assert statements, or where that isn't suitable, using pytest's raises
context manager.

nose has been unmaintained upstream for years now, and so this project
should move with the times. To that end, switch to either using bare
assert statements, or where that isn't suitable, using pytest's raises
context manager.
Comment on lines +123 to +127
with raises(NotImplementedError):
d.fromkeys()
d.viewitems()
d.viewkeys()
d.viewvalues()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with raises(NotImplementedError):
d.fromkeys()
d.viewitems()
d.viewkeys()
d.viewvalues()
with raises(NotImplementedError):
d.fromkeys()
with raises(NotImplementedError):
d.viewitems()
with raises(NotImplementedError):
d.viewkeys()
with raises(NotImplementedError):
d.viewvalues()

Copy link

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with this change the test fails for me with

============================= test session starts ==============================
platform linux -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0
rootdir: /build/source
collected 14 items                                                             

tests/expiringdict_extended_test.py ....                                 [ 28%]
tests/expiringdict_test.py ...F......                                    [100%]

=================================== FAILURES ===================================
__________________________________ test_repr ___________________________________

    def test_repr():
        d = ExpiringDict(max_len=2, max_age_seconds=0.01)
        d['a'] = 'x'
        assert str(d) == "ExpiringDict({'a': 'x'})"
        sleep(0.01)
>       assert str(d) == "ExpiringDict([])"

tests/expiringdict_test.py:70: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = ExpiringDict(), key = 'a', with_age = False

    def __getitem__(self, key, with_age=False):
        """ Return the item of the dict.
    
        Raises a KeyError if key is not in the map.
        """
        with self.lock:
            item = OrderedDict.__getitem__(self, key)
            item_age = time.time() - item[1]
            if item_age < self.max_age:
                if with_age:
                    return item[0], item_age
                else:
                    return item[0]
            else:
                del self[key]
>               raise KeyError(key)
E               KeyError: 'a'

expiringdict/__init__.py:86: KeyError
=========================== short test summary info ============================
FAILED tests/expiringdict_test.py::test_repr - KeyError: 'a'
========================= 1 failed, 13 passed in 0.22s =========================

d = ExpiringDict(max_len=2, max_age_seconds=0.01)
d['a'] = 'x'
eq_(str(d), "ExpiringDict([('a', 'x')])")
assert str(d) == "ExpiringDict([('a', 'x')])"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format changes starting with Python 3.12:

Suggested change
assert str(d) == "ExpiringDict([('a', 'x')])"
assert str(d) == "ExpiringDict({'a': 'x')}"

Copy link

@AndrewKvalheim AndrewKvalheim Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty representation has changed too:

-    assert str(d) == "ExpiringDict([])"
+    assert str(d) == "ExpiringDict()"

But stepping back, the module itself isn’t compatible with Python 3.12 so I don’t think it’s appropriate to update the tests for that yet.

AndrewKvalheim added a commit to AndrewKvalheim/nixpkgs that referenced this pull request Jul 13, 2024
d-xo pushed a commit to d-xo/nixpkgs that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants