The progress page did a number of things that make performance terrible for courses with large numbers of problems, particularly if those problems are customresponse CapaModule problems that need to be executed via codejail. The grading code takes pains to not instantiate student state and execute the problem code. If a student has answered the question, the max score is stored in StudentModule. However, if the student hasn't attempted the question yet, we have to run the problem code just to call .max_score() on it. This is necessary in grade() if the student has answered other problems in the assignment (so we can know what to divide by). This is always necessary to know in progress_summary() because we list out every problem there. Code execution can be especially slow if the problems need to invoke codejail. To address this, we create a MaxScoresCache that will cache the max raw score possible for every problem. We select the cache keys so that it will automatically become invalidated when a new version of the course is published. The fundamental assumption here is that a problem cannot have two different max score values for two unscored students. A problem *can* score two students differently such that they have different max scores. So Carlos can have 2/3 on a problem, while Lyla gets 3/4. But if neither Carlos nor Lyla has ever interacted with the problem (i.e. they're just seeing it on their progress page), they must both see 0/4 -- it cannot be the case that Carlos sees 0/3 and Lyla sees 0/4. We used to load all student state into two separate FieldDataCache instances, after which we do a bunch of individual queries for scored items. Part of this split-up was done because of locking problems, but I think we might have gotten overzealous with our manual transaction hammer. In this commit, we consolidate all state access in grade() and progress() to use one shared FieldDataCache. We also use a filter so that we only pull back StudentModule state for things that might possibly affect the grade -- items that either have scores or have children. Because some older XModules do work in their __init__() methods (like Video), instantiating them takes time, particularly on large courses. This commit also changes the code that fetches the grading_context to filter out children that can't possibly affect the grade. Finally, we introduce a ScoresClient that also tries to fetch score information all at once, instead of in separate queries. Technically, we are fetching this information redundantly, but that's because the state and score interfaces are being teased apart as we move forward. Still, this only amounts to one extra SQL query, and has very little impact on performance overall. Much thanks to @adampalay -- his hackathon work in #7168 formed the basis of this. https://openedx.atlassian.net/browse/CSM-17
197 lines
7.8 KiB
Python
197 lines
7.8 KiB
Python
"""
|
|
Test grade calculation.
|
|
"""
|
|
from django.http import Http404
|
|
from django.test.client import RequestFactory
|
|
|
|
from mock import patch
|
|
from nose.plugins.attrib import attr
|
|
from opaque_keys.edx.locations import SlashSeparatedCourseKey
|
|
|
|
from courseware.grades import field_data_cache_for_grading, grade, iterate_grades_for, MaxScoresCache
|
|
from student.tests.factories import UserFactory
|
|
from student.models import CourseEnrollment
|
|
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
|
|
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
|
|
|
|
|
def _grade_with_errors(student, request, course, keep_raw_scores=False):
|
|
"""This fake grade method will throw exceptions for student3 and
|
|
student4, but allow any other students to go through normal grading.
|
|
|
|
It's meant to simulate when something goes really wrong while trying to
|
|
grade a particular student, so we can test that we won't kill the entire
|
|
course grading run.
|
|
"""
|
|
if student.username in ['student3', 'student4']:
|
|
raise Exception("I don't like {}".format(student.username))
|
|
|
|
return grade(student, request, course, keep_raw_scores=keep_raw_scores)
|
|
|
|
|
|
@attr('shard_1')
|
|
class TestGradeIteration(ModuleStoreTestCase):
|
|
"""
|
|
Test iteration through student gradesets.
|
|
"""
|
|
COURSE_NUM = "1000"
|
|
COURSE_NAME = "grading_test_course"
|
|
|
|
def setUp(self):
|
|
"""
|
|
Create a course and a handful of users to assign grades
|
|
"""
|
|
super(TestGradeIteration, self).setUp()
|
|
|
|
self.course = CourseFactory.create(
|
|
display_name=self.COURSE_NAME,
|
|
number=self.COURSE_NUM
|
|
)
|
|
self.students = [
|
|
UserFactory.create(username='student1'),
|
|
UserFactory.create(username='student2'),
|
|
UserFactory.create(username='student3'),
|
|
UserFactory.create(username='student4'),
|
|
UserFactory.create(username='student5'),
|
|
]
|
|
|
|
def test_empty_student_list(self):
|
|
"""If we don't pass in any students, it should return a zero-length
|
|
iterator, but it shouldn't error."""
|
|
gradeset_results = list(iterate_grades_for(self.course.id, []))
|
|
self.assertEqual(gradeset_results, [])
|
|
|
|
def test_nonexistent_course(self):
|
|
"""If the course we want to get grades for does not exist, a `Http404`
|
|
should be raised. This is a horrible crossing of abstraction boundaries
|
|
and should be fixed, but for now we're just testing the behavior. :-("""
|
|
with self.assertRaises(Http404):
|
|
gradeset_results = iterate_grades_for(SlashSeparatedCourseKey("I", "dont", "exist"), [])
|
|
gradeset_results.next()
|
|
|
|
def test_all_empty_grades(self):
|
|
"""No students have grade entries"""
|
|
all_gradesets, all_errors = self._gradesets_and_errors_for(self.course.id, self.students)
|
|
self.assertEqual(len(all_errors), 0)
|
|
for gradeset in all_gradesets.values():
|
|
self.assertIsNone(gradeset['grade'])
|
|
self.assertEqual(gradeset['percent'], 0.0)
|
|
|
|
@patch('courseware.grades.grade', _grade_with_errors)
|
|
def test_grading_exception(self):
|
|
"""Test that we correctly capture exception messages that bubble up from
|
|
grading. Note that we only see errors at this level if the grading
|
|
process for this student fails entirely due to an unexpected event --
|
|
having errors in the problem sets will not trigger this.
|
|
|
|
We patch the grade() method with our own, which will generate the errors
|
|
for student3 and student4.
|
|
"""
|
|
all_gradesets, all_errors = self._gradesets_and_errors_for(self.course.id, self.students)
|
|
student1, student2, student3, student4, student5 = self.students
|
|
self.assertEqual(
|
|
all_errors,
|
|
{
|
|
student3: "I don't like student3",
|
|
student4: "I don't like student4"
|
|
}
|
|
)
|
|
|
|
# But we should still have five gradesets
|
|
self.assertEqual(len(all_gradesets), 5)
|
|
|
|
# Even though two will simply be empty
|
|
self.assertFalse(all_gradesets[student3])
|
|
self.assertFalse(all_gradesets[student4])
|
|
|
|
# The rest will have grade information in them
|
|
self.assertTrue(all_gradesets[student1])
|
|
self.assertTrue(all_gradesets[student2])
|
|
self.assertTrue(all_gradesets[student5])
|
|
|
|
################################# Helpers #################################
|
|
def _gradesets_and_errors_for(self, course_id, students):
|
|
"""Simple helper method to iterate through student grades and give us
|
|
two dictionaries -- one that has all students and their respective
|
|
gradesets, and one that has only students that could not be graded and
|
|
their respective error messages."""
|
|
students_to_gradesets = {}
|
|
students_to_errors = {}
|
|
|
|
for student, gradeset, err_msg in iterate_grades_for(course_id, students):
|
|
students_to_gradesets[student] = gradeset
|
|
if err_msg:
|
|
students_to_errors[student] = err_msg
|
|
|
|
return students_to_gradesets, students_to_errors
|
|
|
|
|
|
class TestMaxScoresCache(ModuleStoreTestCase):
|
|
"""
|
|
Tests for the MaxScoresCache
|
|
"""
|
|
def setUp(self):
|
|
super(TestMaxScoresCache, self).setUp()
|
|
self.student = UserFactory.create()
|
|
self.course = CourseFactory.create()
|
|
self.problems = []
|
|
for _ in xrange(3):
|
|
self.problems.append(
|
|
ItemFactory.create(category='problem', parent=self.course)
|
|
)
|
|
|
|
CourseEnrollment.enroll(self.student, self.course.id)
|
|
self.request = RequestFactory().get('/')
|
|
self.locations = [problem.location for problem in self.problems]
|
|
|
|
def test_max_scores_cache(self):
|
|
"""
|
|
Tests the behavior fo the MaxScoresCache
|
|
"""
|
|
max_scores_cache = MaxScoresCache("test_max_scores_cache")
|
|
self.assertEqual(max_scores_cache.num_cached_from_remote(), 0)
|
|
self.assertEqual(max_scores_cache.num_cached_updates(), 0)
|
|
|
|
# add score to cache
|
|
max_scores_cache.set(self.locations[0], 1)
|
|
self.assertEqual(max_scores_cache.num_cached_updates(), 1)
|
|
|
|
# push to remote cache
|
|
max_scores_cache.push_to_remote()
|
|
|
|
# create a new cache with the same params, fetch from remote cache
|
|
max_scores_cache = MaxScoresCache("test_max_scores_cache")
|
|
max_scores_cache.fetch_from_remote(self.locations)
|
|
|
|
# see cache is populated
|
|
self.assertEqual(max_scores_cache.num_cached_from_remote(), 1)
|
|
|
|
|
|
class TestFieldDataCacheScorableLocations(ModuleStoreTestCase):
|
|
"""
|
|
Make sure we can filter the locations we pull back student state for via
|
|
the FieldDataCache.
|
|
"""
|
|
def setUp(self):
|
|
super(TestFieldDataCacheScorableLocations, self).setUp()
|
|
self.student = UserFactory.create()
|
|
self.course = CourseFactory.create()
|
|
chapter = ItemFactory.create(category='chapter', parent=self.course)
|
|
sequential = ItemFactory.create(category='sequential', parent=chapter)
|
|
vertical = ItemFactory.create(category='vertical', parent=sequential)
|
|
ItemFactory.create(category='video', parent=vertical)
|
|
ItemFactory.create(category='html', parent=vertical)
|
|
ItemFactory.create(category='discussion', parent=vertical)
|
|
ItemFactory.create(category='problem', parent=vertical)
|
|
|
|
CourseEnrollment.enroll(self.student, self.course.id)
|
|
|
|
def test_field_data_cache_scorable_locations(self):
|
|
"""Only scorable locations should be in FieldDataCache.scorable_locations."""
|
|
fd_cache = field_data_cache_for_grading(self.course, self.student)
|
|
block_types = set(loc.block_type for loc in fd_cache.scorable_locations)
|
|
self.assertNotIn('video', block_types)
|
|
self.assertNotIn('html', block_types)
|
|
self.assertNotIn('discussion', block_types)
|
|
self.assertIn('problem', block_types)
|