From 14e52bd773b2082d8721cb3f128385179cbf4897 Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah Date: Thu, 25 Jan 2024 15:05:43 +0500 Subject: [PATCH] fix: fix tests --- cms/envs/test.py | 7 ------- .../certificates/apis/v0/tests/test_views.py | 6 +++--- .../course_metadata/tests/test_views.py | 2 +- openedx/core/djangoapps/safe_sessions/middleware.py | 11 ++--------- .../djangoapps/user_api/accounts/tests/test_views.py | 10 +++++----- 5 files changed, 11 insertions(+), 25 deletions(-) diff --git a/cms/envs/test.py b/cms/envs/test.py index 9d65525768ed..118d7e27a79c 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -223,13 +223,6 @@ FEATURES['DISABLE_SET_JWT_COOKIES_FOR_TESTS'] = True -# Skip setting user emails in session for specific unit tests. This is necessary -# because certain views raise Http404() exceptions, and since Django wraps all -# tests in transactions, modifying the session and attempting to save it after -# a transaction failure due to Http404() can result in test failures. This is -# because errors within a transaction affect subsequent DB operations. -FEATURES['DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS'] = False - FEATURES['ENABLE_SERVICE_STATUS'] = True # Toggles embargo on for testing diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index 9e62eab6d837..fd31e3456961 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -309,7 +309,7 @@ def test_no_certificate(self): def test_query_counts(self): # Test student with no certificates student_no_cert = UserFactory.create(password=self.user_password) - with self.assertNumQueries(17, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(21, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -319,7 +319,7 @@ def test_query_counts(self): assert len(resp.data) == 0 # Test student with 1 certificate - with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -359,7 +359,7 @@ def test_query_counts(self): download_url='www.google.com', grade="0.88", ) - with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, diff --git a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py index e3b4c3f186ed..4b3524eb1ee2 100644 --- a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py @@ -85,8 +85,8 @@ def test_get_masqueraded_user(self): self.update_masquerade(username=self.user.username) assert self.client.get(self.url).data['username'] == self.user.username - @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS": True}) def test_get_unknown_course(self): + self.client.logout() url = reverse('course-home:course-metadata', args=['course-v1:unknown+course+2T2020']) # Django TestCase wraps every test in a transaction, so we must specifically wrap this when we expect an error with transaction.atomic(): diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index e0e9143cc2d2..f3948217efd9 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -816,16 +816,9 @@ def process_response(self, request, response): if request.session.get('email', None) is None: # .. custom_attribute_name: session_with_no_email_found # .. custom_attribute_description: Indicates that user's email was not - # stored in the user's session. + # yet stored in the user's session. set_custom_attribute('session_with_no_email_found', True) - - # Skip setting user emails in session for specific unit tests. This is necessary - # because certain views raise Http404() exceptions, and since Django wraps all - # tests in transactions, modifying the session and attempting to save it after - # a transaction failure due to Http404() can result in test failures. This is - # because errors within a transaction affect subsequent DB operations. - if not settings.FEATURES.get('DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS', False): - request.session['email'] = request.user.email + request.session['email'] = request.user.email if request_cache.get_cached_response('email_change_requested').is_found: # Update the JWT cookies with new user email diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 546b8afacc2b..0496f45ae2fc 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -232,7 +232,7 @@ def test_get_username(self): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(16) + self._verify_get_own_username(19) def test_get_username_inactive(self): """ @@ -242,7 +242,7 @@ def test_get_username_inactive(self): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(16) + self._verify_get_own_username(19) def test_get_username_not_logged_in(self): """ @@ -358,7 +358,7 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe """ ENABLED_CACHES = ['default'] - TOTAL_QUERY_COUNT = 24 + TOTAL_QUERY_COUNT = 27 FULL_RESPONSE_FIELD_COUNT = 29 def setUp(self): @@ -811,7 +811,7 @@ def verify_get_own_information(queries): assert data['time_zone'] is None self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(self._get_num_queries(22)) + verify_get_own_information(self._get_num_queries(25)) # Now make sure that the user can get the same information, even if not active self.user.is_active = False @@ -831,7 +831,7 @@ def test_get_account_empty_string(self): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(self._get_num_queries(22), table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(self._get_num_queries(25), table_ignorelist=WAFFLE_TABLES): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "state", "bio",): assert response.data[empty_field] is None