Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bookingtests #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Bookingtests #56

wants to merge 3 commits into from

Conversation

CastorK
Copy link

@CastorK CastorK commented Oct 29, 2019

Added tests for checking that booking restrictions are enforced. They dont pass at the moment because of bugs in forms.py that I will fix next

@tlangens
Copy link
Collaborator

tlangens commented Oct 29, 2019

They dont pass at the moment because of bugs in forms.py that I will fix next

Will merge once those are fixed.

Comment on lines +8 to +19
def setUp(self):
Bookable.objects.create(
id_str = "b",
name = "B",
description = "A bookable"
)

User = get_user_model()
User.objects.create_user(
username = "u",
password = "pw"
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend not creating test data in setUp, it will annoy you when you decide to test something that requires a different setup. It's better to call some simple functions at the start of each test.

'bookable': 'b'
}

c.login(username='u', password='pw')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is why you want to create your test data inside your tests. You'd want to avoid using these hardcoded strings.

c.login(username='u', password='pw')
response = c.post('/booking/book/b', postdata)

self.assertEqual(response.status_code, 201)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't assert a whole lot. It would be great to check that a new booking was created.

self.assertEqual(response.status_code, 403)


def test_cant_book_too_far_in_future(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any limit set by default, another reason you might want to create your test data inside the test

self.assertEqual(response.status_code, 403)


def test_cant_book_too_long(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, no limit set by default

}

c.login(username='u', password='pw')
response = c.post('/booking/book/b', postdata)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/booking/book/b ??
I hope you don't intend to allow creating bookings with arbitrary ID's...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants