tenacity: Cannot easily mock out sleep function in order to simulate time in tests

It was a little bit tricky for me to navigate this thread to find exactly what I needed to mock. here’s the easy answer:

class SomeTest(TestCase):

    @mock.patch("tenacity.nap.time.sleep", MagicMock())
    def test_something_with_retry(self):
        # invoke function that uses tenacity

Patching tenacity.nap.time.sleep didn’t stop tenacity from retrying for me.

Here’s what worked for me (pytest):

@pytest.fixture(autouse=True)
def foo(monkeypatch):
    monkeypatch.setattr(
        "tenacity.BaseRetrying.retry", tenacity.retry_if_result(lambda *args, **kwargs: False)
    )

Update: here’s what worked for me on tenacity 8.2.2 (pytest):

import pytest


@pytest.fixture(autouse=True)
def disable_tenacity_retry(monkeypatch):
    for retry_class in (
        "retry_if_exception",
        "retry_if_exception_type",
        "retry_if_not_exception_type",
        "retry_unless_exception_type",
        "retry_if_exception_cause_type",
        "retry_if_result",
        "retry_if_not_result",
        "retry_if_exception_message",
        "retry_if_not_exception_message",
        "retry_any",
        "retry_all",
        "retry_always",
        "retry_never",
    ):
        monkeypatch.setattr(f"tenacity.{retry_class}.__call__", lambda *args, **kwargs: False)

@jd you mean that literally everyone who is serious about their project (and thus writes unit tests) has to subclass Retrying? Forking tenacity would be less work overall…

Hmmm, so just to be clear, the change being talked about here is to change the definition of nap.sleep from sleep = time.sleep to def sleep(seconds): time.sleep(seconds) in order to facilitate easier mocking, in a single place, that works for a variety of usage scenarios.

It’s a pretty minor change, with a major benefit for end-to-end testability of code using the library.

Is there a specific aspect of this change that you think might be a pain to maintain in the long run? If not, then I’m failing to see the reasons behind not wanting to merge such a change.

What are the disadvantages of this change which are making you “not really excited” about making it?

That’s an excellent question to ask. I don’t think there is much. I’m just trying to be really suspicious about changes that are not useful and might be a pain to maintain in the long run. That’s part of my job here. 😄

@jd You said you’re “not really excited about the change”. It’s definitely true that you can work around this already in various ways, but I just think it would be better if you didn’t need to.

In my particular scenario I’m trying to standardise a myriad of retry approaches in a large codebase to all use tenacity, and convince other developers that this is worthwhile. The need to apply workarounds like this in order to be able to test is a barrier to adoption of the library by other developers.

Forking the library and applying the patch I’m suggesting is also an approach, but again carries more overhead than having this in the library as standard, and available to all users of tenacity without each needing to visit this hurdle and come up with a mitigation approach. (e.g. https://github.com/openstack/cinder/blob/307fccea4cc9c6496334b0fe137206cb48499bd5/cinder/utils.py#L56 as discussed earlier in the comment trail)

So to summarise I think there are definite advantages to applying this change to the library, and I can see no downsides.

What are the disadvantages of this change which are making you “not really excited” about making it?