Merge pull request #10700 from edx/benp/cohort-mp-fixes

Make cohort management tests multiprocess capable.

common/test/acceptance/tests/discussion/test_cohort_management.py

@@ -16,6 +16,8 @@ from ...pages.lms.auto_auth import AutoAuthPage
 from ...pages.lms.instructor_dashboard import InstructorDashboardPage, DataDownloadPage
 from ...pages.studio.settings_group_configurations import GroupConfigurationsPage
 
+import os
+import unicodecsv
 import uuid
 
 
@@ -39,23 +41,24 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         self.manual_cohort_id = self.add_manual_cohort(self.course_fixture, self.manual_cohort_name)
 
         # create a non-instructor who will be registered for the course and in the manual cohort.
-        self.student_name = "student_user"
+        self.student_name, self.student_email = self._generate_unique_user_data()
         self.student_id = AutoAuthPage(
-            self.browser, username=self.student_name, email="student_user@example.com",
+            self.browser, username=self.student_name, email=self.student_email,
             course_id=self.course_id, staff=False
         ).visit().get_user_id()
         self.add_user_to_cohort(self.course_fixture, self.student_name, self.manual_cohort_id)
 
         # create a second student user
+        self.other_student_name, self.other_student_email = self._generate_unique_user_data()
         self.other_student_id = AutoAuthPage(
-            self.browser, username="other_student_user", email="other_student_user@example.com",
+            self.browser, username=self.other_student_name, email=self.other_student_email,
             course_id=self.course_id, staff=False
         ).visit().get_user_id()
 
         # login as an instructor
-        self.instructor_name = "instructor_user"
+        self.instructor_name, self.instructor_email = self._generate_unique_user_data()
         self.instructor_id = AutoAuthPage(
-            self.browser, username=self.instructor_name, email="instructor_user@example.com",
+            self.browser, username=self.instructor_name, email=self.instructor_email,
             course_id=self.course_id, staff=True
         ).visit().get_user_id()
 
@@ -310,6 +313,29 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
                     self.cohort_management_page.get_cohort_associated_assignment_type()
                 )
 
+    def _create_csv_file(self, filename, csv_text_as_lists):
+        """
+        Create a csv file with the provided list of lists.
+
+        :param filename: this is the name that will be used for the csv file. Its location will
+         be under the test upload data directory
+        :param csv_text_as_lists: provide the contents of the csv file int he form of a list of lists
+        """
+        filename = self.instructor_dashboard_page.get_asset_path(filename)
+        with open(filename, 'w+') as csv_file:
+            writer = unicodecsv.writer(csv_file)
+            for line in csv_text_as_lists:
+                writer.writerow(line)
+        self.addCleanup(os.remove, filename)
+
+    def _generate_unique_user_data(self):
+        """
+        Produce unique username and e-mail.
+        """
+        unique_username = 'user' + str(uuid.uuid4().hex)[:12]
+        unique_email = unique_username + "@example.com"
+        return unique_username, unique_email
+
     def test_add_new_cohort(self):
         """
         Scenario: A new manual cohort can be created, and a student assigned to it.
@@ -471,9 +497,15 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         Then I can download a file with results
         And appropriate events have been emitted
         """
-        # cohort_users_both_columns.csv adds instructor_user to ManualCohort1 via username and
-        # student_user to AutoCohort1 via email
-        self._verify_csv_upload_acceptable_file("cohort_users_both_columns.csv")
+        csv_contents = [
+            ['username', 'email', 'ignored_column', 'cohort'],
+            [self.instructor_name, '', 'June', 'ManualCohort1'],
+            ['', self.student_email, 'Spring', 'AutoCohort1'],
+            [self.other_student_name, '', 'Fall', 'ManualCohort1'],
+        ]
+        filename = "cohort_csv_both_columns_1.csv"
+        self._create_csv_file(filename, csv_contents)
+        self._verify_csv_upload_acceptable_file(filename)
 
     def test_cohort_by_csv_only_email(self):
         """
@@ -485,8 +517,15 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         Then I can download a file with results
         And appropriate events have been emitted
         """
