diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 3e78b40b56..cc3353c661 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -1,11 +1,12 @@ """ This test file will test registration, login, activation, and session activity timeouts """ +from __future__ import print_function import datetime import time -import unittest import mock +import pytest from ddt import data, ddt, unpack from django.conf import settings from django.contrib.auth.models import User @@ -15,6 +16,7 @@ from django.test import TestCase from django.test.utils import override_settings from freezegun import freeze_time from pytz import UTC +from six.moves import xrange from contentstore.models import PushNotificationConfig from contentstore.tests.test_course_settings import CourseTestCase @@ -85,6 +87,36 @@ class ContentStoreTestCase(ModuleStoreTestCase): self.assertTrue(user(email).is_active) +@pytest.mark.django_db +def test_create_account_email_already_exists(django_db_use_migrations): + """ + This is tricky. Django's user model doesn't have a constraint on + unique email addresses, but we *add* that constraint during the + migration process: + see common/djangoapps/student/migrations/0004_add_email_index.py + + The behavior we *want* is for this account creation request + to fail, due to this uniqueness constraint, but the request will + succeed if the migrations have not run. + + django_db_use_migration is a pytest fixture that tells us if + migrations have been run. Since pytest fixtures don't play nice + with TestCase objects this is a function and doesn't get to use + assertRaises. + """ + if django_db_use_migrations: + email = 'a@b.com' + pw = 'xyz' + username = 'testuser' + User.objects.create_user(username, email, pw) + + # Hack to use the _create_account shortcut + case = ContentStoreTestCase() + resp = case._create_account("abcdef", email, "password") # pylint: disable=protected-access + + assert resp.status_code == 400, 'Migrations are run, but creating an account with duplicate email succeeded!' + + class AuthTestCase(ContentStoreTestCase): """Check that various permissions-related things work""" @@ -113,7 +145,7 @@ class AuthTestCase(ContentStoreTestCase): reverse('signup'), ) for page in pages: - print "Checking '{0}'".format(page) + print("Checking '{0}'".format(page)) self.check_page_get(page, 200) def test_create_account_errors(self): @@ -139,20 +171,6 @@ class AuthTestCase(ContentStoreTestCase): # we can have two users with the same password, so this should succeed self.assertEqual(resp.status_code, 200) - @unittest.skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required") - def test_create_account_email_already_exists(self): - User.objects.create_user(self.username, self.email, self.pw) - resp = self._create_account("abcdef", self.email, "password") - # This is tricky. Django's user model doesn't have a constraint on - # unique email addresses, but we *add* that constraint during the - # migration process: - # see common/djangoapps/student/migrations/0004_add_email_index.py - # - # The behavior we *want* is for this account creation request - # to fail, due to this uniqueness constraint, but the request will - # succeed if the migrations have not run. - self.assertEqual(resp.status_code, 400) - def test_login(self): self.create_account(self.username, self.email, self.pw) @@ -256,17 +274,17 @@ class AuthTestCase(ContentStoreTestCase): self.client = AjaxEnabledTestClient() # Not logged in. Should redirect to login. - print 'Not logged in' + print('Not logged in') for page in auth_pages: - print "Checking '{0}'".format(page) + print("Checking '{0}'".format(page)) self.check_page_get(page, expected=302) # Logged in should work. self.login(self.email, self.pw) - print 'Logged in' + print('Logged in') for page in simple_auth_pages: - print "Checking '{0}'".format(page) + print("Checking '{0}'".format(page)) self.check_page_get(page, expected=200) def test_index_auth(self): diff --git a/cms/envs/test.py b/cms/envs/test.py index 1bcd33372b..86e4312906 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -77,9 +77,6 @@ COMMON_TEST_DATA_ROOT = COMMON_ROOT / "test" / "data" FEATURES['ENABLE_EXPORT_GIT'] = True GIT_REPO_EXPORT_DIR = TEST_ROOT / "export_course_repos" -# Makes the tests run much faster... -SOUTH_TESTS_MIGRATE = False # To disable migrations and use syncdb instead - # TODO (cpennington): We need to figure out how envs/test.py can inject things into common.py so that we don't have to repeat this sort of thing STATICFILES_DIRS = [ COMMON_ROOT / "static", diff --git a/lms/envs/common.py b/lms/envs/common.py index ae9079993d..c965b30356 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -235,8 +235,8 @@ FEATURES = { # Turn on/off Microsites feature 'USE_MICROSITES': False, - # Turn on third-party auth. Disabled for now because full implementations are not yet available. Remember to syncdb - # if you enable this; we don't create tables by default. + # Turn on third-party auth. Disabled for now because full implementations are not yet available. Remember to run + # migrations if you enable this; we don't create tables by default. 'ENABLE_THIRD_PARTY_AUTH': False, # Toggle to enable alternate urls for marketing links diff --git a/lms/envs/test.py b/lms/envs/test.py index af913e7533..d683b05c47 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -89,9 +89,6 @@ WIKI_ENABLED = True # Enable a parental consent age limit for testing PARENTAL_CONSENT_AGE_LIMIT = 13 -# Makes the tests run much faster... -SOUTH_TESTS_MIGRATE = False # To disable migrations and use syncdb instead - _SYSTEM = 'lms' _REPORT_DIR = REPO_ROOT / 'reports' / _SYSTEM diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index 20872871e6..33ce52a7c9 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -47,7 +47,7 @@ def listen_for_grade_calculation(sender, user, course_grade, course_key, deadlin """ # This needs to be imported here to avoid a circular dependency - # that can cause syncdb to fail. + # that can cause migrations to fail. from openedx.core.djangoapps.credit import api course_id = CourseKey.from_string(unicode(course_key)) is_credit = api.is_credit_course(course_id) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index e8260a6803..8b642be3ce 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -5,47 +5,54 @@ Most of the functionality is covered in test_views.py. """ import re -import ddt -from dateutil.parser import parse as parse_datetime -from mock import Mock, patch -from django.test import TestCase -from nose.plugins.attrib import attr -from nose.tools import raises -import unittest -from student.tests.factories import UserFactory +import pytest +from dateutil.parser import parse as parse_datetime from django.conf import settings from django.contrib.auth.models import User from django.core import mail +from django.test import TestCase from django.test.client import RequestFactory -from openedx.core.djangoapps.user_api.accounts import ( - USERNAME_MAX_LENGTH, - PRIVATE_VISIBILITY -) +from mock import Mock, patch +from six import iteritems + +import ddt +from nose.plugins.attrib import attr +from nose.tools import raises +from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNAME_MAX_LENGTH from openedx.core.djangoapps.user_api.accounts.api import ( - get_account_settings, - update_account_settings, - create_account, activate_account, - request_password_change -) -from openedx.core.djangoapps.user_api.errors import ( - UserNotFound, UserNotAuthorized, - AccountUpdateError, AccountValidationError, AccountUserAlreadyExists, - AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, - AccountRequestError + create_account, + get_account_settings, + request_password_change, + update_account_settings ) from openedx.core.djangoapps.user_api.accounts.tests.testutils import ( - INVALID_EMAILS, INVALID_PASSWORDS, INVALID_USERNAMES, VALID_USERNAMES_UNICODE + INVALID_EMAILS, + INVALID_PASSWORDS, + INVALID_USERNAMES, + VALID_USERNAMES_UNICODE +) +from openedx.core.djangoapps.user_api.errors import ( + AccountEmailInvalid, + AccountPasswordInvalid, + AccountRequestError, + AccountUpdateError, + AccountUserAlreadyExists, + AccountUsernameInvalid, + AccountValidationError, + UserNotAuthorized, + UserNotFound ) from openedx.core.djangolib.testing.utils import skip_unless_lms from student.models import PendingEmailChange +from student.tests.factories import UserFactory from student.tests.tests import UserSettingsEventTestMixin def mock_render_to_string(template_name, context): """Return a string that encodes template_name and context""" - return str((template_name, sorted(context.iteritems()))) + return str((template_name, sorted(iteritems(context)))) @attr(shard=2) @@ -311,6 +318,32 @@ class AccountSettingsOnCreationTest(TestCase): }) +@attr(shard=2) +@pytest.mark.django_db +def test_create_account_duplicate_email(django_db_use_migrations): + """ + Test case for duplicate email constraint + Email uniqueness constraints were introduced in a database migration, + which we disable in the unit tests to improve the speed of the test suite + + This test only runs if migrations have been run. + + django_db_use_migrations is a pytest_django fixture which tells us whether + migrations are being used. + """ + password = 'legit' + email = 'zappadappadoo@example.com' + + if django_db_use_migrations: + create_account('zappadappadoo', password, email) + + with pytest.raises( + AccountUserAlreadyExists, + message='Migrations are being used, but creating an account with duplicate email succeeded!' + ): + create_account('different_user', password, email) + + @attr(shard=2) @ddt.ddt class AccountCreationActivationAndPasswordChangeTest(TestCase): @@ -346,14 +379,6 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): with self.assertRaises(AccountUserAlreadyExists): create_account(self.USERNAME, self.PASSWORD, 'different+email@example.com') - # Email uniqueness constraints were introduced in a database migration, - # which we disable in the unit tests to improve the speed of the test suite. - @unittest.skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required") - def test_create_account_duplicate_email(self): - create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - with self.assertRaises(AccountUserAlreadyExists): - create_account('different_user', self.PASSWORD, self.EMAIL) - def test_username_too_long(self): long_username = 'e' * (USERNAME_MAX_LENGTH + 1) with self.assertRaises(AccountUsernameInvalid): diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py index e9e4c2e414..0914cd9d47 100644 --- a/pavelib/utils/test/suites/acceptance_suite.py +++ b/pavelib/utils/test/suites/acceptance_suite.py @@ -57,7 +57,7 @@ def setup_acceptance_db(): sh("./manage.py lms --settings {} migrate --traceback --noinput --fake-initial --database {}".format(settings, db_alias)) sh("./manage.py cms --settings {} migrate --traceback --noinput --fake-initial --database {}".format(settings, db_alias)) else: - # If no cached database exists, syncdb before migrating, then create the cache + # If no cached database exists, migrate then create the cache for db_alias in sorted(DBS.keys()): sh("./manage.py lms --settings {} migrate --traceback --noinput --database {}".format(settings, db_alias)) sh("./manage.py cms --settings {} migrate --traceback --noinput --database {}".format(settings, db_alias))