Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Url audit #107

Merged
merged 2 commits into from

2 participants

@Osmose
Owner

Removes URLs that won't be needed for the new site and fixes the tests to accommodate these changes.

flicks/base/tests/test_views.py
@@ -1,7 +1,7 @@
from funfactory.urlresolvers import reverse
from nose.tools import eq_
-from flicks.base.tests import TestCase
+from flicks.base.tests import skip, TestCase
@pmclanahan Owner

Django includes unittest2, which includes a skip decorator.

from django.utils.unittest import skip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pmclanahan pmclanahan commented on the diff
flicks/videos/tests/test_forms.py
@@ -95,17 +98,19 @@ def test_invalid_content_type(self, head):
valid.
"""
head.return_value = self._response(200, 'invalid/type; charset=UTF-8')
- form = self._form();
@pmclanahan Owner

lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pmclanahan
Owner

Apart from the mass destruction of urls, which was apparently the goal, this looks good.

I won't say you need to delete the custom skip decorator since it was already in there, but it seems redundant given that unittest2 is available.

r+

Osmose added some commits
@Osmose Osmose Fix Bug 822364: Remove old URLs from urlconfs. d5b67e2
@Osmose Osmose Bug 822364: Skip tests for views that aren't in the urlconf.
- Skips tests that rely on the urls that were removed from the urlconf.
- Fixes an issue with the 404 and 500 pages where they would cause test
  failures when encountered during a test because, during tests, the 
  built-in error views are used, and they don't use RequestContext, 
  which the error templates relied on.
416f603
@Osmose Osmose merged commit 416f603 into mozilla:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 11, 2013
  1. @Osmose
  2. @Osmose

    Bug 822364: Skip tests for views that aren't in the urlconf.

    Osmose authored
    - Skips tests that rely on the urls that were removed from the urlconf.
    - Fixes an issue with the 404 and 500 pages where they would cause test
      failures when encountered during a test because, during tests, the 
      built-in error views are used, and they don't use RequestContext, 
      which the error templates relied on.
This page is out of date. Refresh to see the latest.
View
2  flicks/base/templates/404.html
@@ -12,7 +12,7 @@
{% trans %}
Sorry, but we couldn't find the page you're looking for.
{% endtrans %}
- {% trans home_url=url('flicks.videos.recent') %}
+ {% trans home_url=url('flicks.base.home') %}
<a href="{{ home_url }}">Go back</a> to the home page.
{% endtrans %}
</p>
View
2  flicks/base/templates/500.html
@@ -12,7 +12,7 @@
{% trans %}
Sorry, but something went wrong.
{% endtrans %}
- {% trans home_url=url('flicks.videos.recent') %}
+ {% trans home_url=url('flicks.base.home') %}
<a href="{{ home_url }}">Go back</a> to the home page.
{% endtrans %}
</p>
View
20 flicks/base/templates/base.html
@@ -1,3 +1,9 @@
+{# Substitute in an empty settings object if it is not available (error handlers
+ during tests). #}
+{% if not settings %}
+ {% set settings = {} %}
+{% endif %}
+
<!DOCTYPE html>
<html LANG="{{ LANG }}" dir="{{ DIR }}">
<head>
@@ -23,7 +29,7 @@
<!--[if lt IE 9]>
<script src="{{ MEDIA_URL }}js/libs/html5.js"></script>
<![endif]-->
-
+
<link rel="stylesheet" media="all" type="text/css" href="//www.mozilla.org/tabzilla/media/css/tabzilla.css">
{% block site_css %}
@@ -55,7 +61,7 @@
data-token="{{ csrf_token }}"
{% block body_data %}{% endblock %}>
<div id="page">
-
+
{% include "shared/header.html" %}
<div class="content-wrapper">
@@ -67,7 +73,7 @@
{% block footer %}
<footer id="footer">
<div class="wrap">
-
+
<aside class="sponsors">
<h4>Our Sponsors</h4>
<ul>
@@ -75,7 +81,7 @@
<li><a href="http://panavision.com" rel="external"><img src="/media/img/sponsors/sponsor-panavision.png" alt="Panavision" width="100" height="52"></a></li>
</ul>
</aside>
-
+
<div class="util">
<nav id="nav-social-foot" class="nav-social">
<h4>{{ _('Share Firefox Flicks') }}</h4>
@@ -85,18 +91,18 @@
<li><a href="http://twitter.com/firefoxflicks" title="Twitter" class="twitter">Twitter</a></li>
</ul>
</nav>
-
+
<aside class="firefoxos">
<h4>{{ _('Learn about Firefox OS') }}</h4>
<p><a href="http://www.mozilla.org/firefoxos/">{{ _('Firefox OS') }}</a></p>
</aside>
</div>
-
+
<nav id="nav-fineprint">
<ul>
<li><a href="http://www.mozilla.org/en-US/privacy-policy.html">{{ _('Privacy Policy') }}</a></li>
<li><a href="http://www.mozilla.org/about/legal.html">{{ _('Legal') }}</a></li>
- <li><a href="{{ url('flicks.base.partners') }}">{{ _('Partners') }}</a></li>
+ <li><a href="#">{{ _('Partners') }}</a></li>
<li><a href="#">{{ _('Other Languages') }}</a></li>
</ul>
</nav>
View
4 flicks/base/templates/home.html
@@ -52,7 +52,7 @@ <h1 class="section-title">{{ _('Bring the small screen to the big screen') }}</h
</li>
<li>
<h3>{{ _('Submissions open February 13') }}</h3>
- <p><a href="{{ url('flicks.base.faq') }}" class="go">{{ _('Learn more in the FAQ') }}</a></p>
+ <p><a href="#" class="go">{{ _('Learn more in the FAQ') }}</a></p>
</li>
<li>
<h3>{{ _('Start thinking about your flick') }}</h3>
@@ -105,7 +105,7 @@ <h1 class="section-title">{{ _('What is Firefox Flicks?') }}</h1>
and get answers to all your other questions.
{% endtrans %}
</p>
- <p><a href="{{ url('flicks.base.faq') }}" class="button">{{ _('Read the FAQ') }}</a></p>
+ <p><a href="#" class="button">{{ _('Read the FAQ') }}</a></p>
</div>
</div>
View
3  flicks/base/tests/test_views.py
@@ -1,9 +1,12 @@
+from django.utils.unittest import skip
+
from funfactory.urlresolvers import reverse
from nose.tools import eq_
from flicks.base.tests import TestCase
+@skip
class HomeTests(TestCase):
def _get(self):
with self.activate('en-US'):
View
6 flicks/base/urls.py
@@ -4,10 +4,4 @@
urlpatterns = patterns('',
url(r'^/?$', views.home, name='flicks.base.home'),
- url(r'^creative/?$', views.creative, name='flicks.base.creative'),
- url(r'^faq/?$', views.faq, name='flicks.base.faq'),
- url(r'^judges/?$', views.judges, name='flicks.base.judges'),
- url(r'^partners/?$', views.partners, name='flicks.base.partners'),
- url(r'^prizes/?$', views.prizes, name='flicks.base.prizes'),
- url(r'^rules/?$', views.rules, name='flicks.base.rules'),
)
View
7 flicks/urls.py
@@ -12,7 +12,8 @@
def error_page(request, template, status=None):
- """Render error templates, found in the root /templates directory.
+ """
+ Render error templates, found in the root /templates directory.
If no status parameter is explcitedly passed, this function assumes
your HTTP status code is the same as your template name (i.e. passing
@@ -28,10 +29,6 @@ def error_page(request, template, status=None):
urlpatterns = patterns('',
url(r'', include('flicks.base.urls')),
url(r'', include('flicks.videos.urls')),
- url(r'^users/', include('flicks.users.urls')),
-
- url(r'^logout$', 'django.contrib.auth.views.logout', {'next_page': '/'},
- name='logout'),
url(r'^admin/', include(admin.site.urls)),
)
View
3  flicks/users/tests/test_decorators.py
@@ -1,9 +1,12 @@
+from django.utils.unittest import skip
+
from funfactory.urlresolvers import reverse
from nose.tools import eq_
from flicks.base.tests import TestCase
+@skip
class ProfileRequiredTests(TestCase):
urls = 'flicks.users.tests.urls'
View
4 flicks/users/tests/test_views.py
@@ -1,6 +1,7 @@
from contextlib import nested
from django.conf import settings
+from django.utils.unittest import skip
from funfactory.urlresolvers import reverse
from nose.tools import eq_
@@ -10,6 +11,7 @@
from flicks.users.tests import mock_browserid
+@skip
class VerifyTests(TestCase):
def _post(self, email, assertion='asdf'):
with nested(self.activate('en-US'), mock_browserid(email)):
@@ -52,6 +54,7 @@ def test_has_profile(self):
self.assert_viewname_url(response['Location'], settings.LOGIN_REDIRECT)
+@skip
class EditProfileTests(TestCase):
def _post(self, **kwargs):
with self.activate('en-US'):
@@ -82,6 +85,7 @@ def test_has_profile(self):
eq_(profile.bio, '1234')
+@skip
class DetailsTests(TestCase):
def _get(self, user_id):
with self.activate('en-US'):
View
2  flicks/videos/models.py
@@ -198,7 +198,7 @@ class Award(models.Model, CachingMixin):
choices=REGION_CHOICES,
verbose_name=_lazy(u'Region'))
award_type = models.CharField(max_length=50, blank=False,
- choices = AWARD_TYPE_CHOICES)
+ choices=AWARD_TYPE_CHOICES)
objects = CachingManager()
View
11 flicks/videos/tests/__init__.py
@@ -1,10 +1,9 @@
from contextlib import contextmanager
-from functools import wraps
from django.conf import settings
from elasticutils import get_es
-from nose.plugins.skip import SkipTest
+
from flicks.videos.models import Video
@@ -32,11 +31,3 @@ def build_video(user, **kwargs):
yield video
video.delete()
-
-
-def skip_test(f):
- @wraps(f)
- def wrap(*args, **kwargs):
- raise SkipTest
- return wrap
-skip_test.__test__ = False
View
8 flicks/videos/tests/test_forms.py
@@ -1,6 +1,7 @@
from contextlib import nested
from django.conf import settings
+from django.utils.unittest import skip
import requests
from elasticutils.tests import ESTestCase
@@ -51,6 +52,7 @@ def test_invalid_search(self):
eq_(self._videos(category=''), [])
+@skip
class UploadFormTests(TestCase):
def _form(self, **kwargs):
params = {
@@ -95,7 +97,7 @@ def test_invalid_content_type(self, head):
valid.
"""
head.return_value = self._response(200, 'invalid/type; charset=UTF-8')
- form = self._form();
@pmclanahan Owner

lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ form = self._form()
eq_(form.is_valid(), False)
eq_(form.errors.keys(), ['upload_url'])
@@ -103,7 +105,7 @@ def test_invalid_content_type(self, head):
def test_no_content_type(self, head):
"""If the url does not return a content-type, the form is valid."""
head.return_value = self._response(200, None)
- form = self._form();
+ form = self._form()
eq_(form.is_valid(), True)
@patch.object(requests, 'head')
@@ -113,5 +115,5 @@ def test_success(self, head):
valid.
"""
head.return_value = self._response(200, 'video/mpeg')
- form = self._form();
+ form = self._form()
eq_(form.is_valid(), True)
View
7 flicks/videos/tests/test_tasks.py
@@ -1,12 +1,15 @@
+from django.utils.unittest import skip
+
from mock import patch
from nose.tools import eq_
from flicks.base.tests import TestCase
from flicks.videos.models import Video
from flicks.videos.tasks import add_vote, send_video_to_vidly
-from flicks.videos.tests import build_video, skip_test
+from flicks.videos.tests import build_video
+@skip
class SendVideoToVidlyTests(TestCase):
def _send(self, video, return_value):
with patch('flicks.videos.tasks.addMedia') as addMedia:
@@ -19,14 +22,12 @@ def _send(self, video, return_value):
def setUp(self):
self.user = self.build_user()
- @skip_test
def test_error(self):
"""If there's an error, the video's state should change to error."""
with build_video(self.user) as video:
video = self._send(video, None)
eq_(video.state, 'error')
- @skip_test
def test_success(self):
"""A successful upload should mark the video as pending."""
with build_video(self.user, shortlink='') as video:
View
15 flicks/videos/tests/test_views.py
@@ -2,6 +2,7 @@
from django.conf import settings
from django.core import mail
+from django.utils.unittest import skip
from funfactory.urlresolvers import reverse
from mock import patch
@@ -11,13 +12,13 @@
from flicks.videos.forms import UploadForm
from flicks.videos.models import Video
from flicks.videos.tasks import send_video_to_vidly
-from flicks.videos.tests import build_video, skip_test
+from flicks.videos.tests import build_video
from flicks.videos.util import cached_viewcount
+@skip
@patch.object(send_video_to_vidly, 'delay')
class UploadTests(TestCase):
- @skip_test
@patch.object(UploadForm, 'clean_upload_url')
def _post(self, clean_upload_url, **kwargs):
clean_upload_url.return_value = kwargs.get('url', 'http://test.com')
@@ -34,20 +35,17 @@ def _post(self, clean_upload_url, **kwargs):
def setUp(self):
self.user = self.build_user(login=True)
- @skip_test
def test_get(self, send_video_to_vidly):
"""An empty form should do nothing."""
with self.activate('en-US'):
self.client.get(reverse('flicks.videos.upload'))
eq_(send_video_to_vidly.called, False)
- @skip_test
def test_invalid_post(self, send_video_to_vidly):
"""Invalid parameters should not create a video."""
self._post(title='')
eq_(send_video_to_vidly.called, False)
- @skip_test
def test_valid_post(self, send_video_to_vidly):
"""Valid parameters should send a video to vidly."""
self._post()
@@ -56,7 +54,6 @@ def test_valid_post(self, send_video_to_vidly):
video = send_video_to_vidly.call_args[0][0]
eq_(video.user, self.user)
- @skip_test
def test_socket_timeout(self, send_video_to_vidly):
"""A socket timeout is silent to the user."""
send_video_to_vidly.side_effect = socket.timeout
@@ -67,6 +64,7 @@ def test_socket_timeout(self, send_video_to_vidly):
[t.name for t in response.templates])
+@skip
@patch.object(settings, 'VIDLY_USER_ID', '1111')
class NotifyTests(TestCase):
def _post(self, xml, key=settings.NOTIFY_KEY):
@@ -79,7 +77,6 @@ def _post(self, xml, key=settings.NOTIFY_KEY):
def setUp(self):
self.user = self.build_user()
- @skip_test
@patch.object(settings, 'NOTIFY_KEY', 'asdf')
def test_invalid_key_forbidden(self):
"""If an invalid key is provided in the URL, return a 403 and do not
@@ -99,7 +96,6 @@ def test_invalid_key_forbidden(self):
eq_(response.status_code, 403)
eq_(video.state, 'pending')
- @skip_test
def test_valid_notification(self):
"""A valid notification updates a video's state."""
xml = """<?xml version="1.0"?>
@@ -122,7 +118,6 @@ def test_valid_notification(self):
ok_(video_url in mail.outbox[0].body)
eq_(mail.outbox[0].to, [self.user.email])
- @skip_test
def test_error_notification(self):
"""An error notification updates the video's state."""
xml = """<?xml version="1.0"?>
@@ -144,6 +139,7 @@ def test_error_notification(self):
eq_(mail.outbox[0].to, [self.user.email])
+@skip
class AjaxAddViewTests(TestCase):
def _post(self, video_id=None):
params = {'video_id': video_id} if video_id is not None else {}
@@ -182,6 +178,7 @@ def test_add_view(self):
eq_(cached_viewcount(video.id), 11)
+@skip
class DetailsTests(TestCase):
def _post(self, video, **kwargs):
with self.activate('en-US'):
View
4 flicks/videos/urls.py
@@ -3,12 +3,8 @@
from flicks.videos import views
urlpatterns = patterns('',
- url(r'^winners/?$', views.winners, name='flicks.videos.winners'),
- url(r'^recent/?$', views.recent, name='flicks.videos.recent'),
url(r'^video/(?P<video_id>\d+)$', views.details,
name='flicks.videos.details'),
- url(r'^add_view/?$', views.ajax_add_view,
- name='flicks.videos.add_view'),
url(r'^video/noir/$', views.promo_video_noir,
name='flicks.videos.promo_video_noir'),
url(r'^video/dance/$', views.promo_video_dance,
Something went wrong with that request. Please try again.