-        # cohort_users_only_email.csv adds instructor_user to ManualCohort1 and student_user to AutoCohort1 via email
-        self._verify_csv_upload_acceptable_file("cohort_users_only_email.csv")
+        csv_contents = [
+            ['email', 'cohort'],
+            [self.instructor_email, 'ManualCohort1'],
+            [self.student_email, 'AutoCohort1'],
+            [self.other_student_email, 'ManualCohort1'],
+        ]
+        filename = "cohort_csv_emails_only.csv"
+        self._create_csv_file(filename, csv_contents)
+        self._verify_csv_upload_acceptable_file(filename)
 
     def test_cohort_by_csv_only_username(self):
         """
@@ -498,13 +537,46 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         Then I can download a file with results
         And appropriate events have been emitted
         """
-        # cohort_users_only_username.csv adds instructor_user to ManualCohort1 and
-        # student_user to AutoCohort1 via username
-        self._verify_csv_upload_acceptable_file("cohort_users_only_username.csv")
+        csv_contents = [
+            ['username', 'cohort'],
+            [self.instructor_name, 'ManualCohort1'],
+            [self.student_name, 'AutoCohort1'],
+            [self.other_student_name, 'ManualCohort1'],
+        ]
+        filename = "cohort_users_only_username1.csv"
+        self._create_csv_file(filename, csv_contents)
+        self._verify_csv_upload_acceptable_file(filename)
 
-    def _verify_csv_upload_acceptable_file(self, filename):
+    def test_cohort_by_csv_unicode(self):
+        """
+        Scenario: the instructor can upload a file with user and cohort assignments, using both emails and usernames.
+
+        Given I have a course with two cohorts defined
+        And I add another cohort with a unicode name
+        When I go to the cohort management section of the instructor dashboard
+        I can upload a CSV file with assignments of users to the unicode cohort via both usernames and emails
+        Then I can download a file with results
+
+        TODO: refactor events verification to handle this scenario. Events verification assumes movements
+        between other cohorts (manual and auto).
+        """
+        unicode_hello_in_korean = u'안녕하세요'
+        self._verify_cohort_settings(cohort_name=unicode_hello_in_korean, assignment_type=None)
+        csv_contents = [
+            ['username', 'email', 'cohort'],
+            [self.instructor_name, '', unicode_hello_in_korean],
+            ['', self.student_email, unicode_hello_in_korean],
+            [self.other_student_name, '', unicode_hello_in_korean]
+        ]
+        filename = "cohort_unicode_name.csv"
+        self._create_csv_file(filename, csv_contents)
+        self._verify_csv_upload_acceptable_file(filename, skip_events=True)
+
+    def _verify_csv_upload_acceptable_file(self, filename, skip_events=None):
         """
         Helper method to verify cohort assignments after a successful CSV upload.
+
+        When skip_events is specified, no assertions are made on events.
         """
         start_time = datetime.now(UTC)
         self.cohort_management_page.upload_cohort_file(filename)
@@ -512,45 +584,46 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
             "Your file '{}' has been uploaded. Allow a few minutes for processing.".format(filename)
         )
 
