Fix docstring and test for get_cohort

Returns None if course isn't cohorted, even if cohorts exist.  Shouldn't happen on prod, but could if cohorting is turned off mid-course.

common/djangoapps/course_groups/cohorts.py

@@ -70,15 +70,19 @@ def get_cohort(user, course_id):
 
 def _get_cohort(user, course_id):
     """
-    Given a django User and a course_id, return the user's cohort.  In classes with
-    auto-cohorting, put the user in a cohort if they aren't in one already.
+    Given a django User and a course_id, return the user's cohort in that
+    cohort.
+
+    TODO: In classes with auto-cohorting, put the user in a cohort if they
+    aren't in one already.
 
     Arguments:
         user: a Django User object.
         course_id: string in the format 'org/course/run'
 
     Returns:
-        A CourseUserGroup object if the User has a cohort, or None.
+        A CourseUserGroup object if the course is cohorted and the User has a
+        cohort, else None.
 
     Raises:
        ValueError if the course_id doesn't exist.

common/djangoapps/course_groups/tests/tests.py

@@ -1,32 +1,14 @@
-from nose import SkipTest
 import django.test
 from django.contrib.auth.models import User
 from django.conf import settings
 
 from override_settings import override_settings
 
-
 from course_groups.models import CourseUserGroup
 from course_groups.cohorts import get_cohort, get_course_cohorts
 
-from xmodule.tests.test_import import BaseCourseTestCase
 from xmodule.modulestore.django import modulestore
 
-def xml_store_config(data_dir):
-    return {
-    'default': {
-        'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
-        'OPTIONS': {
-            'data_dir': data_dir,
-            'default_class': 'xmodule.hidden_module.HiddenDescriptor',
-        }
-    }
-}
-
-TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
-TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR)
-
-@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
 class TestCohorts(django.test.TestCase):
 
     def test_get_cohort(self):
@@ -36,22 +18,31 @@ class TestCohorts(django.test.TestCase):
 
         # Proper fix: give all tests a standard modulestore that uses the test
         # dir.
-        raise SkipTest()
         course = modulestore().get_course("edX/toy/2012_Fall")
-        cohort = CourseUserGroup.objects.create(name="TestCohort",
-                                                course_id=course.id,
-                               group_type=CourseUserGroup.COHORT)
+        self.assertEqual(course.id, "edX/toy/2012_Fall")
 
         user = User.objects.create(username="test", email="a@b.com")
         other_user = User.objects.create(username="test2", email="a2@b.com")
 
+        self.assertIsNone(get_cohort(user, course.id), "No cohort created yet")
+
+        cohort = CourseUserGroup.objects.create(name="TestCohort",
+                                                course_id=course.id,
+                               group_type=CourseUserGroup.COHORT)
+
         cohort.users.add(user)
 
-        got = get_cohort(user, course.id)
-        self.assertEquals(got.id, cohort.id, "Should find the right cohort")
+        self.assertIsNone(get_cohort(user, course.id),
+                          "Course isn't cohorted, so shouldn't have a cohort")
 
-        got = get_cohort(other_user, course.id)
-        self.assertEquals(got, None, "other_user shouldn't have a cohort")
+        # Make the course cohorted...
+        course.metadata["cohort_config"] = {"cohorted": True}
+        
+        self.assertEquals(get_cohort(user, course.id).id, cohort.id,
+                          "Should find the right cohort")
+
+        self.assertEquals(get_cohort(other_user, course.id), None,
+                          "other_user shouldn't have a cohort")
 
 
     def test_get_course_cohorts(self):