From be1aaa00ff994b66c6b00281b17a254a51771c23 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Tue, 28 Apr 2020 16:12:29 -0400 Subject: [PATCH] remove tests causing flakiness (#23827) These removed tests can be restored once someone works out why they were causing flakiness elsewhere and fix the problem. Reverts tests added in: https://github.com/edx/edx-platform/commit/dca50aacc3093c29c830cccae5746247ecb6f02b --- .../certificates/tests/test_models.py | 14 - .../programs/tasks/v1/tests/test_tasks.py | 274 ------------------ .../djangoapps/programs/tests/test_signals.py | 61 ---- 3 files changed, 349 deletions(-) diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index 5a43e5b0ca..3d26cebd39 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -9,7 +9,6 @@ from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase from django.test.utils import override_settings -from mock import patch from opaque_keys.edx.locator import CourseKey, CourseLocator from path import Path as path @@ -358,16 +357,3 @@ class CertificateInvalidationTest(SharedModuleStoreTestCase): self.assertFalse( CertificateInvalidation.has_certificate_invalidation(self.user, self.course_id) ) - - @patch('openedx.core.djangoapps.programs.tasks.v1.tasks.revoke_program_certificates.delay') - @patch( - 'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled', - return_value=True, - ) - def test_revoke_program_certificates(self, mock_issuance, mock_revoke_task): # pylint: disable=unused-argument - """ Verify that `revoke_program_certificates` is invoked upon invalidation. """ - # Invalidate user certificate - self.certificate.invalidate() - - self.assertEqual(mock_revoke_task.call_count, 1) - self.assertEqual(mock_revoke_task.call_args[0], (self.user.username, self.course_id)) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index ef3c80c6fb..230dc13862 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -591,277 +591,3 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): tasks.award_course_certificate.delay(self.student.username, str(self.certificate.course_id)).get() self.assertFalse(mock_post_course_certificate.called) - - -@skip_unless_lms -class RevokeProgramCertificateTestCase(TestCase): - """ - Test the revoke_program_certificate function - """ - - @httpretty.activate - def test_revoke_program_certificate(self): - """ - Ensure the correct API call gets made - """ - test_username = 'test-username' - test_client = EdxRestApiClient('http://test-server', jwt='test-token') - - httpretty.register_uri( - httpretty.POST, - 'http://test-server/credentials/', - ) - - tasks.revoke_program_certificate(test_client, test_username, 123) - - expected_body = { - 'username': test_username, - 'status': 'revoked', - 'credential': { - 'program_uuid': 123, - 'type': tasks.PROGRAM_CERTIFICATE, - } - } - last_request_body = httpretty.last_request().body.decode('utf-8') - self.assertEqual(json.loads(last_request_body), expected_body) - - -@skip_unless_lms -@ddt.ddt -@mock.patch(TASKS_MODULE + '.revoke_program_certificate') -@mock.patch(TASKS_MODULE + '.get_certified_programs') -@mock.patch(TASKS_MODULE + '.get_inverted_programs') -@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username') -class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): - """ - Tests for the 'revoke_program_certificates' celery task. - """ - - def setUp(self): - super(RevokeProgramCertificatesTestCase, self).setUp() - - self.student = UserFactory.create(username='test-student') - self.course_key = 'course-v1:testX+test101+2T2020' - self.site = SiteFactory() - self.site_configuration = SiteConfigurationFactory(site=self.site) - ApplicationFactory.create(name='credentials') - UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME) - self.create_credentials_config() - - self.inverted_programs = {self.course_key: [{'uuid': 1}, {'uuid': 2}]} - - def test_inverted_programs( - self, - mock_get_inverted_programs, - mock_get_certified_programs, # pylint: disable=unused-argument - mock_revoke_program_certificate, # pylint: disable=unused-argument - ): - """ - Checks that the Programs API is used correctly to determine completed - programs. - """ - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - mock_get_inverted_programs.assert_any_call(self.student) - - def test_revokinging_certificate( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Checks that the Credentials API is used to revoke certificates for - the proper programs. - """ - expected_program_uuid = 1 - mock_get_inverted_programs.return_value = { - self.course_key: [{'uuid': expected_program_uuid}] - } - mock_get_certified_programs.return_value = [expected_program_uuid] - - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - - call_args, _ = mock_revoke_program_certificate.call_args - self.assertEqual(call_args[1], self.student.username) - self.assertEqual(call_args[2], expected_program_uuid) - - @ddt.data( - ('credentials', 'enable_learner_issuance'), - ) - @ddt.unpack - def test_retry_if_config_disabled( - self, - disabled_config_type, - disabled_config_attribute, - *mock_helpers - ): - """ - Checks that the task is aborted if any relevant api configs are - disabled. - """ - getattr(self, 'create_{}_config'.format(disabled_config_type))(**{disabled_config_attribute: False}) - with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning: - with self.assertRaises(MaxRetriesExceededError): - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - self.assertTrue(mock_warning.called) - for mock_helper in mock_helpers: - self.assertFalse(mock_helper.called) - - def test_abort_if_invalid_username(self, *mock_helpers): - """ - Checks that the task will be aborted and not retried if the username - passed was not found, and that an exception is logged. - """ - with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception: - tasks.revoke_program_certificates.delay('nonexistent-username', self.course_key).get() - self.assertTrue(mock_exception.called) - for mock_helper in mock_helpers: - self.assertFalse(mock_helper.called) - - def test_abort_if_no_program( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Checks that the task will be aborted without further action if course is - not part of any program. - """ - mock_get_inverted_programs.return_value = {} - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - self.assertTrue(mock_get_inverted_programs.called) - self.assertFalse(mock_get_certified_programs.called) - self.assertFalse(mock_revoke_program_certificate.called) - - def _make_side_effect(self, side_effects): - """ - DRY helper. Returns a side effect function for use with mocks that - will be called multiple times, permitting Exceptions to be raised - (or not) in a specified order. - - See Also: - http://www.voidspace.org.uk/python/mock/examples.html#multiple-calls-with-different-effects - http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.side_effect - - """ - - def side_effect(*_a): - if side_effects: - exc = side_effects.pop(0) - if exc: - raise exc - return mock.DEFAULT - - return side_effect - - def test_continue_revoking_certs_if_error( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Checks that a single failure to revoke one of several certificates - does not cause the entire task to fail. Also ensures that - successfully revoked certs are logged as INFO and warning is logged - for failed requests if there are retries available. - """ - mock_get_inverted_programs.return_value = self.inverted_programs - mock_get_certified_programs.side_effect = [[1], [1, 2]] - mock_revoke_program_certificate.side_effect = self._make_side_effect([Exception('boom'), None]) - - with mock.patch(TASKS_MODULE + '.LOGGER.info') as mock_info, \ - mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning: - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - - self.assertEqual(mock_revoke_program_certificate.call_count, 3) - mock_warning.assert_called_once_with( - u'Failed to revoke certificate for program {uuid} of user {username}.'.format( - uuid=1, - username=self.student.username) - ) - mock_info.assert_any_call(mock.ANY, 1, self.student.username) - mock_info.assert_any_call(mock.ANY, 2, self.student.username) - - def test_retry_on_credentials_api_errors( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Ensures that any otherwise-unhandled errors that arise while trying - to get existing program credentials (e.g. network issues or other - transient API errors) will cause the task to be failed and queued for - retry. - """ - mock_get_inverted_programs.return_value = self.inverted_programs - mock_get_certified_programs.return_value = [1] - mock_get_certified_programs.side_effect = self._make_side_effect([Exception('boom'), None]) - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - self.assertEqual(mock_get_certified_programs.call_count, 2) - self.assertEqual(mock_revoke_program_certificate.call_count, 1) - - def test_retry_on_credentials_api_429_error( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Verify that a 429 error causes the task to fail and then retry. - """ - exception = exceptions.HttpClientError() - exception.response = mock.Mock(status_code=429) - mock_get_inverted_programs.return_value = self.inverted_programs - mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) - - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - - self.assertEqual(mock_revoke_program_certificate.call_count, 3) - - def test_no_retry_on_credentials_api_404_error( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Verify that a 404 error causes the task to fail but there is no retry. - """ - exception = exceptions.HttpNotFoundError() - exception.response = mock.Mock(status_code=404) - mock_get_inverted_programs.return_value = self.inverted_programs - mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) - - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - - self.assertEqual(mock_revoke_program_certificate.call_count, 2) - - def test_no_retry_on_credentials_api_4XX_error( - self, - mock_get_inverted_programs, - mock_get_certified_programs, - mock_revoke_program_certificate, - ): - """ - Verify that other 4XX errors cause task to fail but there is no retry. - """ - exception = exceptions.HttpClientError() - exception.response = mock.Mock(status_code=418) - mock_get_inverted_programs.return_value = self.inverted_programs - mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) - - tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() - - self.assertEqual(mock_revoke_program_certificate.call_count, 2) diff --git a/openedx/core/djangoapps/programs/tests/test_signals.py b/openedx/core/djangoapps/programs/tests/test_signals.py index dfb2817a19..b47921e8e3 100644 --- a/openedx/core/djangoapps/programs/tests/test_signals.py +++ b/openedx/core/djangoapps/programs/tests/test_signals.py @@ -163,64 +163,3 @@ class CertChangedReceiverTest(TestCase): site_config.save() handle_course_cert_changed(**self.signal_kwargs) self.assertFalse(mock_task.called) - - -# The credentials app isn't installed for the CMS. -@skip_unless_lms -@mock.patch('openedx.core.djangoapps.programs.tasks.v1.tasks.revoke_program_certificates.delay') -@mock.patch( - 'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled', - new_callable=mock.PropertyMock, - return_value=False, -) -class CertRevokedReceiverTest(TestCase): - """ - Tests for the `handle_course_cert_revoked` signal handler function. - """ - - @property - def signal_kwargs(self): - """ - DRY helper. - """ - return dict( - sender=self.__class__, - user=UserFactory.create(username=TEST_USERNAME), - course_key=TEST_COURSE_KEY, - mode='test-mode', - status='test-status', - ) - - def test_signal_received(self, mock_is_learner_issuance_enabled, mock_task): # pylint: disable=unused-argument - """ - Ensures the receiver function is invoked when COURSE_CERT_REVOKED is - sent. - - Suboptimal: because we cannot mock the receiver function itself (due - to the way django signals work), we mock a configuration call that is - known to take place inside the function. - """ - COURSE_CERT_REVOKED.send(**self.signal_kwargs) - self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) - - def test_programs_disabled(self, mock_is_learner_issuance_enabled, mock_task): - """ - Ensures that the receiver function does nothing when the credentials API - configuration is not enabled. - """ - handle_course_cert_revoked(**self.signal_kwargs) - self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) - self.assertEqual(mock_task.call_count, 0) - - def test_programs_enabled(self, mock_is_learner_issuance_enabled, mock_task): - """ - Ensures that the receiver function invokes the expected celery task - when the credentials API configuration is enabled. - """ - mock_is_learner_issuance_enabled.return_value = True - - handle_course_cert_revoked(**self.signal_kwargs) - - self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) - self.assertEqual(mock_task.call_count, 1) - self.assertEqual(mock_task.call_args[0], (TEST_USERNAME, TEST_COURSE_KEY))