From 8d69656d4a3d656115cfe25211225beef678949e Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 22:23:13 +0100 Subject: [PATCH 01/10] Add docstrings to nav_tree.tests.test_models --- conman/nav_tree/tests/test_models.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/conman/nav_tree/tests/test_models.py b/conman/nav_tree/tests/test_models.py index 09ef26d..61f19e3 100644 --- a/conman/nav_tree/tests/test_models.py +++ b/conman/nav_tree/tests/test_models.py @@ -31,6 +31,7 @@ class NodeTest(TestCase): def test_fields(self): + """Check the Node model has the expected fields.""" expected = ( 'id', 'noderedirect', @@ -59,6 +60,7 @@ def test_create_leaf_without_slug(self): class NodeUniqueness(TestCase): def test_unique_slug_per_parent(self): + """Two Nodes cannot share the same slug and parent Node.""" slug = 'slug' root_node = RootNodeFactory.create() NodeFactory.create(slug=slug, parent=root_node) @@ -67,6 +69,7 @@ def test_unique_slug_per_parent(self): NodeFactory.create(slug=slug, parent=root_node) def test_unique_root_url(self): + """Only one Node can exist with an empty slug.""" Node.objects.create(slug='') with self.assertRaises(IntegrityError): @@ -201,6 +204,7 @@ class NodeManagerBestMatchForPathTest(TestCase): LIMIT 1 """ def test_get_root(self): + """Check a Root Node matches a simple '/' path.""" root = RootNodeFactory.create() with self.assertNumQueries(1): node = Node.objects.best_match_for_path('/') @@ -208,6 +212,7 @@ def test_get_root(self): self.assertEqual(node, root) def test_get_leaf(self): + """Check a Node with a slug matches a path of that slug.""" leaf = ChildNodeFactory.create(slug='leaf') with self.assertNumQueries(1): @@ -216,6 +221,7 @@ def test_get_leaf(self): self.assertEqual(node, leaf) def test_get_leaf_on_branch(self): + """Check a Node matches a path containing its slug and parent's slug.""" branch = ChildNodeFactory.create(slug='branch') leaf = NodeFactory.create(slug='leaf', parent=branch) @@ -225,6 +231,7 @@ def test_get_leaf_on_branch(self): self.assertEqual(node, leaf) def test_get_branch_with_leaf(self): + """Check a Branch Node matches a path of its slug even if a Leaf exists.""" branch = ChildNodeFactory.create(slug='branch') NodeFactory.create(slug='leaf', parent=branch) @@ -255,11 +262,13 @@ class NodeManagerBestMatchForBrokenPathTest(TestCase): LIMIT 1 """ def test_throw_error_without_match(self): + """Check Node.DoesNotExist is raised if no Root Node exists.""" with self.assertNumQueries(1): with self.assertRaises(Node.DoesNotExist): Node.objects.best_match_for_path('/') def test_fall_back_to_root(self): + """Check the Root Node matches when no better Node is available.""" root = RootNodeFactory.create() with self.assertNumQueries(1): @@ -268,6 +277,7 @@ def test_fall_back_to_root(self): self.assertEqual(node, root) def test_fall_back_to_branch(self): + """Check a Branch Node matches when no Leaf Node matches.""" branch = ChildNodeFactory.create(slug='branch') with self.assertNumQueries(1): @@ -278,6 +288,7 @@ def test_fall_back_to_branch(self): class NodeGetHandlerClassTest(TestCase): def test_get_handler_class(self): + """A Node's handler is looked up from the handler's path.""" handler_class = handlers.BaseHandler node = NodeFactory.build() node.handler = handler_class.path() @@ -311,6 +322,9 @@ def test_get_handler_again(self): class NodeHandleTest(TestCase): def test_handle(self): + """ + Node delegates a request to its handler after stripping its url from the path. + """ node = NodeFactory.build(url='/branch/') node.get_handler_class = mock.MagicMock() request = mock.Mock() @@ -338,10 +352,12 @@ def test_child_str(self): class NodeCheckTest(TestCase): def test_node_class(self): + """The Node class does not require a handler attribute.""" errors = Node.check() self.assertEqual(errors, []) def test_subclass_with_handler(self): + """A subclass of Node must have a handler attribute.""" class NodeWithHandler(Node): handler = 'has.been.set' @@ -349,6 +365,7 @@ class NodeWithHandler(Node): self.assertEqual(errors, []) def test_subclass_without_handler(self): + """A subclass of Node without a handler fails Node.check.""" class NodeWithoutHandler(Node): pass # handler not set From ba0b4ebeeabce88a40d9f86fe0384fb5471f6046 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 22:28:40 +0100 Subject: [PATCH 02/10] Add docstring to nav_tree.tests.test_urls --- conman/nav_tree/tests/test_urls.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/conman/nav_tree/tests/test_urls.py b/conman/nav_tree/tests/test_urls.py index 2eb5cb2..f363af7 100644 --- a/conman/nav_tree/tests/test_urls.py +++ b/conman/nav_tree/tests/test_urls.py @@ -24,17 +24,22 @@ def test_double_slash_url(self): self.assert_url_uses_router('//') def test_root_url(self): + """The root url is resolved using views.node_router.""" self.assert_url_uses_router('/') def test_child_url(self): + """A child url is resolved using views.node_router.""" self.assert_url_uses_router('/slug/') def test_nested_child_url(self): + """A nested child url is resolved using views.node_router.""" self.assert_url_uses_router('/foo/bar/') def test_numerical_url(self): + """A numeric url is resolved using views.node_router.""" self.assert_url_uses_router('/meanings/42/') def test_without_trailing_slash(self): + """A url without a trailing slash is not resolved by views.node_router.""" with self.assertRaises(Resolver404): self.assert_url_uses_router('/fail') From 4a3b2e55f6025532adffa56523c7d30bba611643 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 22:41:08 +0100 Subject: [PATCH 03/10] Add docstrings to nav_tree.tests.test_utils --- conman/nav_tree/tests/test_utils.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/conman/nav_tree/tests/test_utils.py b/conman/nav_tree/tests/test_utils.py index 706c336..9d072c7 100644 --- a/conman/nav_tree/tests/test_utils.py +++ b/conman/nav_tree/tests/test_utils.py @@ -5,6 +5,7 @@ class TestSplitPath(TestCase): def test_split_path(self): + """split_path returns a list of all sub-paths of a url path.""" paths = utils.split_path('/a/path/with/many/parts/') expected = [ '/', @@ -17,16 +18,19 @@ def test_split_path(self): self.assertCountEqual(paths, expected) # Order does not matter def test_split_empty_path(self): + """An empty path has sub-path '/'.""" paths = utils.split_path('') expected = ['/'] self.assertCountEqual(paths, expected) def test_split_root_path(self): + """The root path '/' has sub-path '/'.""" paths = utils.split_path('/') expected = ['/'] self.assertCountEqual(paths, expected) def test_split_path_with_dots(self): + """split_path does no special processing on a path containing dots.""" paths = utils.split_path('/path/../') expected = [ '/', @@ -38,18 +42,22 @@ def test_split_path_with_dots(self): class TestImportFromDottedPath(TestCase): def test_empty(self): + """An empty path cannot be imported.""" with self.assertRaises(ValueError): utils.import_from_dotted_path('') def test_too_short(self): + """A path with only one component cannot be imported.""" with self.assertRaises(ValueError): utils.import_from_dotted_path('antigravity') def test_import_module(self): + """A module can be imported by dotted path.""" result = utils.import_from_dotted_path('conman.nav_tree.utils') self.assertEqual(result, utils) def test_import_class(self): + """A class can be imported by dotted path.""" this_test = 'conman.nav_tree.tests.test_utils.TestImportFromDottedPath' result = utils.import_from_dotted_path(this_test) self.assertEqual(result, self.__class__) From d5572f698938a5d2e10a888180032bb963ee2bec Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 22:53:39 +0100 Subject: [PATCH 04/10] Add docstrings to nav_tree.tests.test_views --- conman/nav_tree/tests/test_views.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/conman/nav_tree/tests/test_views.py b/conman/nav_tree/tests/test_views.py index 54628f8..cc78d2e 100644 --- a/conman/nav_tree/tests/test_views.py +++ b/conman/nav_tree/tests/test_views.py @@ -10,6 +10,7 @@ class RouterTest(TestCase): """Test that `node_router` correctly deals with the urls handed to it.""" def test_root(self): + """node_router delegates to the Root Node's handler for an empty url.""" url = '' factories.NodeFactory.create() request = mock.MagicMock() @@ -21,6 +22,7 @@ def test_root(self): self.assertEqual(response, handle(request, '/' + url)) def test_complex_url(self): + """node_router finds the url's best match and delegates to its handler.""" url = 'slug/42/foo/bar/' factories.NodeFactory.create() request = mock.MagicMock() @@ -35,6 +37,7 @@ def test_complex_url(self): class RouterIntegrationTest(TestCase): """Test that `node_router` is correctly handed urls.""" def test_root_url(self): + """The Root Node handler is passed the correct request and root url.""" url = '/' factories.NodeFactory.create() handle_path = 'conman.nav_tree.models.Node.handle' @@ -45,6 +48,7 @@ def test_root_url(self): handle.assert_called_with(response.wsgi_request, url) def test_complex_url(self): + """The correct request and url is passed through to the node's handler.""" url = '/slug/42/foo/bar/' factories.NodeFactory.create() handle_path = 'conman.nav_tree.models.Node.handle' From c1931b37ded8ffb6282015933412809c488de58e Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:07:05 +0100 Subject: [PATCH 05/10] Add docstrings to pages.tests.test_handlers --- conman/pages/tests/test_handlers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conman/pages/tests/test_handlers.py b/conman/pages/tests/test_handlers.py index 35ce907..f477ba8 100644 --- a/conman/pages/tests/test_handlers.py +++ b/conman/pages/tests/test_handlers.py @@ -6,9 +6,11 @@ class TestPageHandler(TestCase): def test_heritage(self): + """PageHandler subclasses SimpleHandler.""" self.assertTrue(issubclass(handlers.PageHandler, SimpleHandler)) def test_view(self): + """PageHandler uses the PageDetail view.""" view = handlers.PageHandler.view expected = views.PageDetail.as_view() From e41a07b92db83ea87628f0887f3d2b5e6c4d47f1 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:09:02 +0100 Subject: [PATCH 06/10] Add docstring to pages.tests.test_models --- conman/pages/tests/test_models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conman/pages/tests/test_models.py b/conman/pages/tests/test_models.py index 092b27d..d7dd8c9 100644 --- a/conman/pages/tests/test_models.py +++ b/conman/pages/tests/test_models.py @@ -6,6 +6,7 @@ class PageTest(TestCase): def test_fields(self): + """Check Page has Node's fields and a few of its own.""" expected = ( 'id', 'node_ptr', From 0ed6ed657911d6e4aedf532560e0587d3081c6d7 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:21:32 +0100 Subject: [PATCH 07/10] Add docstrings to pages.tests.test_views --- conman/pages/tests/test_views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conman/pages/tests/test_views.py b/conman/pages/tests/test_views.py index 38638ec..30bfdce 100644 --- a/conman/pages/tests/test_views.py +++ b/conman/pages/tests/test_views.py @@ -7,6 +7,7 @@ class TestPageDetail(RequestTestCase): def test_get_object(self): + """PageDetail displays the page instance passed in the node kwarg.""" request = self.create_request() page = factories.PageFactory.create() @@ -20,6 +21,7 @@ class TestPageDetailIntegration(TestCase): view = views.PageDetail def test_get(self): + """A page's content is rendered at its url.""" page = factories.PageFactory.create(content='This is a test') response = self.client.get(page.url) self.assertIn(page.content, response.rendered_content) From db26c88a016c5347c1bef2ccf1f7c2ab0061e52f Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:24:32 +0100 Subject: [PATCH 08/10] Add docstrings to redirects.tests.test_handlers --- conman/redirects/tests/test_handlers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conman/redirects/tests/test_handlers.py b/conman/redirects/tests/test_handlers.py index 84ff593..45c55c1 100644 --- a/conman/redirects/tests/test_handlers.py +++ b/conman/redirects/tests/test_handlers.py @@ -7,9 +7,11 @@ class TestNodeRedirectHandler(TestCase): def test_heritage(self): + """NodeRedirectHandler sublcasses SimpleHandler.""" self.assertTrue(issubclass(NodeRedirectHandler, SimpleHandler)) def test_view(self): + """NodeRedirectHandler uses the NodeRedirectView.""" view = NodeRedirectHandler.view expected = NodeRedirectView.as_view() From 4046cbf99e692cf67cea6e8e14c199a6bddf1f44 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:29:23 +0100 Subject: [PATCH 09/10] Add docstrings for redirects.tests.test_models --- conman/redirects/tests/test_models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conman/redirects/tests/test_models.py b/conman/redirects/tests/test_models.py index 4cf3d14..696c07d 100644 --- a/conman/redirects/tests/test_models.py +++ b/conman/redirects/tests/test_models.py @@ -7,6 +7,7 @@ class NodeRedirectTest(TestCase): def test_fields(self): + """NodeRedirect has Node's fields and some specific to redirects.""" expected = ( 'id', 'node_ptr', @@ -22,6 +23,7 @@ def test_fields(self): class NodeRedirectUnicodeMethodTest(TestCase): """We should get something nice when RedirectNode is cast to string""" def test_str(self): + """The str of a NodeRedirect identifies it by class and url.""" leaf = ChildNodeRedirectFactory.create(slug='leaf') self.assertEqual(str(leaf), 'NodeRedirect @ /leaf/') From 48c20047c222f7a948dd65f151a174226a401983 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Tue, 7 Oct 2014 23:39:13 +0100 Subject: [PATCH 10/10] Refactor redirects.tests.test_views * Split tests for permanent/temporary redirects from test for the redirect's target url * Simplify integration test to focus on integration * Remove essentially duplicate integration test * Add docstrings --- conman/redirects/tests/test_views.py | 40 ++++++++++------------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/conman/redirects/tests/test_views.py b/conman/redirects/tests/test_views.py index 7d89f28..5dc0b96 100644 --- a/conman/redirects/tests/test_views.py +++ b/conman/redirects/tests/test_views.py @@ -12,25 +12,26 @@ def setUp(self): self.request = self.create_request() self.view = views.NodeRedirectView.as_view() + def test_target(self): + """NodeRedirectView redirects to the target's url.""" + node = ChildNodeRedirectFactory.create(target=self.target) + response = self.view(self.request, node=node) + + self.assertEqual(response['Location'], self.target.url) + def test_permanent(self): - node = ChildNodeRedirectFactory.create( - target=self.target, - permanent=True, - ) + """A permanent redirect has status_code 301.""" + node = ChildNodeRedirectFactory.create(permanent=True) response = self.view(self.request, node=node) self.assertEqual(response.status_code, 301) - self.assertEqual(response['Location'], self.target.url) def test_temporary(self): - node = ChildNodeRedirectFactory.create( - target=self.target, - permanent=False, - ) + """A temporary redirect has status_code 302.""" + node = ChildNodeRedirectFactory.create(permanent=False) response = self.view(self.request, node=node) self.assertEqual(response.status_code, 302) - self.assertEqual(response['Location'], self.target.url) class TestNodeRedirectViewIntegration(TestCase): @@ -38,22 +39,9 @@ def setUp(self): self.target = ChildNodeFactory.create() self.expected = 'http://testserver' + self.target.url - def test_permanent(self): - node = ChildNodeRedirectFactory.create( - target=self.target, - permanent=True, - ) - response = self.client.get(node.url) - - self.assertEqual(response.status_code, 301) - self.assertEqual(response['Location'], self.expected) - - def test_temporary(self): - node = ChildNodeRedirectFactory.create( - target=self.target, - permanent=False, - ) + def test_access_redirect(self): + """Accessing a NodeRedirect's url redirects to its target's url.""" + node = ChildNodeRedirectFactory.create(target=self.target) response = self.client.get(node.url) - self.assertEqual(response.status_code, 302) self.assertEqual(response['Location'], self.expected)