-
Notifications
You must be signed in to change notification settings - Fork 22
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
Test suite #9
Test suite #9
Conversation
entry.save() | ||
|
||
def test_get_single_entry_data(self): | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want to test for "by title" with two same-titled entries too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make two entries with the same title and see if we get two entries back.
self.assertEqual(response.status_code, 200) | ||
|
||
def test_post_minimum_entry(self): | ||
#Entry with minimum amount of content (title, description?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want negative tests, too, so that there's no way to regress to a model where title+description are no longer necessary?
(test for entry missing a title, and one missing a descr., and make sure those are errors)
def test_get_tag_list(self): | ||
#maybe for another view. But tags list should have multiple tags, some with multiple entries associated | ||
#do we need an endpoint for /tags/{tagname} that has the associated entries? or does /entries/?tag=foo work? | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want an issues list too, since those'll have to be API exposed for 3rd parties (so they can generate an issue dropdown)
#test filtering entires by issue | ||
|
||
def test_post_entry_new_issue(self): | ||
#posting an entry with a new Issue should result in an error. Permission denied? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably not permission denied, but maybe a 409 "please rephrase yourself" with a specific JSON response that explains the problem is a non-existent issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current response is 400 that Object with name does not exist
. DRF does this on its own, so I'd say let's stick with that?
#make sure 5 issues are in the database. Does testing use a "real" database? | ||
|
||
def test_post_authentication_requirement(self): | ||
#uhhhh how do we do this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... can we fake request.session? these might need to be unit tests on the entries
and users
views.py instead though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't authenticate, then try to post.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://httpstatuses.com/422 looks pretty appropriate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've ended up attempting to get a nonce as an unauthenticated user. That gives a 404, which is a passing test.
google-api-python-client==1.5.5 | ||
httplib2==0.9.2 | ||
oauth2client==4.0.0 | ||
oauthlib==2.0.0 | ||
pyasn1==0.1.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what keeps adding pyasn.... O_O
(from what I can tell nothing actually needs it, it just gets added for... convenience?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collecting pyasn1>=0.1.7 (from oauth2client==4.0.0->-r requirements.txt (line 10))
Downloading pyasn1-0.1.9-py2.py3-none-any.whl
Collecting pyasn1-modules>=0.0.5 (from oauth2client==4.0.0->-r requirements.txt (line 10))
Downloading pyasn1_modules-0.0.8-py2.py3-none-any.whl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oauth~~~
#posting an entry with a new Issue should result in an error. Permission denied? | ||
|
||
def test_check_for_issues(self): | ||
#make sure 5 issues are in the database. Does testing use a "real" database? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it uses the real database, but we may need to verify it uses django.test so that tests on the db happen as transactions (i.e. properly independent)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow this. To rephrase my original comment though, I was hoping the migrations from the original issue generation would be in there. It looks like they are, as writing this test was trivial.
And posting tags, creators from sample form.
I don't think I can request you to review this since you opened it @Pomax but I think it's ready. |
(Though I realize I didn't implement some of the things from your comments. Digging into those now) |
class Creator(models.Model): | ||
""" | ||
A pulse entry | ||
Person responsible for doing the thing an entry is about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Person recognized as the creator of the thing an entry links out to"?
'internal_notes': 'Some internal notes' | ||
}) | ||
creatorList = json.loads(str(self.client.get('/creators/?search=A').content, 'utf-8')) | ||
self.assertEqual(creatorList, ['Alan']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does github even care about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Truly a question I've wondered for a while now
@@ -39,6 +42,9 @@ class EntrySerializer(serializers.ModelSerializer): | |||
issues = serializers.SlugRelatedField(many=True, | |||
slug_field='name', | |||
queryset=Issue.objects) | |||
creators = CreatableSlugRelatedField(many=True, | |||
slug_field='name', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: indent looks off, no idea if PEP8 has opinions here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these are comments now...
|
||
class EntryFactory(factory.DjangoModelFactory): | ||
|
||
# def __init__(self, tags=['foo','bar']): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a bit of commenting in this file that we can probably clean up.
|
||
class TestEntryView(TestCase): | ||
def setUp(self): | ||
self.entries = [EntryFactory() for i in range(2)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmm this looks identical to the code in creators - worth DRYing out?
|
||
values = json.loads(str(self.client.get('/nonce/').content, 'utf-8')) | ||
# Set up with some curated data for all tests to use | ||
postresponse = self.client.post('/entries/', data={ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably capture this data
object, and then for each test copy it and modify as needed.
for user in self.users: | ||
user.save() | ||
|
||
# def test_list_users_returns_user_data(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these bad tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure where these came from, but yeah I don't think they're useful. Removing.
google-api-python-client==1.5.5 | ||
httplib2==0.9.2 | ||
oauth2client==4.0.0 | ||
oauthlib==2.0.0 | ||
pyasn1==0.1.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oauth~~~
Ok, I've updated this branch @Pomax. Take another look? |
R+, lets merge this in |
forming a PR because it lets me actually conveniently comment on tests =)