5
\$\begingroup\$

I'm learning TDD with Django and currently I'm testing my CreateView class. I feel like I'm not using best practices but am not sure on how to improve my code.

test_reservation_create.py

from django.test import TestCase
from django.urls import reverse

from reservations.models import Reservation
from src.factories import UserFactory, GroupFactory, ProfileFactory, TagFactory


class TestCreateReservation(TestCase):

    def setUp(self):
        persona = UserFactory(groups=(GroupFactory.create(),))

        ProfileFactory(user=persona, )
        persona.profile.tags.add(TagFactory())

    def test_create_view_denies_anonymous(self):
        response = self.client.get(reverse('reservation-create'), follow=True)
        self.assertRedirects(response, '/login?next=/r/create/')

    def test_loads_template_for_user(self):
        self.client.login(username='john', password='defaultpassword')
        response = self.client.get(reverse('reservation-create'))
        self.assertEquals(response.status_code, 200)

    def test_create_blank_for_user(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
        }
        response = self.client.post(reverse('reservation-create'), data=data)
        self.assertFormError(response,'form','passenger_name','This field is required.')
        self.assertFormError(response,'form','passenger_lastname','This field is required.')
        self.assertFormError(response,'form','service_date','This field is required.')
        self.assertFormError(response,'form','author_alias','This field is required.')

    def test_formset_validation(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
            'service_date':'02/07/2027',
            'passenger_name':'John',
            'passenger_lastname':'Doe',
            'service_type':'BUSINESS',
            'status':0,
            'payment_options':'CASH',
            'amount':'0.00',
            'author_alias':'Faker',
            'vehicle':'CAR',
            'pax_number':1
        }
        response = self.client.post(reverse('reservation-create'), data=data, follow=True)
        self.assertContains(response,'You must have specify both place1 and place2')

    def test_create_minimal_for_user(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
            'service_date':'02/07/2027',
            'passenger_name':'John',
            'passenger_lastname':'Doe',
            'stops-0-time': '11:00',
            'stops-0-place':'Place1',
            'stops-1-place': 'Place2',
            'service_type':'BUSINESS',
            'status':0,
            'payment_options':'CASH',
            'amount':'0.00',
            'author_alias':'Faker',
            'vehicle':'CAR',
            'pax_number':1
        }
        response = self.client.post(reverse('reservation-create'), data=data, follow=True)
        self.assertRedirects(response, reverse('dashboard'))

        reservation = Reservation.objects.first()
        self.assertEquals(reservation.service_date.strftime('%Y-%m-%d'),'2027-02-07')
        self.assertEquals(reservation.passenger_name,'John')
        self.assertEquals(reservation.passenger_lastname,'Doe')
        self.assertEquals(reservation.service_type,'BUSINESS')
        self.assertEquals(reservation.status,0)
        self.assertEquals(reservation.author_alias,'Faker')
        self.assertEquals(reservation.payment_options,'CASH')
        self.assertEquals(reservation.amount,0.00)
        self.assertEquals(reservation.vehicle,'CAR')
        self.assertEquals(reservation.pax_number,1)

        self.assertEquals(reservation.stops.first().place, 'Place1')
        self.assertEquals(reservation.stops.last().place, 'Place2')
        self.assertEquals(reservation.stops.first().time.strftime('%H:%M'), '11:00')

    def test_create_minimal_for_user_and_new(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
            'service_date':'02/07/2027',
            'passenger_name':'John',
            'passenger_lastname':'Doe',
            'stops-0-time': '11:00',
            'stops-0-place':'Place1',
            'stops-1-place': 'Place2',
            'service_type':'BUSINESS',
            'status':0,
            'payment_options':'CASH',
            'save_and_new':'Submit',
            'amount':'0.00',
            'author_alias':'Faker',
            'vehicle':'CAR',
            'pax_number':1
        }
        response = self.client.post(reverse('reservation-create'), data=data, follow=True)
        self.assertRedirects(response, reverse('reservation-create'))

        reservation = Reservation.objects.first()
        self.assertEquals(reservation.service_date.strftime('%Y-%m-%d'),'2027-02-07')
        self.assertEquals(reservation.passenger_name,'John')
        self.assertEquals(reservation.passenger_lastname,'Doe')
        self.assertEquals(reservation.service_type,'BUSINESS')
        self.assertEquals(reservation.status,0)
        self.assertEquals(reservation.author_alias,'Faker')
        self.assertEquals(reservation.payment_options,'CASH')
        self.assertEquals(reservation.amount,0.00)
        self.assertEquals(reservation.vehicle,'CAR')
        self.assertEquals(reservation.pax_number,1)

        self.assertEquals(reservation.stops.first().place, 'Place1')
        self.assertEquals(reservation.stops.last().place, 'Place2')
        self.assertEquals(reservation.stops.first().time.strftime('%H:%M'), '11:00')

    def test_create_minimal_for_user_and_clone(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
            'service_date':'02/07/2027',
            'passenger_name':'John',
            'passenger_lastname':'Doe',
            'stops-0-time': '11:00',
            'stops-0-place':'Place1',
            'stops-1-place': 'Place2',
            'service_type':'BUSINESS',
            'status':0,
            'payment_options':'CASH',
            'save_and_clone':'Submit',
            'amount':'0.00',
            'author_alias':'Faker',
            'vehicle':'CAR',
            'pax_number':1
        }
        response = self.client.post(reverse('reservation-create'), data=data, follow=True)
        self.assertEquals(response.status_code, 200)
        self.assertContains(response, 'John')

        reservation = Reservation.objects.first()
        self.assertEquals(reservation.service_date.strftime('%Y-%m-%d'),'2027-02-07')
        self.assertEquals(reservation.passenger_name,'John')
        self.assertEquals(reservation.passenger_lastname,'Doe')
        self.assertEquals(reservation.service_type,'BUSINESS')
        self.assertEquals(reservation.status,0)
        self.assertEquals(reservation.author_alias,'Faker')
        self.assertEquals(reservation.payment_options,'CASH')
        self.assertEquals(reservation.amount,0.00)
        self.assertEquals(reservation.vehicle,'CAR')
        self.assertEquals(reservation.pax_number,1)

        self.assertEquals(reservation.stops.first().place, 'Place1')
        self.assertEquals(reservation.stops.last().place, 'Place2')
        self.assertEquals(reservation.stops.first().time.strftime('%H:%M'), '11:00')

    def test_invalid_date_in_past(self):
        self.client.login(username='john', password='defaultpassword')
        data = {
            'stops-TOTAL_FORMS': u'2',
            'stops-INITIAL_FORMS': u'0',
            'stops-MIN_NUM_FORMS': u'0',
            'stops-MAX_NUM_FORMS': u'1000',
            'stops-0-id': u'',
            'service_date': '02/07/2010',
        }
        response = self.client.post(reverse('reservation-create'), data=data)
        self.assertFormError(response, 'form', 'service_date', 'Date can\'t be in the past!')
\$\endgroup\$
4
\$\begingroup\$

The code has a lot of repetitive blocks, a test flow is mixed up with the data and the logic making it difficult to follow, the number of different assertions makes the tests bigger and more complicated.

I think you should have a much more readable code if you would switch to "data-driven tests" with "parameterized tests" (e.g. there is a built-in support in pytest) or take a look at the ddt package.

Also, instead of checking multiple model fields with multiple asserts, try dumping the model to, for example, a dictionary and compare it with a desired dictionary - basically this way you would check all the fields at once. Of course, in this case, in case of a failure, the output would be less clear, but if you have a good test runner like nose or pytest, this should not be a problem for the number of fields we are talking about.

Other notes:

  • the user credentials should probably be moved to a single place - to a config file for your tests. This way, if they change, we would need to change it in a single place instead of looking for them in multiple places in the code.
  • think about splitting your test case into multiple test cases - e.g. one for access control, the other for validation tests etc.
\$\endgroup\$
0
\$\begingroup\$

I have a few suggestions for how you can improve your tests to better guard against possible issues as well as make them more maintainable.

  • Always call super().setUp() if you override setUp in your tests.

    This may not seem like something which is required, but sometimes you'll use a custom test class which has some custom logic in the setUp method (usually involving the database). It's caught me a few times and I've ended up having to remove a bunch of test objects from my database.

  • If you aren't going to hard-code your test urls, don't hard code where you expect them to redirect to.

    test_create_view_denies_anonymous tests that unauthenticated users will get redirected, but you're hard-coding the url that it should be redirected to. You can safely rely on Django's reverse not messing up, which will allow you to build out the url it's redirected to while still handling you renaming the url in the future.

  • Don't hard-code your username and password in your test factory.

    This was mentioned already, but you should instead be setting the password for the user within the setUp for your test class, or at the start of the test itself. This allows you to create multiple users with different names, all within the same test.

  • Always assert your assumptions

    In test_create_minimal_for_user you are asserting that the user is redirected to the dashboard, which I'm assuming should mean the reservation was created. And then you call Reservation.objects.first(), which will either give you a Reservation or will give you None without raising an error. You then continue on, assuming that the reservation was returned, to test the properties of the reservation.

    You should instead be asserting that a Reservation exists before you try to retrieve it. Also, you can use .get() on any manager as long as there is a single object, which you appear to be assuming.

    self.assertEqual(Reservation.objects.count(), 1)
    
    reservation = Reservation.objects.get()
    

    This will ensure that you will either get a Reservation back, or your test will fail (not error out) because there were no reservations created. This also handles the case where multiple reservations were created for some reason.

  • Avoid hard-coding dates in your tests

    You managed to pick dates that were far into the future, but usually people forget to do that. In my experience, we had a bunch of tests which were hard-coded to fail in 2 years and 2 months, and we never touched those tests to update the dates until everything started failing on May 1st, 2015.

    You can avoid hard-coding dates in your tests by generating relative dates in the future using the timedelta module.

    from datetime import datetime, timedelta
    
    current_date = datetime.now()
    one_week_into_the_future = current_date + timedelta(weeks=1)
    
    formatted_date = one_week_into_the_future.strftime('%m/%d/%Y')
    
\$\endgroup\$

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.