Merge pull request #16410 from edx/bmedx/django111_syncdb_cleanup

syncdb cleanup

cms/djangoapps/contentstore/tests/tests.py

 """
 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):

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",

lms/envs/common.py

@@ -236,8 +236,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

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

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)

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)
@@ -312,6 +319,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):

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))