Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add tests and clean up A/B testing

Browse Source 
Also fixes STUD-1351
This commit is contained in:
Diana Huang
2014-03-01 14:15:56 -05:00
committed by Calen Pennington
parent 2d5c37b2a0
commit bce7d9e43d
29 changed files with 715 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
common/lib/xmodule/xmodule/course_module.py
View File

@@ -158,6 +158,7 @@ class TextbookList(List):
class UserPartitionList(List):
"""Special List class for listing UserPartitions"""
def from_json(self, values):
return [UserPartition.from_json(v) for v in values]
@@ -175,8 +176,9 @@ class CourseFields(object):
# advanced_settings.
user_partitions = UserPartitionList(
help="List of user partitions of this course into groups, used e.g. for experiments",
default=[], scope=Scope.content)
default=[],
scope=Scope.content
)
wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content)
enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings)

19
common/lib/xmodule/xmodule/js/fixtures/split_test_staff.html Normal file
View File

@@ -0,0 +1,19 @@
<div class="split-test-view" id="split-test">
<select class="split-test-select">
<option value="0">Group 0</option>
<option value="1">Group 1</option>
<option value="2">Group 2</option>
</select>
<div class="split-test-child" data-group-id="0">
&lt;div class='condition-text'&gt;condition 0&lt;/div&gt;
</div>
<div class="split-test-child" data-group-id="1">
&lt;div class='condition-text'&gt;condition 1&lt;/div&gt;
</div>
<div class="split-test-child" data-group-id="2">
&lt;div class='condition-text'&gt;condition 2&lt;/div&gt;
</div>
<div class='split-test-child-container'></div>
</div>

1
common/lib/xmodule/xmodule/js/js_test.yml
View File

@@ -57,6 +57,7 @@ lib_paths:
- common_static/js/vendor/analytics.js
- common_static/js/test/add_ajax_prefix.js
- common_static/js/src/utility.js
- public/js/split_test_staff.js
# Paths to spec (test) JavaScript files
spec_paths:

1
common/lib/xmodule/xmodule/js/public Symbolic link
View File

@@ -0,0 +1 @@
../public/

37
common/lib/xmodule/xmodule/js/spec/split_test/staff_view_spec.js Normal file
View File

@@ -0,0 +1,37 @@
describe('Tests for split_test staff view switching', function() {
var ab_module;
var elem;
beforeEach(function() {
loadFixtures('split_test_staff.html');
elem = $('#split-test');
window.XBlock = jasmine.createSpyObj('XBlock', ['initializeBlocks']);
ab_module = ABTestSelector(null, elem);
});
afterEach(function() {
delete window.XBlock;
});
it("test that we have only one visible condition", function() {
var containers = elem.find('.split-test-child-container').length;
var conditions_shown = elem.find('.split-test-child-container .condition-text').length;
expect(containers).toEqual(1);
expect(conditions_shown).toEqual(1);
expect(XBlock.initializeBlocks).toHaveBeenCalled();
});
it("test that the right child is visible when selected", function() {
var groups = ['0', '1', '2'];
for(var i = 0; i < groups.length; i++) {
var to_select = groups[i];
elem.find('.split-test-select').val(to_select).change();
var child_text = elem.find('.split-test-child-container .condition-text').text();
expect(child_text).toContain(to_select);
expect(XBlock.initializeBlocks).toHaveBeenCalled();
}
});
});

12
common/lib/xmodule/xmodule/js/src/sequence/display.coffee
View File

@@ -23,8 +23,8 @@ class @Sequence
updatePageTitle: ->
# update the page title to include the current section
position_link = @link_for(@position)
if position_link and position_link.attr('title')
document.title = position_link.attr('title') + @base_page_title
if position_link and position_link.data('page-title')
document.title = position_link.data('page-title') + @base_page_title
hookUpProgressEvent: ->
$('.problems-wrapper').bind 'progressChanged', @updateProgress
@@ -98,10 +98,10 @@ class @Sequence
# Added for aborting video bufferization, see ../video/10_main.js
@el.trigger "sequence:change"
@mark_active new_position
current_tab = @contents.eq(new_position - 1)
@content_container.html(current_tab.text()).attr("aria-labelledby", current_tab.attr("aria-labelledby"))
XBlock.initializeBlocks(@content_container)
window.update_schematics() # For embedded circuit simulator exercises in 6.002x
@@ -115,8 +115,8 @@ class @Sequence
sequence_links.click @goto
# Focus on the first available xblock.
@content_container.find('.vert .xblock :first').focus()
@$("a.active").blur()
@$("a.active").blur()
goto: (event) =>
event.preventDefault()
if $(event.target).hasClass 'seqnav' # Links from courseware <a class='seqnav' href='n'>...</a>

26
common/lib/xmodule/xmodule/modulestore/xml_importer.py
View File

@@ -6,7 +6,7 @@ import json
from .xml import XMLModuleStore, ImportSystem, ParentTracker
from xmodule.modulestore import Location
from xblock.fields import Scope, Reference, ReferenceList
from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict
from xmodule.contentstore.content import StaticContent
from .inheritance import own_metadata
from xmodule.errortracker import make_error_tracker
@@ -547,15 +547,25 @@ def remap_namespace(module, target_location_namespace):
).url()
return new_ref
for field in all_fields:
if isinstance(module.fields.get(field), Reference):
new_ref = convert_ref(getattr(module, field))
setattr(module, field, new_ref)
for field_name in all_fields:
field_object = module.fields.get(field_name)
if isinstance(field_object, Reference):
new_ref = convert_ref(getattr(module, field_name))
setattr(module, field_name, new_ref)
module.save()
elif isinstance(module.fields.get(field), ReferenceList):
references = getattr(module, field)
elif isinstance(field_object, ReferenceList):
references = getattr(module, field_name)
new_references = [convert_ref(reference) for reference in references]
setattr(module, field, new_references)
setattr(module, field_name, new_references)
module.save()
elif isinstance(field_object, ReferenceValueDict):
reference_dict = getattr(module, field_name)
new_reference_dict = {
key: convert_ref(reference)
for key, reference
in reference_dict.items()
}
setattr(module, field_name, new_reference_dict)
module.save()
return module

4
common/lib/xmodule/xmodule/partitions/partitions_service.py
View File

