From eabd6c8d27b14969d70366491b598ce4b3248d6b Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 28 Nov 2014 17:23:45 +0000 Subject: [PATCH] Quality check and test fixes. --- .../contentstore/tests/test_contentstore.py | 8 ++--- .../contentstore/tests/test_i18n.py | 10 +++--- cms/djangoapps/contentstore/tests/tests.py | 10 +++--- cms/djangoapps/contentstore/views/public.py | 2 +- .../views/tests/test_course_index.py | 4 +-- cms/templates/index.html | 2 +- common/djangoapps/student/tests/test_login.py | 2 +- .../pages/lms/login_and_register.py | 2 ++ common/test/acceptance/tests/lms/test_lms.py | 5 ++- docs/shared/conf.py | 1 - pavelib/paver_tests/test_prereqs.py | 31 +++++++++++++++++-- pavelib/prereqs.py | 2 +- 12 files changed, 55 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 27714230dc..e0122f4cd9 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1167,7 +1167,7 @@ class ContentStoreTest(ContentStoreTestCase): def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" # Create a course so there is something to view - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, '

My Courses

', @@ -1189,7 +1189,7 @@ class ContentStoreTest(ContentStoreTestCase): def test_course_index_view_with_course(self): """Test viewing the index page with an existing course""" CourseFactory.create(display_name='Robot Super Educational Course') - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, '

Robot Super Educational Course

', @@ -1604,7 +1604,7 @@ class RerunCourseTest(ContentStoreTestCase): Asserts that the given course key is in the accessible course listing section of the html and NOT in the unsucceeded course action section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 1) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 0) @@ -1613,7 +1613,7 @@ class RerunCourseTest(ContentStoreTestCase): Asserts that the given course key is in the unsucceeded course action section of the html and NOT in the accessible course listing section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 0) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 1) diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index e9e1739488..da79117220 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -44,9 +44,9 @@ class InternationalizationTest(ModuleStoreTestCase): self.client = AjaxEnabledTestClient() self.client.login(username=self.uname, password=self.password) - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains(resp, - '

My Courses

', + '

Studio Home

', status_code=200, html=True) @@ -56,13 +56,13 @@ class InternationalizationTest(ModuleStoreTestCase): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='en', ) self.assertContains(resp, - '

My Courses

', + '

Studio Home

', status_code=200, html=True) @@ -81,7 +81,7 @@ class InternationalizationTest(ModuleStoreTestCase): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='eo' ) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 9975764345..76429befa6 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -234,13 +234,13 @@ class AuthTestCase(ContentStoreTestCase): def test_private_pages_auth(self): """Make sure pages that do require login work.""" auth_pages = ( - '/course/', + '/home/', ) # These are pages that should just load when the user is logged in # (no data needed) simple_auth_pages = ( - '/course/', + '/home/', ) # need an activated user @@ -266,7 +266,7 @@ class AuthTestCase(ContentStoreTestCase): def test_index_auth(self): # not logged in. Should return a redirect. - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertEqual(resp.status_code, 302) # Logged in should work. @@ -283,7 +283,7 @@ class AuthTestCase(ContentStoreTestCase): self.login(self.email, self.pw) # make sure we can access courseware immediately - course_url = '/course/' + course_url = '/home/' resp = self.client.get_html(course_url) self.assertEquals(resp.status_code, 200) @@ -293,7 +293,7 @@ class AuthTestCase(ContentStoreTestCase): resp = self.client.get_html(course_url) # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/course/') + self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/home/') class ForumTestCase(CourseTestCase): diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 597bb5e187..7bc6545868 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -66,6 +66,6 @@ def login_page(request): def howitworks(request): "Proxy view" if request.user.is_authenticated(): - return redirect('/course/') + return redirect('/home/') else: return render_to_response('howitworks.html', {}) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 965879e154..32b5ccbf2c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -42,7 +42,7 @@ class TestCourseIndex(CourseTestCase): """ Test getting the list of courses and then pulling up their outlines """ - index_url = '/course/' + index_url = '/home/' index_response = authed_client.get(index_url, {}, HTTP_ACCEPT='text/html') parsed_html = lxml.html.fromstring(index_response.content) course_link_eles = parsed_html.find_class('course-link') @@ -68,7 +68,7 @@ class TestCourseIndex(CourseTestCase): # Add a library: lib1 = LibraryFactory.create() - index_url = '/course/' + index_url = '/home/' index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') parsed_html = lxml.html.fromstring(index_response.content) library_link_elements = parsed_html.find_class('library-link') diff --git a/cms/templates/index.html b/cms/templates/index.html index 64d78ffeda..378dff06bd 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -2,7 +2,7 @@ <%inherit file="base.html" /> <%def name="online_help_token()"><% return "home" %> -<%block name="title">${_("My Courses")} +<%block name="title">${_("Studio Home")} <%block name="bodyclass">is-signedin index view-dashboard <%block name="requirejs"> diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 828e951cbb..8cf07e3a80 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -482,7 +482,7 @@ class LoginOAuthTokenMixin(object): self._setup_user_response(success=True) response = self.client.post(self.url, {"access_token": "dummy"}) self.assertEqual(response.status_code, 204) - self.assertEqual(self.client.session['_auth_user_id'], self.user.id) + self.assertEqual(self.client.session['_auth_user_id'], self.user.id) # pylint: disable=no-member def test_invalid_token(self): self._setup_user_response(success=False) diff --git a/common/test/acceptance/pages/lms/login_and_register.py b/common/test/acceptance/pages/lms/login_and_register.py index 3714179858..33d5779a01 100644 --- a/common/test/acceptance/pages/lms/login_and_register.py +++ b/common/test/acceptance/pages/lms/login_and_register.py @@ -246,6 +246,7 @@ class CombinedLoginAndRegisterPage(PageObject): def wait_for_errors(self): """Wait for errors to be visible, then return them. """ def _check_func(): + """Return success status and any errors that occurred.""" errors = self.errors return (bool(errors), errors) return Promise(_check_func, "Errors are visible").fulfill() @@ -259,6 +260,7 @@ class CombinedLoginAndRegisterPage(PageObject): def wait_for_success(self): """Wait for a success message to be visible, then return it.""" def _check_func(): + """Return success status and any errors that occurred.""" success = self.success return (bool(success), success) return Promise(_check_func, "Success message is visible").fulfill() diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 94dd57d64a..cf3be2d193 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -119,7 +119,7 @@ class LoginFromCombinedPageTest(UniqueCourseTest): def test_password_reset_success(self): # Create a user account - email, password = self._create_unique_user() + email, password = self._create_unique_user() # pylint: disable=unused-variable # Navigate to the password reset form and try to submit it self.login_page.visit().password_reset(email=email) @@ -141,6 +141,9 @@ class LoginFromCombinedPageTest(UniqueCourseTest): ) def _create_unique_user(self): + """ + Create a new user with a unique name and email. + """ username = "test_{uuid}".format(uuid=self.unique_id[0:6]) email = "{user}@example.com".format(user=username) password = "password" diff --git a/docs/shared/conf.py b/docs/shared/conf.py index dfedf01ca8..992f0e54f3 100644 --- a/docs/shared/conf.py +++ b/docs/shared/conf.py @@ -22,7 +22,6 @@ # ----------------------------------------------------------------------------- import os -import sys BASEDIR = os.path.dirname(os.path.abspath(__file__)) diff --git a/pavelib/paver_tests/test_prereqs.py b/pavelib/paver_tests/test_prereqs.py index e4586d1843..47437ee907 100644 --- a/pavelib/paver_tests/test_prereqs.py +++ b/pavelib/paver_tests/test_prereqs.py @@ -1,12 +1,21 @@ - import os import unittest from pavelib.prereqs import no_prereq_install class TestPaverPrereqInstall(unittest.TestCase): - + """ + Test the status of the NO_PREREQ_INSTALL variable, its presence and how + paver handles it. + """ def check_val(self, set_val, expected_val): + """ + Verify that setting the variable to a certain value returns + the expected boolean for it. + + As environment variables are only stored as strings, we have to cast + whatever it's set at to a boolean that does not violate expectations. + """ _orig_environ = dict(os.environ) os.environ['NO_PREREQ_INSTALL'] = set_val self.assertEqual( @@ -21,19 +30,37 @@ class TestPaverPrereqInstall(unittest.TestCase): os.environ.update(_orig_environ) def test_no_prereq_install_true(self): + """ + Ensure that 'true' will be True. + """ self.check_val('true', True) def test_no_prereq_install_false(self): + """ + Ensure that 'false' will be False. + """ self.check_val('false', False) def test_no_prereq_install_True(self): + """ + Ensure that 'True' will be True. + """ self.check_val('True', True) def test_no_prereq_install_False(self): + """ + Ensure that 'False' will be False. + """ self.check_val('False', False) def test_no_prereq_install_0(self): + """ + Ensure that '0' will be False. + """ self.check_val('0', False) def test_no_prereq_install_1(self): + """ + Ensure that '1' will be True. + """ self.check_val('1', True) diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 0a334a8152..220a5387c4 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -41,7 +41,7 @@ def no_prereq_install(): try: return vals[val] - except: + except KeyError: return False