diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index ce2960307b..2d83091f38 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/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): diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index 70372ce96b..b5e70a3da3 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/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) diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index b75190e290..a4ce56c3b9 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/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'}} + ] + ) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index cdaf037338..027e132cc9 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/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': []}