Yoong Kang Lim

Test logic, not configuration, or why you shouldn't test `login_required`

Recently, there was a question on a Django subreddit asking about the best way to test Django views with the login_required decorator.

Some people recommend testing both the authenticated pathway for the view, and also the non-authenticated pathway.

My opinion? Don’t test the non-authenticated pathway. Let me tell you why.

Whose code does it hit?

Let’s say we have this plain function-based view in Django:

@login_required
def book_list(request):
    return render(request, 'books/list.html', {'books': Book.objects.all()})

Now let’s say you decide to go with the recommendation to test the non-authenticated pathway:

class BookListTestCase(TransactionTestCase):
    def test_not_authenticated(self):
        self.client.logout()
        response = self.client.get(reverse('book-list'))
        self.assertEquals(response.status_code, 302)

Here’s a question for you. How many lines of your own application code did this test hit?

Exactly zero.

You’re testing the functionality of the login_required decorator in your tests.

The decorator already has extensive tests in the Django codebase, so why repeat this?

That is a pointless test.

Don’t test the framework, test your own code.

Some objections

You might ask, “The purpose of the test isn’t to get code coverage, it’s to make sure your requirement is met. What if someone accidentally removes the decorator?”.

Okay, let me try another way to convince you.

Let’s decouple the decorator from the view, and put the decorator in urls.py:

# urls.py

# now the decorator is in here
urlpatterns = [
    # public views
    path('', home, 'home')

    # login_required views
    path('books', login_required(book_list), 'book-list'),
    path('book/<int:id>', login_required(book_detail), 'book-detail'),
]

There is one question I like to ask whenever I see something like this:

Can we express this bit of code as a YAML file?

Actually, we can:

- urlpatterns

  - public
    - home
      - 'home'
      - base.views.home

  - login_required
    - book-list
      - 'books'
      - books.views.book_list

    - book-detail
      - 'books/<int:id>'
      - books.views.book_detail

There’s a word for something that looks like this. It’s called “configuration”.

The fact that the mechanism in Django happens to be Python code doesn’t matter – we’re looking at something that is configuration, not logic.

Another example of configuration in Django is the settings.py file.

Do we test anything in the settings.py file?

I have a requirement that my user’s passwords are secure, but I don’t write tests to check that my application doesn’t use an insecure algorithm like MD5 for password hashing. I configure the hashing algorithm in settings.py and have faith that the framework does the right thing.

I have a requirement that my application has CSRF protection. You might argue that someone would “accidentally” remove the CSRF middleware as well. Do you test every view for CSRF protection? Nope, I put the middleware in the configuration, and trust that the framework does the right thing.

I could go on.

My point? The file urls.py is configuration, not logic. So no, I don’t test this. You put in the configuration (login_required) in the configuration file (urls.py) and have faith that the framework does the right thing.

Don’t test configuration, test logic!

To me, these two things are clear signs that you shouldn’t test that functionality:

  • The test hits exactly 0 of your own code
  • You can replace the relevant bits of code with a YAML file + a parser.

But… isn’t it easier for someone to forget to include the decorator?

This is why you should separate logic from configuration, and put the configuration in one place.

If you decouple login_required from the view (that is, put login_required in urls.py as described above), then it’s exceedingly easy to catch which views aren’t secured with the decorator. Just look at urls.py.

You can trivially catch that in code reviews, just like you can catch crazy things being done in settings.py.

This decoupling also leads to some benefits. You can now now use Django’s RequestFactory, and pass a test double of a request object into a view. And now you only hit your view code, not things like middleware (which is already tested within the Django codebase).

For more on that topic: https://tech.people-doc.com/django-unit-test-your-views.html

I still want to test this

You can.

Write a test for urls.py by mocking login_required and testing that it was called for each view.

That’s a pretty pointless test in my opinion, but it’s “correct” in the TDD sense.

You can do that if it gives you piece of mind. And it doesn’t have to hit any of the code within login_required, this only tests that it was called.

Summary

  • Don’t write tests for your framework.
  • Write tests for your logic, not configuration

If you like posts like this, you might want to follow me on Twitter. Also, if you need any help building or improving your projects (Python/Django, JavaScript, Machine Learning, etc.) feel free to shoot me an email.