-        # student_user is moved from manual cohort to auto cohort
-        self.assertEqual(
-            self.event_collection.find({
-                "name": "edx.cohort.user_added",
-                "time": {"$gt": start_time},
-                "event.user_id": {"$in": [int(self.student_id)]},
-                "event.cohort_name": self.auto_cohort_name,
-            }).count(),
-            1
-        )
-        self.assertEqual(
-            self.event_collection.find({
-                "name": "edx.cohort.user_removed",
-                "time": {"$gt": start_time},
-                "event.user_id": int(self.student_id),
-                "event.cohort_name": self.manual_cohort_name,
-            }).count(),
-            1
-        )
-        # instructor_user (previously unassigned) is added to manual cohort
-        self.assertEqual(
-            self.event_collection.find({
-                "name": "edx.cohort.user_added",
-                "time": {"$gt": start_time},
-                "event.user_id": {"$in": [int(self.instructor_id)]},
-                "event.cohort_name": self.manual_cohort_name,
-            }).count(),
-            1
-        )
-        # other_student_user (previously unassigned) is added to manual cohort
-        self.assertEqual(
-            self.event_collection.find({
-                "name": "edx.cohort.user_added",
-                "time": {"$gt": start_time},
-                "event.user_id": {"$in": [int(self.other_student_id)]},
-                "event.cohort_name": self.manual_cohort_name,
-            }).count(),
-            1
-        )
+        if not skip_events:
+            # student_user is moved from manual cohort to auto cohort
+            self.assertEqual(
+                self.event_collection.find({
+                    "name": "edx.cohort.user_added",
+                    "time": {"$gt": start_time},
+                    "event.user_id": {"$in": [int(self.student_id)]},
+                    "event.cohort_name": self.auto_cohort_name,
+                }).count(),
+                1
+            )
+            self.assertEqual(
+                self.event_collection.find({
+                    "name": "edx.cohort.user_removed",
+                    "time": {"$gt": start_time},
+                    "event.user_id": int(self.student_id),
+                    "event.cohort_name": self.manual_cohort_name,
+                }).count(),
+                1
+            )
+            # instructor_user (previously unassigned) is added to manual cohort
+            self.assertEqual(
+                self.event_collection.find({
+                    "name": "edx.cohort.user_added",
+                    "time": {"$gt": start_time},
+                    "event.user_id": {"$in": [int(self.instructor_id)]},
+                    "event.cohort_name": self.manual_cohort_name,
+                }).count(),
+                1
+            )
+            # other_student_user (previously unassigned) is added to manual cohort
+            self.assertEqual(
+                self.event_collection.find({
+                    "name": "edx.cohort.user_added",
+                    "time": {"$gt": start_time},
+                    "event.user_id": {"$in": [int(self.other_student_id)]},
+                    "event.cohort_name": self.manual_cohort_name,
+                }).count(),
+                1
+            )
 
         # Verify the results can be downloaded.
         data_download = self.instructor_dashboard_page.select_data_download()

common/test/acceptance/tests/lms/test_learner_profile.py

@@ -604,7 +604,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
 
         self.assert_default_image_has_public_access(profile_page)
 
-        profile_page.upload_file(filename='cohort_users_only_username.csv')
+        profile_page.upload_file(filename='generic_csv.csv')
         self.assertEqual(
             profile_page.profile_image_message,
             "The file must be one of the following types: .gif, .png, .jpeg, .jpg."

common/test/data/uploads/cohort_users_both_columns.csv

@@ -1,4 +0,0 @@
-username,email,ignored_column,cohort
-instructor_user,,June,ManualCohort1
-,student_user@example.com,Spring,AutoCohort1
-other_student_user,,Fall,ManualCohort1

common/test/data/uploads/cohort_users_only_email.csv

@@ -1,5 +0,0 @@
-email,cohort
-instructor_user@example.com,ManualCohort1
-student_user@example.com,AutoCohort1
-other_student_user@example.com,ManualCohort1
-

common/test/data/uploads/cohort_users_only_username.csv

@@ -1,4 +0,0 @@
-username,cohort
-instructor_user,ManualCohort1
-student_user,AutoCohort1
-other_student_user,ManualCohort1

common/test/data/uploads/generic_csv.csv

@@ -0,0 +1,7 @@
+column_1,column_2
+foo,bar
+foo,baz
+hello,there
+file_size,must_be_100_bytes
+the story,was
+very very very very very,interesting