Don't Repeat Yourself (DRY) and Testing

At globo.com my team is responsible for live video streaming infrastructure, and we have a huge infrastructure with lots of projects.

One of our projects is PremiereFC’s website, where we live stream brazilian soccer games.

PremiereFC is written in Python + Tornado + MongoDB, the project is not complex and we have great test suites. It seems perfect.

We did not touch the project for months, and some months ago we needed to adapt it to embrace new championships and other stuff. My colleague Flávio Ribeiro started running the current tests against our QA1 environment. For his surprise some tests were broken! Can you believe it? Nobody touch the project and BOOM, some tests are failing.

He started digging into the tests to find out what happened, and unfortunately he could not understand easily what was happening.

WHY? Our conclusion is that we have lots and lots of test helpers…

Note: I started to draft this post in the end of 2012, but I’ve forgotten to publish it.

Testing and Test Helpers

We do love Test-Driven Development, and we have lots of unit, functional, and integration tests. As we do not like repeating ourselves, creating abstractions around our repeated tests seems obvious, and we started doing it in the beginning of PremiereFC.

We have a test to ensure our FAQ is only visible to authorized users:

def test_should_access_faq():
    headers, html = get_authorized('/ajuda')
    assert "200" == headers['status']

The test name is just kind of generic and there is a get_authorized() test helper. The assertion is based on HTTP status code. Seems ok, let’s move on.

Do not repeat yourself - DRY

Why do we have a get_authorized() test helper? Because we have other tests that share common steps, and we like the idea of not repeating ourselves. Using that test helper we can create new tiny tests that do a lot and we do not need to care about lots of details. That idea is really great when you are writing new tests and they don’t fail or break.

Let’s see what get_authorized() is:

def get_authorized(url, user_name='PFCGuy', session_key=None):
    cookies = set_user_in_cookie(user_name)
    return get_html_with_cookie(url, cookies)

Hmm, it uses other two test helpers: set_user_in_cookie() and get_html_with_cookie(), and it seems to use "PFCGuy" as the user trying to see that page. Keep going.

def set_user_in_cookie(user_name):
    pfc_cookie = serializer.serialize({'authorized': True, 'user_name': user_name})
    cookies = {
        settings.PFC_COOKIE: pfc_cookie
    }
    return cookies

Okay, it just creates a dictionary with a serialized entry.

We are not done yet and we had to stack lots of stuff in our head just to understand something I can’t remember, because I had to dig into a lot to understand what my test is doing. If our brain is limited to deal with 7 different things (based on the Seven, Plus or Minus Two paper), we are in trouble, because we already stacked 1) test_should_access_faq() 2) get_authorized() and 3) set_user_in_cookie().

Now that we know that we first need to set a cookie before requesting the FAQ page, we can move on get_html_with_cookie():

def get_html_with_cookie(url, cookie_value):
    return get_with_cookie(url, cookie_value, get_html)

Hmm, it uses another helper and there is a magic get_html thing. What is that? Using my text-editor search I could find that is yet another helper. Oh god…

def get_html(url, follow_redirects=False, cookies=None, headers=None):
    headers, body = get(url, follow_redirects, headers, cookies=cookies)
    if not body:
        pytest.fail("Response could not be parsed as html. "
                    "Status: %r. Body: %r" % (headers['status'], body))
    else:
        return headers, html.fromstring(body)

Oh, get_html() is just a wrapper to yet another f*cking helper:

def get(url, follow_redirects=False, headers=None, host=settings.SITE_URL, cookies=None):
    if url.startswith('/'):
        url = host + url
    resp = requests.get(url, headers=headers, allow_redirects=follow_redirects, cookies=cookies)
    headers = resp.headers
    content = resp.text or ''
    headers['status'] = str(resp.status_code)
    return headers, content

Now that we got here we can go back and look for get_with_cookie(), that is called inside get_html_with_cookie().

def get_with_cookie(url, cookies, get_function=get, follow_redirects=False, **kwargs):
    headers, body = get_function(url,
                                 follow_redirects=follow_redirects,
                                 cookies=cookies,
                                 **kwargs)
    return headers, body

It just call that get_html callback, named get_function here, using the arguments we already had.

I am pretty sure you are lost here, because we, that wrote the code in the past, got lost multiple times. And as I am writing this post I got lost again.

Let’s see what our mental stack is:

  1. test_should_access_faq()
  2. get_authorized()
  3. set_user_in_cookie()
  4. get_html_with_cookie()
  5. get_html()
  6. get()
  7. get_with_cookie()

SEVEN? That magical number?! Really? My mind can’t stack that much and not get lost. When we reached get_html() I was already lost.

DO REPEAT YOURSELF! (when testing)

Our conclusion is that when you are writing tests, YOU SHOULD repeat yourself, and make the test very clear.

No matter how much similar test code you have, repeat it and make yourself as clear as possible.

We discussed these ideas with our colleague Juarez Bochi, and he mentioned the beginning of the Structure and Interpration of Computer Programs book, where the authors describe three mechanisms every powerful language have:

  • primitive expressions, which represent the simplest entities the language is concerned with,
  • means of combination, by which compound elements are built from simpler ones, and
  • means of abstraction, by which compound elements can be named and manipulated as units.

And after mentioning that he asked us: “Are you saying that we should not combine abstraction?”

My conclusion is that we should apply these abstraction ideas to our tests, but not so extensively as we should in our business code. We must be very careful to not fall into those matrioskas traps.

Flávio and I tried to rewrite that single test as clear as possible. The result:

def test_authorized_user_should_access_faq():
    user_name = "PFCGuy"
    url = settings.SITE_URL + "/ajuda"
    pfc_cookie = serializer.serialize({'authorized': True, 'user_name': user_name})
    cookies = { settings.PFC_COOKIE: pfc_cookie }

    resp = requests.get(url, cookies=cookies)

    assert 200 == resp.status_code
    assert resp.text

Now we have one thing to stack in our head, nothing else. If we have a new test that shares that authorization logic, we are going to repeat ourselves, and we are ok with it. The benefit is immediate: if the test break, we know where to set breakpoints and debug. And everyone that looks at that test know exactly what happened.

If we compare these two approaches we realize the first (with lots of helpers) have business logic spread all over the code, while the second is self-contained. We like self-contained tests, they are better to mantain.

After this story we discussed some guidelines in order to avoid that kind of trap.

Guideline

We are not set yet, but we are discussing some set of rules when testing:

  • If your helper calls another helper, stop right now. Do not do that.
  • If your helper hides business logic, stop it. That should be clear in your test.
  • If your helper has default arguments, you are probably hiding important information and hurting the next developer that touch that code. Think twice before doing it.

Next steps

We have lots of great discussions here in my team and we are always getting better. That’s why I love these guys. Unfortunately we lost Igor Sobreira (he moved to Hawaii), a great developer and he always made our discussions richer. And I am about to leave the team (that’s for another post).

Some ideas we have discussed over and over again are: how to write and maintain documentation, and how to do environment-dependent configuration. These subjects need a special post, but let me just introduce them.

Documentation

My team works phisically in the same place, one next to the other, and most of what we write is not opensource. We work at the same place and day hours, and because of that we are not forced to write better documentation. Our opensource projects have a way better documentation, due to the nature of the project itself (people around the world will try to use and contribute), they are not only for ourselves. We need to learn how to improve on it.

Configuration

We have projects where there is a settings/ directory and there are prod.py, qa1.py, dev.py, and these files contain environment-specific configuration. It sucks because you must remember to change all files if one configuration changes and they tend to mix code and configuration. If you use Django you are probably used to this workflow, but think twice the next time you touch a settings file.

That settings on different files practice is bad because someone needs to manage that, and it is possible to add code to one file and forget to add to another. There is a possibility that you have all tests passing and everything working fine in an environment, but it may break when you deploy to production because of a missing line at prod.py.

Twelve Factor App has a section on Configs, and they recommend using environment variables for configuration. We like that idea and in some projects we have a hybrid approach.

blog comments powered by Disqus