Added unit tests for djblets.auth.views.register
Review Request #10832 — Created Jan. 18, 2020 and submitted
Added unit tests for djblets.auth.views.register
Changes for latest revision
Change 1:
Created new templates for testing:base.html
andregister.html
.
This was to ensure that therender
function withinviews.py
could use a
template when running the test cases.Change 2:
Added url to view mapping fordjblets/auth/register
so that the url could be
used for http requests (get/post) to test responses. In previous revisions,RequestFactory
was used, and the functionregister()
fromviews.py
was directly called within the test cases.Change 3:
Used Django's test client instead ofRequestFactory
. This ensures that the register view actually
works as intended when its mapped url is called (rather than callingregister()
directly.
- When register receives an http request that is not a POST request
- When register receives a POST request, but an invalid form is passed
- When register receives a POST request and a valid form is passed
Description | From | Last Updated |
---|---|---|
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (101 > 79 characters) |
reviewbot | |
E501 line too long (86 > 79 characters) |
reviewbot | |
E501 line too long (91 > 79 characters) |
reviewbot | |
E501 line too long (81 > 79 characters) |
reviewbot | |
E501 line too long (92 > 79 characters) |
reviewbot | |
Docstring summaries must always begin on the """ line. If the docstring is single-line, the trailing """ should also be … |
chipx86 | |
The very first import on any module needs to be: from __future__ import unicode_literals This turns on Unicode strings on … |
chipx86 | |
Two important things: There must be only a single blank line between groups. Imports must always be alphabetical, so it's … |
chipx86 | |
Since we support Python 2.x, you'll need to always inherit from something. In this case, object. |
chipx86 | |
Same note as above. This is missing a period, and doesn't really tell us anything about the class. |
chipx86 | |
This is implied in a class, so you shouldn't need it. |
chipx86 | |
Same note about inheritence here. |
chipx86 | |
We have potentially thousands of unit tests running (in Review Board especially), so it's always important that our docstrings contain … |
chipx86 | |
The way we want to use this is to actually have SpyAgency mixed in to the parent class, like: class … |
chipx86 | |
We try to avoid using call_original=False where possible, because this masks other issues. Let's try without these and see if … |
chipx86 | |
It's generally best that we avoid calling views directly. Or mocking a request, for that matter. Instaed, we should take … |
chipx86 | |
You won't need this once you inherit from SpyAgency. |
chipx86 | |
No blank line here. |
chipx86 | |
W292 no newline at end of file |
reviewbot |
- Commit:
-
9a3b05a0fecc579525d0813a72a978791eab12153310c96d6f8d4ac3187ab69374c1317f5deb84fc
- Diff:
-
Revision 2 (+138)
Checks run (2 succeeded)
-
Thanks for the change!
I have a handful of comments here, which really focuses on a few key areas:
- Getting things in shape for our documentation, coding, and unit test conventions.
- Best practices for working with kgb and spies.
- A better approach for how to test views.
The end result will be a file that's probably half the size :) Come talk to me if you have any questions!
-
Docstring summaries must always begin on the
"""
line. If the docstring is single-line, the trailing"""
should also be on the same line.You'll want to go through our Writing Python Docs guide and make sure all your docstrings follow the standards we list there.
-
The very first import on any module needs to be:
from __future__ import unicode_literals
This turns on Unicode strings on Python 2.x, which ensures that 2.x and 3.x will work the same way.
There must be a blank line between this and any other imports.
-
Two important things:
- There must be only a single blank line between groups.
- Imports must always be alphabetical, so it's clear where we should put new imports. This applies to the
from ...
part and to the list of things imported (import ...
).
-
-
-
-
-
We have potentially thousands of unit tests running (in Review Board especially), so it's always important that our docstrings contain enough information to help us know what's being tested.
As such, we want to list the class/function/whatever being tested in the docstring, like:
"""Testing register view when ..."""
The test method must also contain information as well, so
test_register_with_...
. It must also be completely lowercase. So I'd call thistest_register_without_post_method
. -
The way we want to use this is to actually have
SpyAgency
mixed in to the parent class, like:class ViewTests(SpyAgency, TestCase):
Then we'd call
self.spy_on(...)
.This helps take care of some important cleanup tasks.
-
We try to avoid using
call_original=False
where possible, because this masks other issues. Let's try without these and see if anything breaks. If something does, it means our test state isn't complete. -
It's generally best that we avoid calling views directly. Or mocking a request, for that matter. Instaed, we should take advantage of what Django offers, which is a nice test client that simulates HTTP requests. This ensures that we are working with real-world objects, testing real-world view/response paths.
(Documentation on the test client)
This is done by running code like:
response = self.client.get('/some-url/') # or response = self.client.post( '/some-url/', { 'key': 'value', ... })
The latter for specifying form fields/POST data.
You'll set up these URLs by defining a
urls
attribute to a module you'll need to create alongside this file. See the documentation.We also want to make sure we're always checking the responses, at a minimum checking the HTTP status code, but also checking whether it redirected to the right place (in this case).
-
-
-
A few minor things on the review request itself:
1. Can you add the bug ticket number (4588) to the "Bugs" field of this review request? It'll automatically link to the Splat ticket.
2. Testing info should be listed in "Testing Done", not "Description".
3. Can you wrap your "Description" and "Testing Done" to 80 chars to make it more readable?
- Change Summary:
-
Used test client instead of RequestFactory
- Description:
-
~ Added unit tests for djblets.auth.views.register. There's currently no test coverage for the register view; by adding unit tests, we can make sure that this doesn't break in the future. Tested the following cases:
~ Added unit tests for djblets.auth.views.register
- - - When register receives an http request that is not a POST request
- - When register receives a POST request, but an invalid form is passed
- - When register receives a POST request and a valid form is passed
- Testing Done:
-
+ - When register receives an http request that is not a POST request
+ - When register receives a POST request, but an invalid form is passed
+ - When register receives a POST request and a valid form is passed
- Bugs:
-
- Commit:
3310c96d6f8d4ac3187ab69374c1317f5deb84fc1de1e18ac48d23894b3b6d8e7f94fdfdcd99732c- Diff:
Revision 3 (+100)
- Change Summary:
-
Fixed "W292 no newline at end of file" lint issue
- Commit:
-
1de1e18ac48d23894b3b6d8e7f94fdfdcd99732c617418ad81ca5f1edb0456c9e64fdca6218de0bb
Checks run (2 succeeded)
- Description:
-
Added unit tests for djblets.auth.views.register
+ + Changes for latest revision
+ Change 1: + Created new templates for testing: base.html
andregister.html
.+ This was to ensure that the render
function withinviews.py
could use a+ template when running the test cases. + + Change 2:
+ Added url to view mapping for djblets/auth/register
so that the url could be+ used for http requests (get/post) to test responses. In previous revisions, RequestFactory
+ was used, and the function register()
fromviews.py
was directly called within the test cases.+ + Change 3:
+ Used Django's test client instead of RequestFactory
. This ensures that the register view actually+ works as intended when its mapped url is called (rather than calling register()
directly.
-
-
@Hannah& Kat
Yes, they do.
setUp()
andtearDown()
are optional functions. We can use them to factor out the code common to all tests.setUp()
runs before each test in the class.tearDown()
runs after each test in the class.- Testcases are often used in cooperative multiple inheritance, so we should call
super
in these methods so the base class'ssetUp
andtearDown
methods also get called.
references:
1.(https://riptutorial.com/python/example/13280/test-setup-and-teardown-within-a-unittest-testcase)