diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 8ba35628c2..1b58c3e0a5 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -567,7 +567,11 @@ def _create_new_course(request, org, number, run, fields): fields.update(definition_data) store = modulestore() - with store.default_store(settings.FEATURES.get('DEFAULT_STORE_FOR_NEW_COURSE', 'mongo')): + store_for_new_course = ( + settings.FEATURES.get('DEFAULT_STORE_FOR_NEW_COURSE') or + store.default_modulestore.get_modulestore_type() + ) + with store.default_store(store_for_new_course): # Creating the course raises DuplicateCourseError if an existing course with this org/name is found new_course = store.create_course( org, @@ -584,7 +588,8 @@ def _create_new_course(request, org, number, run, fields): initialize_permissions(new_course.id, request.user) return JsonResponse({ - 'url': reverse_course_url('course_handler', new_course.id) + 'url': reverse_course_url('course_handler', new_course.id), + 'course_key': unicode(new_course.id), }) diff --git a/cms/envs/common.py b/cms/envs/common.py index 250a053da5..44c787b99c 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -106,7 +106,7 @@ FEATURES = { 'ADVANCED_SECURITY': False, # Modulestore to use for new courses - 'DEFAULT_STORE_FOR_NEW_COURSE': 'mongo', + 'DEFAULT_STORE_FOR_NEW_COURSE': None, } ENABLE_JASMINE = False diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index b28c4474f1..9ec6b9f572 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -11,6 +11,8 @@ from textwrap import dedent from collections import namedtuple from path import path from lazy import lazy +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import CourseLocator from . import STUDIO_BASE_URL @@ -204,6 +206,7 @@ class CourseFixture(StudioApiFixture): self.children = [] self._assets = [] self._advanced_settings = {} + self._course_key = None def __str__(self): """ @@ -263,19 +266,13 @@ class CourseFixture(StudioApiFixture): return self - @property - def _course_key(self): - """ - Return the locator string for the course. - """ - return "{org}/{number}/{run}".format(**self._course_dict) - @property def _course_location(self): """ Return the locator string for the course. """ - return "i4x://{org}/{number}/course/{run}".format(**self._course_dict) + course_key = CourseKey.from_string(self._course_key) + return unicode(course_key.make_usage_key('course', self._course_dict['run'])) @property def _assets_url(self): @@ -289,7 +286,8 @@ class CourseFixture(StudioApiFixture): """ Return the locator string for the course handouts """ - return "i4x://{org}/{number}/course_info/handouts".format(**self._course_dict) + course_key = CourseKey.from_string(self._course_key) + return unicode(course_key.make_usage_key('course_info', 'handouts')) def _create_course(self): """ @@ -315,7 +313,9 @@ class CourseFixture(StudioApiFixture): if err is not None: raise CourseFixtureError("Could not create course {0}. Error message: '{1}'".format(self, err)) - if not response.ok: + if response.ok: + self._course_key = response.json()['course_key'] + else: raise CourseFixtureError( "Could not create course {0}. Status was {1}".format( self._course_dict, response.status_code)) diff --git a/common/test/acceptance/tests/helpers.py b/common/test/acceptance/tests/helpers.py index c6f74700f1..2fe931be96 100644 --- a/common/test/acceptance/tests/helpers.py +++ b/common/test/acceptance/tests/helpers.py @@ -5,8 +5,10 @@ import json import unittest import functools import requests +import os from path import path from bok_choy.web_app_test import WebAppTest +from opaque_keys.edx.locator import CourseLocator def skip_if_browser(browser): @@ -170,8 +172,6 @@ class UniqueCourseTest(WebAppTest): Test that provides a unique course ID. """ - COURSE_ID_SEPARATOR = "/" - def __init__(self, *args, **kwargs): """ Create a unique course ID. @@ -190,11 +190,18 @@ class UniqueCourseTest(WebAppTest): @property def course_id(self): - return self.COURSE_ID_SEPARATOR.join([ + """ + Returns the serialized course_key for the test + """ + # TODO - is there a better way to make this agnostic to the underlying default module store? + default_store = os.environ.get('DEFAULT_STORE', 'draft') + course_key = CourseLocator( self.course_info['org'], self.course_info['number'], - self.course_info['run'] - ]) + self.course_info['run'], + deprecated=(default_store == 'draft') + ) + return unicode(course_key) class YouTubeConfigError(Exception): diff --git a/docs/en_us/internal/testing.md b/docs/en_us/internal/testing.md index 669eee24a0..3e2c56bbc1 100644 --- a/docs/en_us/internal/testing.md +++ b/docs/en_us/internal/testing.md @@ -274,6 +274,14 @@ To put a debugging breakpoint in a test use: from nose.tools import set_trace; set_trace() +By default, all bokchoy tests are run with the 'split' ModuleStore. +To override the modulestore that is used, use the default_store option. The currently supported stores are: + 'split' (xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore) and + 'draft' (xmodule.modulestore.mongo.DraftMongoModuleStore). +For example: + + paver test_bokchoy --default_store='draft' + ### Running Lettuce Acceptance Tests @@ -309,6 +317,14 @@ To start the debugger on failure, add the `--pdb` option to extra_args: To run tests faster by not collecting static files, you can use `paver test_acceptance -s lms --fasttest` and `paver test_acceptance -s cms --fasttest`. +By default, all acceptance tests are run with the 'draft' ModuleStore. +To override the modulestore that is used, use the default_store option. Currently, the possible stores for acceptance tests are: + 'split' (xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore) and + 'draft' (xmodule.modulestore.mongo.DraftMongoModuleStore). +For example: + paver test_acceptance --default_store='draft' +Note, however, all acceptance tests currently do not pass with 'split'. + Acceptance tests will run on a randomized port and can be run in the background of paver cms and lms or unit tests. To specify the port, change the LETTUCE_SERVER_PORT constant in cms/envs/acceptance.py and lms/envs/acceptance.py as well as the port listed in cms/djangoapps/contentstore/feature/upload.py diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index 5ea785290d..c510a9f54d 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -22,6 +22,7 @@ __test__ = False # do not collect ('test_spec=', 't', 'Specific test to run'), ('fasttest', 'a', 'Skip some setup'), ('extra_args=', 'e', 'adds as extra args to the test command'), + ('default_store=', 's', 'Default modulestore'), make_option("--verbose", action="store_const", const=2, dest="verbosity"), make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"), make_option("-v", "--verbosity", action="count", dest="verbosity"), @@ -45,13 +46,12 @@ def test_bokchoy(options): opts = { 'test_spec': getattr(options, 'test_spec', None), 'fasttest': getattr(options, 'fasttest', False), + 'default_store': getattr(options, 'default_store', None), 'verbosity': getattr(options, 'verbosity', 2), 'extra_args': getattr(options, 'extra_args', ''), 'test_dir': 'tests', } - - test_suite = BokChoyTestSuite('bok-choy', **opts) - test_suite.run() + run_bokchoy(**opts) @task @@ -60,6 +60,7 @@ def test_bokchoy(options): ('test_spec=', 't', 'Specific test to run'), ('fasttest', 'a', 'Skip some setup'), ('imports_dir=', 'd', 'Directory containing (un-archived) courses to be imported'), + ('default_store=', 's', 'Default modulestore'), make_option("--verbose", action="store_const", const=2, dest="verbosity"), make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"), make_option("-v", "--verbosity", action="count", dest="verbosity"), @@ -72,13 +73,33 @@ def perf_report_bokchoy(options): 'test_spec': getattr(options, 'test_spec', None), 'fasttest': getattr(options, 'fasttest', False), 'imports_dir': getattr(options, 'imports_dir', None), + 'default_store': getattr(options, 'default_store', None), 'verbosity': getattr(options, 'verbosity', 2), 'test_dir': 'performance', 'ptests': True, } + run_bokchoy(**opts) - test_suite = BokChoyTestSuite('bok-choy', **opts) - test_suite.run() + +def run_bokchoy(**opts): + """ + Runs BokChoyTestSuite with the given options. + If a default store is not specified, runs the test suite for 'split' as the default store. + """ + if opts['default_store'] not in ['draft', 'split']: + msg = colorize( + 'red', + 'No modulestore specified, running tests for split.' + ) + print(msg) + stores = ['split'] + else: + stores = [opts['default_store']] + + for store in stores: + opts['default_store'] = store + test_suite = BokChoyTestSuite('bok-choy', **opts) + test_suite.run() @task diff --git a/pavelib/utils/test/bokchoy_utils.py b/pavelib/utils/test/bokchoy_utils.py index c71c6164aa..686bc515fc 100644 --- a/pavelib/utils/test/bokchoy_utils.py +++ b/pavelib/utils/test/bokchoy_utils.py @@ -18,7 +18,7 @@ except ImportError: __test__ = False # do not collect -def start_servers(): +def start_servers(default_store): """ Start the servers we will run tests on, returns PIDs for servers. """ @@ -33,9 +33,11 @@ def start_servers(): for service, info in Env.BOK_CHOY_SERVERS.iteritems(): address = "0.0.0.0:{}".format(info['port']) cmd = ( + "DEFAULT_STORE={default_store} " "coverage run --rcfile={coveragerc} -m " "manage {service} --settings bok_choy runserver " "{address} --traceback --noreload".format( + default_store=default_store, coveragerc=Env.BOK_CHOY_COVERAGERC, service=service, address=address, diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index da98d021e6..3301d326d8 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/pavelib/utils/test/suites/bokchoy_suite.py @@ -28,6 +28,7 @@ class BokChoyTestSuite(TestSuite): self.cache = Env.BOK_CHOY_CACHE self.fasttest = kwargs.get('fasttest', False) self.test_spec = kwargs.get('test_spec', None) + self.default_store = kwargs.get('default_store') self.verbosity = kwargs.get('verbosity', 2) self.extra_args = kwargs.get('extra_args', '') self.ptests = kwargs.get('ptests', False) @@ -64,17 +65,26 @@ class BokChoyTestSuite(TestSuite): self.cache.flush_all() sh( - "./manage.py lms --settings bok_choy loaddata --traceback" - " common/test/db_fixtures/*.json" + "DEFAULT_STORE={default_store}" + " ./manage.py lms --settings bok_choy loaddata --traceback" + " common/test/db_fixtures/*.json".format( + default_store=self.default_store, + ) ) if self.imports_dir: - sh("./manage.py cms --settings=bok_choy import {}".format(self.imports_dir)) + sh( + "DEFAULT_STORE={default_store}" + " ./manage.py cms --settings=bok_choy import {import_dir}".format( + default_store=self.default_store, + import_dir=self.imports_dir + ) + ) # Ensure the test servers are available msg = colorize('green', "Starting test servers...") print(msg) - bokchoy_utils.start_servers() + bokchoy_utils.start_servers(self.default_store) msg = colorize('green', "Waiting for servers to start...") print(msg) @@ -101,6 +111,7 @@ class BokChoyTestSuite(TestSuite): # Construct the nosetests command, specifying where to save # screenshots and XUnit XML reports cmd = [ + "DEFAULT_STORE={}".format(self.default_store), "SCREENSHOT_DIR='{}'".format(self.log_dir), "HAR_DIR='{}'".format(self.har_dir), "SELENIUM_DRIVER_LOG_DIR='{}'".format(self.log_dir),