7

I have a method that looks like this:

def foobar(mock=False, **kwargs):
    # ... snipped

foobar actually makes several calls to Amazon S3 and returns a composed result. In order to make this testable, I introduced the mock parameter to turn off making live network connections. It feels like a code smell for me but testability is also very important. What else can I do if I want to do away with the parameter ?

  • 1
    Is foobar the real method? I mean, it's outside the tests, right? – Gabriel Aug 1 '11 at 2:17
  • foobar is not the real name of my method. – Bon Ami Aug 1 '11 at 2:45
29

Why not have a connection class instead?

class Connection(object):
    def retrieve(self, resource):
        return something_from_s3()

class MockConnection(Connection):
    def retrieve(self, resource):
        return 42

def foobar(connection = Connection(), **kwargs):
    whatever = connection.retrieve("foobar")

Not only is this cleaner, but you can trivially test it with various mock connections. If you ever decide to support a different kind of service instead of S3, you can easily extend your product to support it.

| improve this answer | |
  • Also makes unit testing much easier! – John Lyon Aug 1 '11 at 6:07
10

Yes, that's a huge, HUGE code smell.

If you want to test something that's backed by S3, stub out or mock any calls you make to your S3 API and do it that way.

For a simple example, have a look at Paperclip: https://github.com/thoughtbot/paperclip/blob/master/test/storage_test.rb

| improve this answer | |
0

Sometimes, it's easier to just pass some "s3 engine" as a parameter and pass some simple mock-object instead when it's a test (and provide it's return values). Sometimes you need to call lots of random functions that cause side-effects (for example, if you would work with s3, get current time, do some sql-queries, open some files and more), and here it wouldn't be nice to pass it all as function-parameters (since it isn't needed except for testability), I'd suggest you to use mockstar to declaratively describe your side-effects. Your code would then something like this:

def foobar():
    foo = get_from_database()
    bar = read_from_file()
    baz = read_from_s3()

    if foo > 10:
        return foo + bar + baz
    else:
        return foo - bar - baz

# And your test would look something like this:

from mockstar import prefixed_p
from nose.tools import assert_equal

ppatch = prefixed_p('module.with.foobar.func')


class TestFoobar(BaseTestCase):
    @ppatch('get_from_database')
    @ppatch('read_from_file')
    @ppatch('read_from_s3')
    def side_effects(self, se):
        se.read_from_file.return_value = 10  # default behaviour
        se.read_from_s3.return_value = 0  # also default behaviour
        return self.invoke(se)

    def test_should_get_30(self, se):
        se.get_from_database.return_value = 20

        # do
        result = foobar()

        assert_equal(result, 30)

    def test_should_get_minus5(self, se):
        se.get_from_database.return_value = 5

        # do
        result = foobar()

        assert_equal(result, -5)
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.