A tale of tests, greenlets and patches

Once upon a working day there was some Python code I needed to update. The changes required were unremarkably clear, however some unit tests managed to hit me good enough with surprise.

Green(let) pastures

I needed to verify that the system under test called its dependencies on a certain order, and the way I decided to do this was by setting assertions up as side effects on the mocked dependencies:

def test_execution_order():
    dependency1 = mock.Mock()
    dependency2 = mock.Mock()

    # dependency1.foo() must be called before dependency2.foo()
    dependency1.foo.side_effect = dependency2.foo.assert_not_called
    dependency2.foo.side_effect = dependency1.foo.assert_called_once

    sut(dependency1, dependency2)

This test would have passed/failed just fine on a plain, synchronous implementation of the system under test:

# right order, the test passes
def sut(dependency1, dependency2):
    dependency1.foo()
    dependency2.foo()

# wrong order, the test fails
def sut(dependency1, dependency2):
    dependency2.foo()
    dependency1.foo()

However, the actual implementation made fairly heavy use of gevent to execute the dependencies in separate greenlets, then wait for them to complete:

# right order, the test passes
def sut(dependency1, dependency2):
    gevent.joinall([gevent.spawn(dependency1.foo)])
    gevent.joinall([gevent.spawn(dependency2.foo)])

# wrong order, the test... passes!?
def sut(dependency1, dependency2):
    gevent.joinall([gevent.spawn(dependency2.foo)])
    gevent.joinall([gevent.spawn(dependency1.foo)])

Here came the puzzling bit as the test passed in both cases.

All the cats join in

The test output showed assertion exceptions when run against the bad implementation, which at least reassured me on the approach taken. Still, the test was passing nonetheless.

I went back to the gevent documentation and soon the reason behind revealed itself:

joinall(greenlets, timeout=None, raise_error=False, count=None)

Notice the raise_error=False bit? gevent.joinall waits for a set of greenlets to complete, but by default does not bubble up any exceptions raised from within the awaited greenlets - including the failed assertions.

Unfortunately, changing the production code to call gevent.joinall with raise_error=True was not an option.

Patchwork

When one door closes, another opens, and the one opening here would allow me to patch the behavior of gevent.joinall in the test to force the raise_error=True parameter on it.

The first attempt naively fell into a RecursionError as the patched function ended up calling itself ad infinitum:

def gevent_joinall_raising_errors(*args, **kwargs):
    kwargs['raise_error'] = True
    return gevent.joinall(*args, **kwargs)  # calls the patched function (itself)

@patch('gevent.joinall', gevent_joinall_raising_errors)
def test_execution_order():
    ...

Second time was the charm, though. The final solution involved capturing the original, non-patched gevent.joinall function inside a nested scope:

def gevent_joinall_with(**kwargs_):
    gevent_joinall = gevent.joinall

    def gevent_joinall_wrapper(*args, **kwargs):
        kwargs.update(kwargs_)
        return gevent_joinall(*args, **kwargs)  # calls the non-patched function

    return gevent_joinall_wrapper

@patch('gevent.joinall', gevent_joinall_with(raise_error=True))
def test_execution_order():
    ...

In the end

This is one of those cases where I could have easily written a test, seen it pass and moved on. I am very glad that over the years many authors have reinforced on me the idea of always asking a test to fail before accepting it passing.

A remark on unittest.mock.patch. By definition, patching the system under test changes its behavior, which is inherently dangerous as we might not end up testing the logic we mean to anymore. In this scenario, I was sufficiently assured that the patch would not affect the behavior of my production code beyond allowing assertion exceptions up.