@@ -88,7 +88,7 @@ class PartitionService(object):
and persist the info.
"""
key = self._key_for_partition(user_partition)
scope = self._user_tags_service.COURSE
scope = self._user_tags_service.COURSE_SCOPE
group_id = self._user_tags_service.get_tag(scope, key)
if group_id is not None:
@@ -133,6 +133,6 @@ class PartitionService(object):
'partition_name': user_partition.name
}
# TODO: Use the XBlock publish api instead
self._track_function('edx.split_test.assigned_user_to_partition', event_info)
self._track_function('xmodule.partitions.assigned_user_to_partition', event_info)
return group.id

165
common/lib/xmodule/xmodule/partitions/test_partitions.py
View File

@@ -3,6 +3,7 @@ Test the partitions and partitions service
"""
from collections import defaultdict
from unittest import TestCase
from mock import Mock, MagicMock
@@ -49,6 +50,131 @@ class TestGroup(TestCase):
self.assertEqual(group.id, test_id)
self.assertEqual(group.name, name)
def test_from_json_broken(self):
test_id = 5
name = "Grendel"
# Bad version
jsonified = {
"id": test_id,
"name": name,
"version": 9001
}
with self.assertRaisesRegexp(TypeError, "has unexpected version"):
group = Group.from_json(jsonified)
# Missing key "id"
jsonified = {
"name": name,
"version": Group.VERSION
}
with self.assertRaisesRegexp(TypeError, "missing value key 'id'"):
group = Group.from_json(jsonified)
# Has extra key - should not be a problem
jsonified = {
"id": test_id,
"name": name,
"version": Group.VERSION,
"programmer": "Cale"
}
group = Group.from_json(jsonified)
self.assertNotIn("programmer", group.to_json())
class TestUserPartition(TestCase):
"""Test constructing UserPartitions"""
def test_construct(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
user_partition = UserPartition(0, 'Test Partition', 'for testing purposes', groups)
self.assertEqual(user_partition.id, 0)
self.assertEqual(user_partition.name, "Test Partition")
self.assertEqual(user_partition.description, "for testing purposes")
self.assertEqual(user_partition.groups, groups)
def test_string_id(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
user_partition = UserPartition("70", 'Test Partition', 'for testing purposes', groups)
self.assertEqual(user_partition.id, 70)
def test_to_json(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
upid = 0
upname = "Test Partition"
updesc = "for testing purposes"
user_partition = UserPartition(upid, upname, updesc, groups)
jsonified = user_partition.to_json()
act_jsonified = {
"id": upid,
"name": upname,
"description": updesc,
"groups": [group.to_json() for group in groups],
"version": user_partition.VERSION
}
self.assertEqual(jsonified, act_jsonified)
def test_from_json(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
upid = 1
upname = "Test Partition"
updesc = "For Testing Purposes"
jsonified = {
"id": upid,
"name": upname,
"description": updesc,
"groups": [group.to_json() for group in groups],
"version": UserPartition.VERSION
}
user_partition = UserPartition.from_json(jsonified)
self.assertEqual(user_partition.id, upid)
self.assertEqual(user_partition.name, upname)
self.assertEqual(user_partition.description, updesc)
for act_group in user_partition.groups:
self.assertIn(act_group.id, [0, 1])
exp_group = groups[act_group.id]
self.assertEqual(exp_group.id, act_group.id)
self.assertEqual(exp_group.name, act_group.name)
def test_from_json_broken(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
upid = 1
upname = "Test Partition"
updesc = "For Testing Purposes"
# Missing field
jsonified = {
"name": upname,
"description": updesc,
"groups": [group.to_json() for group in groups],
"version": UserPartition.VERSION
}
with self.assertRaisesRegexp(TypeError, "missing value key 'id'"):
user_partition = UserPartition.from_json(jsonified)
# Wrong version (it's over 9000!)
jsonified = {
'id': upid,
"name": upname,
"description": updesc,
"groups": [group.to_json() for group in groups],
"version": 9001
}
with self.assertRaisesRegexp(TypeError, "has unexpected version"):
user_partition = UserPartition.from_json(jsonified)
# Has extra key - should not be a problem
jsonified = {
'id': upid,
"name": upname,
"description": updesc,
"groups": [group.to_json() for group in groups],
"version": UserPartition.VERSION,
"programmer": "Cale"
}
user_partition = UserPartition.from_json(jsonified)
self.assertNotIn("programmer", user_partition.to_json())
class StaticPartitionService(PartitionService):
"""
@@ -63,32 +189,37 @@ class StaticPartitionService(PartitionService):
return self._partitions
class MemoryUserTagsService(object):
"""
An implementation of a user_tags XBlock service that
uses an in-memory dictionary for storage
"""
COURSE_SCOPE = 'course'
def __init__(self):
self._tags = defaultdict(dict)
def get_tag(self, scope, key):
"""Sets the value of ``key`` to ``value``"""
print 'GETTING', scope, key, self._tags
return self._tags[scope].get(key)
def set_tag(self, scope, key, value):
"""Gets the value of ``key``"""
self._tags[scope][key] = value
print 'SET', scope, key, value, self._tags
class TestPartitionsService(TestCase):
"""
Test getting a user's group out of a partition
"""
def setUp(self):
groups = [Group(0, 'Group 1'), Group(1, 'Group 2')]
self.partition_id = 0
# construct the user_service
self.user_tags = dict()
self.user_tags_service = MagicMock()
def mock_set_tag(_scope, key, value):
"""Sets the value of ``key`` to ``value``"""
self.user_tags[key] = value
def mock_get_tag(_scope, key):
"""Gets the value of ``key``"""
if key in self.user_tags:
return self.user_tags[key]
return None
self.user_tags_service.set_tag = mock_set_tag
self.user_tags_service.get_tag = mock_get_tag
self.user_tags_service = MemoryUserTagsService()
user_partition = UserPartition(self.partition_id, 'Test Partition', 'for testing purposes', groups)
self.partitions_service = StaticPartitionService(

26
common/lib/xmodule/xmodule/public/js/split_test_staff.js
View File

@@ -4,30 +4,26 @@
* @constructor
*/
function ABTestSelector(elem) {
me = this;
me.elem = $(elem);
function ABTestSelector(runtime, elem) {
var _this = this;
_this.elem = $(elem);
_this.children = _this.elem.find('.split-test-child');
_this.content_container = _this.elem.find('.split-test-child-container');
select_child = function(group_id) {
function select_child(group_id) {
// iterate over all the children and hide all the ones that haven't been selected
// and show the one that was selected
me.elem.find('.split-test-child').each(function() {
_this.children.each(function() {
// force this id to remain a string, even if it looks like something else
child_group_id = $(this).data('group-id').toString();
var child_group_id = $(this).data('group-id').toString();
if(child_group_id === group_id) {
$(this).show();
_this.content_container.html($(this).text());
XBlock.initializeBlocks(_this.content_container);
}
else {
$(this).hide();
}
});
}
// hide all the children
me.elem.find('.split-test-child').hide();
select = me.elem.find('.split-test-select');
select = _this.elem.find('.split-test-select');
cur_group_id = select.val();
select_child(cur_group_id);

2
common/lib/xmodule/xmodule/public/js/split_test_student.js
View File

@@ -1,4 +1,4 @@
/* Javascript for the Acid XBlock. */
/* Javascript for the Split Test XBlock. */
function SplitTestStudentView(runtime, element) {
$.post(runtime.handlerUrl(element, 'log_child_render'));
return {};

9
common/lib/xmodule/xmodule/seq_module.py
View File

@@ -88,13 +88,12 @@ class SequenceModule(SequenceFields, XModule):
rendered_child = child.render('student_view', context)
fragment.add_frag_resources(rendered_child)
titles = child.get_content_titles()
print titles
childinfo = {
'content': rendered_child.content,
'title': "\n".join(
grand_child.display_name
for grand_child in child.get_children()
if grand_child.display_name is not None
),
'title': "\n".join(titles),
'page_title': titles[0] if titles else '',
'progress_status': Progress.to_js_status_str(progress),
'progress_detail': Progress.to_js_detail_str(progress),
'type': child.get_icon_class(),

52
common/lib/xmodule/xmodule/split_test_module.py
View File

@@ -12,15 +12,18 @@ from xmodule.x_module import XModule, module_attr
from lxml import etree
from xblock.core import XBlock
from xblock.fields import Scope, Integer, Dict
from xblock.fields import Scope, Integer, ReferenceValueDict
from xblock.fragment import Fragment
log = logging.getLogger('edx.' + __name__)
class SplitTestFields(object):
user_partition_id = Integer(help="Which user partition is used for this test",
scope=Scope.content)
"""Fields needed for split test module"""
user_partition_id = Integer(
help="Which user partition is used for this test",
scope=Scope.content
)
# group_id is an int
# child is a serialized UsageId (aka Location). This child
@@ -31,9 +34,10 @@ class SplitTestFields(object):
# TODO: is there a way to add some validation around this, to
# be run on course load or in studio or ....
group_id_to_child = Dict(help="Which child module students in a particular "
"group_id should see",
scope=Scope.content)
group_id_to_child = ReferenceValueDict(
help="Which child module students in a particular group_id should see",
scope=Scope.content
)
@XBlock.needs('user_tags')
@@ -79,6 +83,25 @@ class SplitTestModule(SplitTestFields, XModule):
return None
def get_content_titles(self):
"""
Returns list of content titles for split_test's child.
This overwrites the get_content_titles method included in x_module by default.
WHY THIS OVERWRITE IS NECESSARY: If we fetch *all* of split_test's children,
we'll end up getting all of the possible conditions users could ever see.
Ex: If split_test shows a video to group A and HTML to group B, the
regular get_content_titles in x_module will get the title of BOTH the video
AND the HTML.
We only want the content titles that should actually be displayed to the user.
split_test's .child property contains *only* the child that should actually
be shown to the user, so we call get_content_titles() on only that child.
"""
return self.child.get_content_titles()
def get_child_descriptors(self):
"""
For grading--return just the chosen child.
@@ -127,13 +150,9 @@ class SplitTestModule(SplitTestFields, XModule):
fragment.add_content(self.system.render_template('split_test_staff_view.html', {
'items': contents,
}))
frag_js = """
$(document).ready(function() {{
ABTestSelector($('.split-test-view'));
}});
"""
fragment.add_javascript(frag_js)
fragment.add_css('.split-test-child { display: none; }')
fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/split_test_staff.js'))
fragment.initialize_js('ABTestSelector')
return fragment
def student_view(self, context):
@@ -159,9 +178,12 @@ class SplitTestModule(SplitTestFields, XModule):
return fragment
@XBlock.handler
def log_child_render(self, request, suffix=''):
def log_child_render(self, request, suffix=''): # pylint: disable=unused-argument
"""
Record in the tracking logs which child was rendered
"""
# TODO: use publish instead, when publish is wired to the tracking logs
self.system.track_function('split-test-child-render', {'child-id': self.child.scope_ids.usage_id})
self.system.track_function('xblock.split_test.child_render', {'child-id': self.child.scope_ids.usage_id})
return Response()
def get_icon_class(self):
@@ -184,6 +206,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor):
child_descriptor = module_attr('child_descriptor')
log_child_render = module_attr('log_child_render')
get_content_titles = module_attr('get_content_titles')
def definition_to_xml(self, resource_fs):
@@ -200,3 +223,4 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor):
makes it use module.get_child_descriptors().
"""
return True

78
common/lib/xmodule/xmodule/tests/test_split_module.py
View File

@@ -8,9 +8,7 @@ from xmodule.tests.xml import factories as xml
from xmodule.tests.xml import XModuleXmlImportTest
from xmodule.tests import get_test_system
from xmodule.partitions.partitions import Group, UserPartition
from xmodule.partitions.partitions_service import PartitionService
from xmodule.partitions.test_partitions import StaticPartitionService
from xmodule.partitions.test_partitions import StaticPartitionService, MemoryUserTagsService
class SplitTestModuleFactory(xml.XmlImportFactory):
@@ -38,15 +36,24 @@ class SplitTestModuleTest(XModuleXmlImportTest):
'group_id_to_child': '{"0": "i4x://edX/xml_test_course/html/split_test_cond0", "1": "i4x://edX/xml_test_course/html/split_test_cond1"}'
}
)
xml.HtmlFactory(parent=split_test, url_name='split_test_cond0')
xml.HtmlFactory(parent=split_test, url_name='split_test_cond1')
xml.HtmlFactory(parent=split_test, url_name='split_test_cond0', text='HTML FOR GROUP 0')
xml.HtmlFactory(parent=split_test, url_name='split_test_cond1', text='HTML FOR GROUP 1')
self.course = self.process_xml(course)
course_seq = self.course.get_children()[0]
self.module_system = get_test_system()
self.tags_service = Mock(name='user_tags')
self.module_system._services['user_tags'] = self.tags_service
def get_module(descriptor):
module_system = get_test_system()
module_system.get_module = get_module
descriptor.bind_for_student(module_system, descriptor._field_data)
return descriptor
self.module_system.get_module = get_module
self.module_system.descriptor_system = self.course.runtime
self.tags_service = MemoryUserTagsService()
self.module_system._services['user_tags'] = self.tags_service # pylint: disable=protected-access
self.partitions_service = StaticPartitionService(
[
@@ -57,18 +64,59 @@ class SplitTestModuleTest(XModuleXmlImportTest):
course_id=self.course.id,
track_function=Mock(name='track_function'),
)
self.module_system._services['partitions'] = self.partitions_service
self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access
self.split_test_module = course_seq.get_children()[0]
self.split_test_module.bind_for_student(self.module_system, self.split_test_module._field_data)
self.split_test_descriptor = course_seq.get_children()[0]
self.split_test_descriptor.bind_for_student(
self.module_system,
self.split_test_descriptor._field_data
)
@ddt.data(('0', 'split_test_cond0'), ('1', 'split_test_cond1'))
@ddt.unpack
def test_child(self, user_tag, child_url_name):
self.tags_service.get_tag.return_value = user_tag
self.tags_service.set_tag(
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
user_tag
)
self.assertEquals(self.split_test_descriptor.child_descriptor.url_name, child_url_name)
self.assertEquals(self.split_test_module.child_descriptor.url_name, child_url_name)
@ddt.data(('0',), ('1',))
@ddt.unpack
def test_child_old_tag_value(self, user_tag):
# If user_tag has a stale value, we should still get back a valid child url
self.tags_service.set_tag(
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
'2'
)
self.assertIn(self.split_test_module.child_descriptor.url_name, ['split_test_cond0', 'split_test_cond1'])
@ddt.data(('0', 'HTML FOR GROUP 0'), ('1', 'HTML FOR GROUP 1'))
@ddt.unpack
def test_get_html(self, user_tag, child_content):
self.tags_service.set_tag(
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
user_tag
)
self.assertIn(
child_content,
self.module_system.render(self.split_test_module, 'student_view').content
)
@ddt.data(('0',), ('1',))
@ddt.unpack
def test_child_missing_tag_value(self, user_tag):
# If user_tag has a missing value, we should still get back a valid child url
self.assertIn(self.split_test_module.child_descriptor.url_name, ['split_test_cond0', 'split_test_cond1'])
@ddt.data(('100',), ('200',), ('300',), ('400',), ('500',), ('600',), ('700',), ('800',), ('900',), ('1000',))
@ddt.unpack
def test_child_persist_new_tag_value_when_tag_missing(self, user_tag):
# If a user_tag has a missing value, a group should be saved/persisted for that user.
# So, we check that we get the same url_name when we call on the url_name twice.
# We run the test ten times so that, if our storage is failing, we'll be most likely to notice it.
self.assertEquals(self.split_test_module.child_descriptor.url_name, self.split_test_module.child_descriptor.url_name)

6
common/lib/xmodule/xmodule/tests/xml/factories.py
View File

@@ -146,6 +146,10 @@ class SequenceFactory(XmlImportFactory):
"""Factory for <sequential> nodes"""
tag = 'sequential'
class VerticalFactory(XmlImportFactory):
"""Factory for <vertical> nodes"""
tag = 'vertical'
class ProblemFactory(XmlImportFactory):
"""Factory for <problem> nodes"""
@@ -154,5 +158,5 @@ class ProblemFactory(XmlImportFactory):
class HtmlFactory(XmlImportFactory):
"""Factory for <problem> nodes"""
"""Factory for <html> nodes"""
tag = 'html'

25
common/lib/xmodule/xmodule/x_module.py
View File

@@ -217,6 +217,31 @@ class XModuleMixin(XBlockMixin):
self.save()
return self._field_data._kvs # pylint: disable=protected-access
def get_content_titles(self):
"""
Returns list of content titles for all of self's children.
SEQUENCE
|
VERTICAL
/ \
SPLIT_TEST DISCUSSION
/ \
VIDEO A VIDEO B
Essentially, this function returns a list of display_names (e.g. content titles)
for all of the leaf nodes. In the diagram above, calling get_content_titles on
SEQUENCE would return the display_names of `VIDEO A`, `VIDEO B`, and `DISCUSSION`.
This is most obviously useful for sequence_modules, which need this list to display
tooltips to users, though in theory this should work for any tree that needs
the display_names of all its leaf nodes.
"""
if self.has_children:
return sum((child.get_content_titles() for child in self.get_children()), [])
else:
return [self.display_name_with_default]
def get_children(self):
"""Returns a list of XBlock instances for the children of
this module"""