From b43c1ab27cb86d57243c5a4905736c1dcfb141cc Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Mon, 3 May 2021 09:26:22 -0400 Subject: [PATCH] test: Update allowlist tests, and remove verification of certificates app functionality (#27486) MICROBA-1013 --- lms/djangoapps/certificates/tests/test_api.py | 2 +- .../tests/test_generation_handler.py | 12 +- .../certificates/tests/test_signals.py | 112 +++-------- .../tests/test_tasks_helper.py | 187 ++++++------------ 4 files changed, 89 insertions(+), 224 deletions(-) diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index d953276801..5bebdd2b9b 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -1024,7 +1024,7 @@ class CertificateAllowlistTests(ModuleStoreTestCase): def test_can_be_added_to_allowlist_not_enrolled(self): """ - Test to verify that a learner will be rejected from the allowlist without an active enrollmeint in a + Test to verify that a learner will be rejected from the allowlist without an active enrollment in a course-run. """ new_course_run = CourseFactory() diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 6bf201ac71..5d101a6f02 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -151,11 +151,13 @@ class AllowlistTests(ModuleStoreTestCase): """ Test handling of an invalid user/course run combo """ - assert not _can_generate_allowlist_certificate(self.user, self.course_run_key) - assert not generate_allowlist_certificate_task(self.user, self.course_run_key) - assert not can_generate_certificate_task(self.user, self.course_run_key) - assert not generate_certificate_task(self.user, self.course_run_key) - assert _set_allowlist_cert_status(self.user, self.course_run_key) is None + u = UserFactory() + + assert not _can_generate_allowlist_certificate(u, self.course_run_key) + assert not generate_allowlist_certificate_task(u, self.course_run_key) + assert not can_generate_certificate_task(u, self.course_run_key) + assert not generate_certificate_task(u, self.course_run_key) + assert _set_allowlist_cert_status(u, self.course_run_key) is None def test_handle_valid(self): """ diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 3bf29aa049..c249742905 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -68,14 +68,8 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): def setUp(self): super().setUp() - self.course = CourseFactory.create(self_paced=True) self.user = UserFactory.create() - CourseEnrollmentFactory( - user=self.user, - course_id=self.course.id, - is_active=True, - mode="verified", - ) + # Instructor paced course self.ip_course = CourseFactory.create(self_paced=False) CourseEnrollmentFactory( user=self.user, @@ -84,86 +78,10 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): mode="verified", ) - def test_cert_generation_on_allowlist_append_self_paced_auto_cert_generation_disabled(self): - """ - Verify that signal is sent, received, and fires task - based on 'AUTO_CERTIFICATE_GENERATION' flag - """ - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): - CertificateWhitelistFactory( - user=self.user, - course_id=self.course.id - ) - mock_generate_certificate_apply_async.assert_not_called() - - def test_cert_generation_on_allowlist_append_self_paced_auto_cert_generation_enabled(self): - """ - Verify that signal is sent, received, and fires task - based on 'AUTO_CERTIFICATE_GENERATION' flag - """ - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): - CertificateWhitelistFactory( - user=self.user, - course_id=self.course.id - ) - mock_generate_certificate_apply_async.assert_called_with( - countdown=CERTIFICATE_DELAY_SECONDS, - kwargs={ - 'student': str(self.user.id), - 'course_key': str(self.course.id), - } - ) - - def test_cert_generation_on_allowlist_append_instructor_paced_cert_generation_disabled(self): - """ - Verify that signal is sent, received, and fires task - based on 'AUTO_CERTIFICATE_GENERATION' flag - """ - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): - CertificateWhitelistFactory( - user=self.user, - course_id=self.ip_course.id - ) - mock_generate_certificate_apply_async.assert_not_called() - - def test_cert_generation_on_allowlist_append_instructor_paced_cert_generation_enabled(self): - """ - Verify that signal is sent, received, and fires task - based on 'AUTO_CERTIFICATE_GENERATION' flag - """ - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): - CertificateWhitelistFactory( - user=self.user, - course_id=self.ip_course.id - ) - mock_generate_certificate_apply_async.assert_called_with( - countdown=CERTIFICATE_DELAY_SECONDS, - kwargs={ - 'student': str(self.user.id), - 'course_key': str(self.ip_course.id), - } - ) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) - def test_fire_task_allowlist_enabled(self): + def test_fire_task_allowlist_auto_enabled(self): """ - Test that the allowlist generation is invoked if the allowlist is enabled for a user on the list + Test that the allowlist generation is invoked if automatic generation is enabled """ with mock.patch( 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', @@ -182,9 +100,31 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): mock_generate_certificate_apply_async.assert_not_called() mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id) + @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) + def test_fire_task_allowlist_auto_disabled(self): + """ + Test that the allowlist generation is not invoked if automatic generation is disabled + """ + with mock.patch( + 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', + return_value=None + ) as mock_generate_certificate_apply_async: + with mock.patch( + 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', + return_value=None + ) as mock_generate_allowlist_task: + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): + CertificateWhitelistFactory( + user=self.user, + course_id=self.ip_course.id, + whitelist=True + ) + mock_generate_certificate_apply_async.assert_not_called() + mock_generate_allowlist_task.assert_not_called() + def test_fire_task_allowlist_disabled(self): """ - Test that the normal logic is followed if the allowlist is disabled for a user on the list + Test that the V1 logic is followed if the allowlist is disabled """ with mock.patch( 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index db1d857e2c..728c5e2c75 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1999,19 +1999,19 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): """ Verify that certificates generated for all eligible students enrolled in a course. """ - # create 10 students + # Create 10 students students = self._create_students(10) - # mark 2 students to have certificates generated already + # Grant 2 students downloadable certs for student in students[:2]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # white-list 5 students + # Allowlist 5 students for student in students[2:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) @@ -2027,37 +2027,27 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): with self.assertNumQueries(170): self.assertCertificatesGenerated(task_input, expected_results) - expected_results = { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 0, - 'succeeded': 0, - 'failed': 0, - 'skipped': 10 - } - with self.assertNumQueries(3): - self.assertCertificatesGenerated(task_input, expected_results) - @ddt.data( CertificateStatuses.downloadable, CertificateStatuses.generating, CertificateStatuses.notpassing, CertificateStatuses.audit_passing, ) - def test_certificate_generation_all_whitelisted(self, status): + def test_certificate_generation_all_allowlisted(self, status): """ - Verify that certificates are generated for all white-listed students, + Verify that certificates are generated for all allowlisted students, whether or not they already had certs generated for them. """ + # Create 5 students students = self._create_students(5) - # whitelist 3 + # Allowlist 3 students for student in students[:3]: CertificateWhitelistFactory.create( user=student, course_id=self.course.id, whitelist=True ) - # generate certs for 2 + # Grant certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, @@ -2066,7 +2056,8 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): ) task_input = {'student_set': 'all_whitelisted'} - # only certificates for the 3 whitelisted students should have been run + + # Only certificates for the 3 allowlisted students should have been run expected_results = { 'action_name': 'certificates generated', 'total': 3, @@ -2077,16 +2068,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): } self.assertCertificatesGenerated(task_input, expected_results) - # the first 3 students (who were whitelisted) have passing - # certificate statuses - for student in students[:3]: - assert GeneratedCertificate.certificate_for_student(student, self.course.id).status in\ - CertificateStatuses.PASSED_STATUSES - - # The last 2 students still don't have certs - for student in students[3:]: - assert GeneratedCertificate.certificate_for_student(student, self.course.id) is None - @ddt.data( (CertificateStatuses.downloadable, 2), (CertificateStatuses.generating, 2), @@ -2094,15 +2075,15 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): (CertificateStatuses.audit_passing, 4), ) @ddt.unpack - def test_certificate_generation_whitelisted_not_generated(self, status, expected_certs): + def test_certificate_generation_allowlist_not_generated(self, status, expected_certs): """ Verify that certificates are generated only for those students who do not have `downloadable` or `generating` certificates. """ - # create 5 students + # Create 5 students students = self._create_students(5) - # mark 2 students to have certificates generated already + # Grant certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, @@ -2110,7 +2091,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): status=status, ) - # white-list 4 students + # Allowlist 4 students for student in students[:4]: CertificateWhitelistFactory.create( user=student, course_id=self.course.id, whitelist=True @@ -2118,7 +2099,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): task_input = {'student_set': 'whitelisted_not_generated'} - # certificates should only be generated for the whitelisted students + # Certificates should only be generated for the allowlisted students # who do not yet have passing certificates. expected_results = { 'action_name': 'certificates generated', @@ -2133,15 +2114,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): expected_results ) - # the first 4 students have passing certificate statuses since - # they either were whitelisted or had one before - for student in students[:4]: - assert GeneratedCertificate.certificate_for_student(student, self.course.id).status in\ - CertificateStatuses.PASSED_STATUSES - - # The last student still doesn't have a cert - assert GeneratedCertificate.certificate_for_student(students[4], self.course.id) is None - def test_certificate_generation_specific_student(self): """ Tests generating a certificate for a specific student. @@ -2187,37 +2159,37 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): Verify that certificates are regenerated for all eligible students enrolled in a course whose generated certificate statuses lies in the list 'statuses_to_regenerate' given in task_input. """ - # create 10 students + # Create 10 students students = self._create_students(10) - # mark 2 students to have certificates generated already + # Grant downloadable certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # mark 3 students to have certificates generated with status 'error' + # Grant error certs to 3 students for student in students[2:5]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.error, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # mark 6th students to have certificates generated with status 'deleted' + # Grant a deleted cert to the 6th student for student in students[5:6]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.deleted, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # white-list 7 students + # Allowlist 7 students for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) @@ -2247,53 +2219,50 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Default grade for students default_grade = '-1' - # create 10 students + # Create 10 students students = self._create_students(10) - # mark 2 students to have certificates generated already + # Grant downloadable certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark 3 students to have certificates generated with status 'error' + # Grant error certs to 3 students for student in students[2:5]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.error, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark 6th students to have certificates generated with status 'deleted' + # Grant a deleted cert to the 6th student for student in students[5:6]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.deleted, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark rest of the 4 students with having generated certificates with status 'generating' - # These students are not added in white-list and they have not completed grades so certificate generation - # for these students should fail other than the one student that has been added to white-list - # so from these students 3 failures and 1 success + # Grant generating certs to 4 students for student in students[6:]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.generating, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # white-list 7 students + # Allowlist 7 students for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) @@ -2312,26 +2281,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): self.assertCertificatesGenerated(task_input, expected_results) - generated_certificates = GeneratedCertificate.eligible_certificates.filter( - user__in=students, - course_id=self.course.id, - mode='honor' - ) - certificate_statuses = [generated_certificate.status for generated_certificate in generated_certificates] - certificate_grades = [generated_certificate.grade for generated_certificate in generated_certificates] - - # Verify from results from database - # Certificates are being generated for 2 white-listed students that had statuses in 'deleted'' and 'generating' - assert certificate_statuses.count(CertificateStatuses.generating) == 2 - # 5 students are skipped that had Certificate Status 'downloadable' and 'error' - assert certificate_statuses.count(CertificateStatuses.downloadable) == 2 - assert certificate_statuses.count(CertificateStatuses.error) == 3 - - # grades will be '0.0' as students are either white-listed or ending in error - assert certificate_grades.count('0.0') == 5 - # grades will be '-1' for students that were skipped - assert certificate_grades.count(default_grade) == 5 - def test_certificate_regeneration_with_existing_unavailable_status(self): """ Verify that certificates are regenerated for all eligible students enrolled in a course whose generated @@ -2341,50 +2290,50 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Default grade for students default_grade = '-1' - # create 10 students + # Create 10 students students = self._create_students(10) - # mark 2 students to have certificates generated already + # Grant downloadable certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark 3 students to have certificates generated with status 'error' + # Grant error certs to 3 students for student in students[2:5]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.error, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark 2 students to have generated certificates with status 'unavailable' + # Grant unavailable certs to 2 students for student in students[5:7]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.unavailable, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # mark 3 students to have generated certificates with status 'generating' + # Grant generating certs to 3 students for student in students[7:]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.generating, - mode='honor', + mode=GeneratedCertificate.CourseMode.VERIFIED, grade=default_grade ) - # white-list all students + # Allowlist all students for student in students[:]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) @@ -2412,76 +2361,50 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): expected_results ) - generated_certificates = GeneratedCertificate.eligible_certificates.filter( - user__in=students, - course_id=self.course.id, - mode='honor' - ) - certificate_statuses = [generated_certificate.status for generated_certificate in generated_certificates] - certificate_grades = [generated_certificate.grade for generated_certificate in generated_certificates] - - # Verify from results from database - # Certificates are being generated for 8 students that had statuses in 'downloadable', 'error' and 'generating' - assert certificate_statuses.count(CertificateStatuses.generating) == 8 - # 2 students are skipped that had Certificate Status 'unavailable' - assert certificate_statuses.count(CertificateStatuses.unavailable) == 2 - - # grades will be '0.0' as students are white-listed and have not completed any tasks - assert certificate_grades.count('0.0') == 8 - # grades will be '-1' for students that have not been processed - assert certificate_grades.count(default_grade) == 2 - - # Verify that students with status 'unavailable were skipped - unavailable_certificates = \ - [cert for cert in generated_certificates - if cert.status == CertificateStatuses.unavailable and cert.grade == default_grade] - - assert len(unavailable_certificates) == 2 - def test_certificate_regeneration_for_students(self): """ Verify that certificates are regenerated for all students passed in task_input. """ - # create 10 students + # Create 10 students students = self._create_students(10) - # mark 2 students to have certificates generated already + # Grant downloadable certs to 2 students for student in students[:2]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # mark 3 students to have certificates generated with status 'error' + # Grant error certs to 3 students for student in students[2:5]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.error, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # mark 6th students to have certificates generated with status 'deleted' + # Grant a deleted cert to the 6th student for student in students[5:6]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.deleted, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # mark 7th students to have certificates generated with status 'notpassing' + # Grant a notpassing cert to the 7th student for student in students[6:7]: GeneratedCertificateFactory.create( user=student, course_id=self.course.id, status=CertificateStatuses.notpassing, - mode='honor' + mode=GeneratedCertificate.CourseMode.VERIFIED ) - # white-list 7 students + # Allowlist 7 students for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) @@ -2516,7 +2439,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): mode='verified' ) - # Whitelist a student + # Allowlist a student CertificateWhitelistFactory.create(user=s1, course_id=self.course.id) statuses = [CertificateStatuses.downloadable]