Merge lp://qastaging/~registry/horizon/25_dashboard_model_tests into lp://qastaging/~hudson-openstack/horizon/trunk

Proposed by Mark Gius
Status: Merged
Approved by: Devin Carlen
Approved revision: 69
Merged at revision: 62
Proposed branch: lp://qastaging/~registry/horizon/25_dashboard_model_tests
Merge into: lp://qastaging/~hudson-openstack/horizon/trunk
Diff against target: 268 lines (+193/-6)
5 files modified
django-openstack/src/django_openstack/models.py (+6/-5)
django-openstack/src/django_openstack/tests/__init__.py (+2/-0)
django-openstack/src/django_openstack/tests/test_models.py (+169/-0)
django-openstack/src/django_openstack/testsettings.py (+7/-1)
django-openstack/src/django_openstack/utils.py (+9/-0)
To merge this branch: bzr merge lp://qastaging/~registry/horizon/25_dashboard_model_tests
Reviewer Review Type Date Requested Status
Devin Carlen Approve
Review via email: mp+63913@code.qastaging.launchpad.net

Description of the change

Unit tests for django-openstack.models

To post a comment you must log in.
66. By Mark Gius

Merge localization changes from trunk

Revision history for this message
Devin Carlen (devcamcar) wrote :

A few style nits:

These should go above the from ... imports for consistency:

65 +import datetime
66 +import hashlib
67 +import mox
68 +import random

Use the full module path in this case, which I think is probably from django_openstack.test_models or something:

51 from django_openstack.nova.tests import *
52 +
53 +from test_models import *

Prefer underscores to camelCase (ignore the fact that the setUp and tearDown methods ignore this - its a weird holdover from the old pyunit library):

84 + testCred = nova_models.CredentialsAuthorization()
85 + testCred.username = TEST_USER
86 + testCred.project = TEST_PROJECT
87 + testCred.auth_date = TEST_AUTH_DATE
88 + testCred.auth_token = TEST_AUTH_TOKEN
89 + testCred.save()

Use utcnow() instead of now():

122 + cred.auth_date = datetime.datetime.now() - AUTH_EXPIRATION_LENGTH \
123 + - HOUR

Better yet, don't use datetime directly at all, since it makes testing hard. Nova gets around this problem by supplying a utils.utcnow() method that wraps this and makes it a little more syntactically pretty.

180 + # testing with time is tricky. Mock out "right now" test to avoid
181 + # timing issues
182 + time = datetime.datetime.now()

Supernit! Need a space to left of equals:

248 +CREDENTIAL_DOWNLOAD_URL= TESTSERVER + '/credentials/'

review: Needs Fixing
67. By Mark Gius

Address review comments

68. By Mark Gius

Remove hacky post_save short-circuit. Post_save tests to follow later

Revision history for this message
Devin Carlen (devcamcar) wrote :

Looks good. Only one minor nit:

156 + self.assertTrue(cred.username == TEST_USER2)
157 + self.assertTrue(cred.project == TEST_PROJECT)
158 + self.assertTrue(cred.auth_token == TEST_AUTH_TOKEN_2)

self.assertTrue should be replaced with assertEquals.

review: Needs Fixing
69. By Mark Gius

Fix final nit

Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches