Merge pull request #5728 from edx/dan-f/grade-export-split-test

Verify grade export works for problems in split tests

lms/djangoapps/instructor_task/tests/test_api.py

@@ -23,6 +23,7 @@ from instructor_task.models import InstructorTask, PROGRESS
 from instructor_task.tests.test_base import (InstructorTaskTestCase,
                                              InstructorTaskCourseTestCase,
                                              InstructorTaskModuleTestCase,
+                                             TestReportMixin,
                                              TEST_COURSE_KEY)
 
 
@@ -158,7 +159,7 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase):
         self._test_submit_task(submit_delete_problem_state_for_all_students)
 
 
-class InstructorTaskCourseSubmitTest(InstructorTaskCourseTestCase):
+class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCase):
     """Tests API methods that involve the submission of course-based background tasks."""
 
     def setUp(self):

lms/djangoapps/instructor_task/tests/test_base.py

@@ -2,12 +2,16 @@
 Base test classes for LMS instructor-initiated background tasks
 
 """
+import os
+import shutil
+
 import json
 from uuid import uuid4
 from mock import Mock
 
 from celery.states import SUCCESS, FAILURE
 
+from django.conf import settings
 from django.test.testcases import TestCase
 from django.contrib.auth.models import User
 from django.test.utils import override_settings
@@ -104,14 +108,25 @@ class InstructorTaskCourseTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase)
     course = None
     current_user = None
 
-    def initialize_course(self):
-        """Create a course in the store, with a chapter and section."""
+    def initialize_course(self, course_factory_kwargs=None):
+        """
+        Create a course in the store, with a chapter and section.
+
+        Arguments:
+            course_factory_kwargs (dict): kwargs dict to pass to
+            CourseFactory.create()
+        """
         self.module_store = modulestore()
 
         # Create the course
-        self.course = CourseFactory.create(org=TEST_COURSE_ORG,
-                                           number=TEST_COURSE_NUMBER,
-                                           display_name=TEST_COURSE_NAME)
+        course_args = {
+            "org": TEST_COURSE_ORG,
+            "number": TEST_COURSE_NUMBER,
+            "display_name": TEST_COURSE_NAME
+        }
+        if course_factory_kwargs is not None:
+            course_args.update(course_factory_kwargs)
+        self.course = CourseFactory.create(**course_args)
 
         # Add a chapter to the course
         chapter = ItemFactory.create(parent_location=self.course.location,
@@ -134,20 +149,21 @@ class InstructorTaskCourseTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase)
             self.login(InstructorTaskCourseTestCase.get_user_email(username), "test")
             self.current_user = username
 
-    def _create_user(self, username, is_staff=False):
+    def _create_user(self, username, email=None, is_staff=False):
         """Creates a user and enrolls them in the test course."""
-        email = InstructorTaskCourseTestCase.get_user_email(username)
+        if email is None:
+            email = InstructorTaskCourseTestCase.get_user_email(username)
         thisuser = UserFactory.create(username=username, email=email, is_staff=is_staff)
         CourseEnrollmentFactory.create(user=thisuser, course_id=self.course.id)
         return thisuser
 
-    def create_instructor(self, username):
+    def create_instructor(self, username, email=None):
         """Creates an instructor for the test course."""
-        return self._create_user(username, is_staff=True)
+        return self._create_user(username, email, is_staff=True)
 
-    def create_student(self, username):
+    def create_student(self, username, email=None):
         """Creates a student for the test course."""
-        return self._create_user(username, is_staff=False)
+        return self._create_user(username, email, is_staff=False)
 
     @staticmethod
     def get_task_status(task_id):
@@ -184,16 +200,18 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         else:
             return TEST_COURSE_KEY.make_usage_key('problem', problem_url_name)
 
-    def define_option_problem(self, problem_url_name):
+    def define_option_problem(self, problem_url_name, parent=None):
         """Create the problem definition so the answer is Option 1"""
+        if parent is None:
+            parent = self.problem_section
         factory = OptionResponseXMLFactory()
         factory_args = {'question_text': 'The correct answer is {0}'.format(OPTION_1),
                         'options': [OPTION_1, OPTION_2],
                         'correct_option': OPTION_1,
                         'num_responses': 2}
         problem_xml = factory.build_xml(**factory_args)
-        ItemFactory.create(parent_location=self.problem_section.location,
-                           parent=self.problem_section,
+        ItemFactory.create(parent_location=parent.location,
+                           parent=parent,
                            category="problem",
                            display_name=str(problem_url_name),
                            data=problem_xml)
@@ -220,3 +238,13 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
                                          module_type=descriptor.location.category,
                                          module_state_key=descriptor.location,
                                          )
+
+
+class TestReportMixin(object):
+    """
+    Cleans up after tests that place files in the reports directory.
+    """
+    def tearDown(self):
+        reports_download_path = settings.GRADES_DOWNLOAD['ROOT_PATH']
+        if os.path.exists(reports_download_path):
+            shutil.rmtree(reports_download_path)

lms/djangoapps/instructor_task/tests/test_integration.py

@@ -5,6 +5,7 @@ Runs tasks on answers to course problems to validate that code
 paths actually work.
 
 """
+import csv
 import logging
 import json
 from mock import patch
@@ -16,8 +17,10 @@ from django.core.urlresolvers import reverse
 
 from capa.tests.response_xml_factory import (CodeResponseXMLFactory,
                                              CustomResponseXMLFactory)
+from user_api.tests.factories import UserCourseTagFactory
 from xmodule.modulestore.tests.factories import ItemFactory
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.partitions.partitions import Group, UserPartition
 
 from courseware.model_data import StudentModule
 
@@ -25,9 +28,10 @@ from instructor_task.api import (submit_rescore_problem_for_all_students,
                                  submit_rescore_problem_for_student,
                                  submit_reset_problem_attempts_for_all_students,
                                  submit_delete_problem_state_for_all_students)
-from instructor_task.models import InstructorTask
-from instructor_task.tests.test_base import (InstructorTaskModuleTestCase, TEST_COURSE_ORG, TEST_COURSE_NUMBER,
-                                             OPTION_1, OPTION_2)
+from instructor_task.models import InstructorTask, ReportStore
+from instructor_task.tasks_helper import upload_grades_csv
+from instructor_task.tests.test_base import (InstructorTaskModuleTestCase, TestReportMixin, TEST_COURSE_ORG,
+                                             TEST_COURSE_NUMBER, OPTION_1, OPTION_2)
 from capa.responsetypes import StudentInputError
 from lms.lib.xblock.runtime import quote_slashes
 
@@ -488,3 +492,201 @@ class TestDeleteProblemTask(TestIntegrationTask):
         instructor_task = self.delete_problem_state('instructor', location)
         instructor_task = InstructorTask.objects.get(id=instructor_task.id)
         self.assertEqual(instructor_task.task_state, SUCCESS)
+
+
+class TestGradeReportConditionalContent(TestReportMixin, TestIntegrationTask):
+    """
+    Check that grade export works when graded content exists within
+    split modules.
+    """
+    def setUp(self):
+        """
+        Set up a course with graded problems within a split test.
+
+        Course hierarchy is as follows (modeled after how split tests
+        are created in studio):
+        -> course
+            -> chapter
+                -> sequential (graded)
+                    -> vertical
+                        -> split_test
+                            -> vertical (Group A)
+                                -> problem
+                            -> vertical (Group B)
+                                -> problem
+        """
+        super(TestGradeReportConditionalContent, self).setUp()
+
+        # Create user partitions
+        self.user_partition_group_a = 0
+        self.user_partition_group_b = 1
+        self.partition = UserPartition(
+            0,
+            'first_partition',
+            'First Partition',
+            [
+                Group(self.user_partition_group_a, 'Group A'),
+                Group(self.user_partition_group_b, 'Group B')
+            ]
+        )
+
+        # Create course with group configurations and grading policy
+        self.initialize_course(
+            course_factory_kwargs={
+                'user_partitions': [self.partition],
+                'grading_policy': {
+                    "GRADER": [{
+                        "type": "Homework",
+                        "min_count": 1,
+                        "drop_count": 0,
+                        "short_label": "HW",
+                        "weight": 1.0
+                    }]
+                }
+            }
+        )
+
+        # Create users and partition them
+        self.student_a = self.create_student('student_a')
+        self.student_b = self.create_student('student_b')
+        UserCourseTagFactory(
+            user=self.student_a,
+            course_id=self.course.id,
+            key='xblock.partition_service.partition_{0}'.format(self.partition.id),  # pylint: disable=no-member
+            value=str(self.user_partition_group_a)
+        )
+        UserCourseTagFactory(
+            user=self.student_b,
+            course_id=self.course.id,
+            key='xblock.partition_service.partition_{0}'.format(self.partition.id),  # pylint: disable=no-member
+            value=str(self.user_partition_group_b)
+        )
+
+        # Create a vertical to contain our split test
+        problem_vertical = ItemFactory.create(
+            parent_location=self.problem_section.location,
+            category='vertical',
+            display_name='Problem Unit'
+        )
+
+        # Create the split test and child vertical containers
+        vertical_a_url = self.course.id.make_usage_key('vertical', 'split_test_vertical_a')
+        vertical_b_url = self.course.id.make_usage_key('vertical', 'split_test_vertical_b')
+        self.split_test = ItemFactory.create(
+            parent_location=problem_vertical.location,
+            category='split_test',
+            display_name='Split Test',
+            user_partition_id=self.partition.id,  # pylint: disable=no-member
+            group_id_to_child={str(index): url for index, url in enumerate([vertical_a_url, vertical_b_url])}
+        )
+        self.vertical_a = ItemFactory.create(
+            parent_location=self.split_test.location,
+            category='vertical',
+            display_name='Group A problem container',
+            location=vertical_a_url
+        )
+        self.vertical_b = ItemFactory.create(
+            parent_location=self.split_test.location,
+            category='vertical',
+            display_name='Group B problem container',
+            location=vertical_b_url
+        )
+
+    def verify_csv_task_success(self, task_result):
+        """
+        Verify that all students were successfully graded by
+        `upload_grades_csv`.
+
+        Arguments:
+            task_result (dict): Return value of `upload_grades_csv`.
+        """
+        self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, task_result)
+
+    def verify_rows_in_csv(self, expected_rows):
+        """
+        Verify that the grades CSV contains the expected content.
+
+        Arguments:
+            expected_rows (iterable): An iterable of dictionaries, where
+                each dict represents a row of data in the grades
+                report CSV.  Each dict maps keys from the CSV header
+                to values in that row's corresponding cell.
+        """
+        report_store = ReportStore.from_config()
+        report_csv_filename = report_store.links_for(self.course.id)[0][0]
+        with open(report_store.path_to(self.course.id, report_csv_filename)) as csv_file:
+            # Expand the dict reader generator so we don't lose it's content
+            csv_rows = [row for row in csv.DictReader(csv_file)]
+            self.assertEqual(csv_rows, expected_rows)
+
+    def verify_grades_in_csv(self, students_grades):
+        """
+        Verify that the grades CSV contains the expected grades data.
+
+        Arguments:
+            students_grades (iterable): An iterable of dictionaries,
+                where each dict maps a student to another dict
+                representing their grades we expect to see in the CSV.
+                For example: [student_a: {'grade': 1.0, 'HW': 1.0}]
+        """
+        def merge_dicts(dict_1, dict_2):
+            """Return the union of dict_1 and dict_2"""
+            return dict(dict_1.items() + dict_2.items())
+
+        self.verify_rows_in_csv(
+            [
+                merge_dicts(
+                    {'id': str(student.id), 'username': student.username, 'email': student.email},
+                    grades
+                )
+                for student_grades in students_grades for student, grades in student_grades.iteritems()
+            ]
+        )
+
+    def test_both_groups_problems(self):
+        """
+        Verify that grade export works when each user partition
+        receives (different) problems.  Each user's grade on their
+        particular problem should show up in the grade report.
+        """
+        problem_a_url = 'problem_a_url'
+        problem_b_url = 'problem_b_url'
+        self.define_option_problem(problem_a_url, parent=self.vertical_a)
+        self.define_option_problem(problem_b_url, parent=self.vertical_b)
+        # student A will get 100%, student B will get 50% because
+        # OPTION_1 is the correct option, and OPTION_2 is the
+        # incorrect option
+        self.submit_student_answer(self.student_a.username, problem_a_url, [OPTION_1, OPTION_1])
+        self.submit_student_answer(self.student_b.username, problem_b_url, [OPTION_1, OPTION_2])
+
+        with patch('instructor_task.tasks_helper._get_current_task'):
+            result = upload_grades_csv(None, None, self.course.id, None, 'graded')
+            self.verify_csv_task_success(result)
+            self.verify_grades_in_csv(
+                [
+                    {self.student_a: {'grade': '1.0', 'HW': '1.0'}},
+                    {self.student_b: {'grade': '0.5', 'HW': '0.5'}}
+                ]
+            )
+
+    def test_one_group_problem(self):
+        """
+        Verify that grade export works when only the Group A user
+        partition receives a problem.  We expect to see a column for
+        the homework where student_a's entry includes their grade, and
+        student b's entry shows a 0.
+        """
+        problem_a_url = 'problem_a_url'
+        self.define_option_problem(problem_a_url, parent=self.vertical_a)
+
+        self.submit_student_answer(self.student_a.username, problem_a_url, [OPTION_1, OPTION_1])
+
+        with patch('instructor_task.tasks_helper._get_current_task'):
+            result = upload_grades_csv(None, None, self.course.id, None, 'graded')
+            self.verify_csv_task_success(result)
+            self.verify_grades_in_csv(
+                [
+                    {self.student_a: {'grade': '1.0', 'HW': '1.0'}},
+                    {self.student_b: {'grade': '0.0', 'HW': '0.0'}}
+                ]
+            )

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -4,13 +4,9 @@ Unit tests for LMS instructor-initiated background tasks helper functions.
 Tests that CSV grade report generation works with unicode emails.
 
 """
-import os
-import shutil
-
 import ddt
 from mock import Mock, patch
 
-from django.conf import settings
 from django.test.testcases import TestCase
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -20,30 +16,17 @@ from student.tests.factories import CourseEnrollmentFactory, UserFactory
 
 from instructor_task.models import ReportStore
 from instructor_task.tasks_helper import upload_grades_csv, upload_students_csv
+from instructor_task.tests.test_base import InstructorTaskCourseTestCase, TestReportMixin
 
 
-class TestReport(ModuleStoreTestCase):
+@ddt.ddt
+class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
     """
-    Base class for testing CSV download tasks.
+    Tests that CSV grade report generation works.
     """
     def setUp(self):
         self.course = CourseFactory.create()
 
-    def tearDown(self):
-        if os.path.exists(settings.GRADES_DOWNLOAD['ROOT_PATH']):
-            shutil.rmtree(settings.GRADES_DOWNLOAD['ROOT_PATH'])
-
-    def create_student(self, username, email):
-        student = UserFactory.create(username=username, email=email)
-        CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
-        return student
-
-
-@ddt.ddt
-class TestInstructorGradeReport(TestReport):
-    """
-    Tests that CSV grade report generation works.
-    """
     @ddt.data([u'student@example.com', u'ni\xf1o@example.com'])
     def test_unicode_emails(self, emails):
         """
@@ -79,10 +62,13 @@ class TestInstructorGradeReport(TestReport):
 
 
 @ddt.ddt
-class TestStudentReport(TestReport):
+class TestStudentReport(TestReportMixin, InstructorTaskCourseTestCase):
     """
     Tests that CSV student profile report generation works.
     """
+    def setUp(self):
+        self.course = CourseFactory.create()
+
     def test_success(self):
         self.create_student('student', 'student@example.com')
         task_input = {'features': []}