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

Improved test coverage for django.contrib.sitemaps. #8208

Merged
merged 1 commit into from Jul 26, 2017

Conversation

desecho
Copy link
Contributor

@desecho desecho commented Mar 17, 2017

No description provided.

@desecho desecho force-pushed the improve_sitemaps_tests branch 2 times, most recently from 1a71ee6 to 940674b Compare March 17, 2017 20:51
@desecho desecho changed the title Improve test coverage for django.contrib.sitemaps. Improved test coverage for django.contrib.sitemaps. Mar 29, 2017
@desecho desecho force-pushed the improve_sitemaps_tests branch 2 times, most recently from a7e067f to c54c09b Compare March 31, 2017 18:11
@@ -52,6 +71,18 @@ def test_simple_sitemap_section(self):
""" % (self.base_url, date.today())
self.assertXMLEqual(response.content.decode(), expected_content)

def test_simple_sitemap_no_section(self):
response = self.client.get('/simple/sitemap-simple2.xml')
self.assertEqual(response.status_code, 404)
Copy link
Member

Choose a reason for hiding this comment

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

You can check str(response.context['exception'] is '"No sitemap available for section..." to improve these 404 tests.

@@ -27,6 +27,25 @@ def test_simple_sitemap_index(self):
""" % self.base_url
self.assertXMLEqual(response.content.decode(), expected_content)

def test_simple_sitemap_index_not_callable(self):
"A simple sitemap index can be rendered"
Copy link
Member

Choose a reason for hiding this comment

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

this is the same as the previous docstring -- can it be differentiated?

Choose a reason for hiding this comment

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

If this is exactly the same as the above test, was the goal to add a new test or should this be removed for the time being?

Copy link
Member

Choose a reason for hiding this comment

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

Only the docstring is the same, obviously the test is different.

self.assertXMLEqual(response.content.decode(), expected_content)

def test_simple_sitemap_index_paged(self):
response = self.client.get('/simple-paged/index.xml')
Copy link
Member

Choose a reason for hiding this comment

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

Add docstring?

Choose a reason for hiding this comment

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

Reference on how to add docstring?

Copy link
Member

Choose a reason for hiding this comment

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

You don't know what a docstring is? Trying googling "python docstring".

Choose a reason for hiding this comment

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

Sorry, was thinking of something else.

@desecho
Copy link
Contributor Author

desecho commented Jul 24, 2017

I made the changes.

@django django deleted a comment from gsilvapt Jul 26, 2017
@timgraham timgraham merged commit 0eefda4 into django:master Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants