From ea49975a733585d85e0ea953777b202015653268 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 11:08:25 -0400 Subject: [PATCH 01/24] BOM-617 Fix test_static_replace test failures. Update the tests around static_file code to use byte streams instead of string streams for generating static content. This is a fix to get these tests passing on python 3. --- .../djangoapps/static_replace/test/test_static_replace.py | 6 +++--- common/lib/xmodule/xmodule/contentstore/content.py | 6 +++--- common/lib/xmodule/xmodule/contentstore/mongo.py | 4 +++- common/lib/xmodule/xmodule/modulestore/__init__.py | 3 ++- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index ace36a09fe..04da1ad689 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -3,7 +3,7 @@ from __future__ import absolute_import, print_function import re -from six import StringIO +from six import BytesIO from six.moves.urllib.parse import parse_qsl, urlparse, urlunparse import ddt @@ -331,7 +331,7 @@ class CanonicalContentTest(SharedModuleStoreTestCase): StaticContent: the StaticContent object for the created image """ new_image = Image.new('RGB', dimensions, color) - new_buf = StringIO() + new_buf = BytesIO() new_image.save(new_buf, format='png') new_buf.seek(0) new_name = name.format(prefix) @@ -355,7 +355,7 @@ class CanonicalContentTest(SharedModuleStoreTestCase): StaticContent: the StaticContent object for the created content """ - new_buf = StringIO('testingggggggggggg') + new_buf = BytesIO(b'testingggggggggggg') new_name = name.format(prefix) new_key = StaticContent.compute_location(cls.courses[prefix].id, new_name) new_content = StaticContent(new_key, new_name, 'application/octet-stream', new_buf.getvalue(), locked=locked) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index bbfb8fe80e..0842219b90 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -270,7 +270,7 @@ class StaticContent(object): if query_val.startswith("/static/"): new_val = StaticContent.get_canonicalized_asset_path( course_key, query_val, base_url, excluded_exts, encode=False) - updated_query_params.append((query_name, new_val)) + updated_query_params.append((query_name, new_val.encode('utf-8'))) else: # Make sure we're encoding Unicode strings down to their byte string # representation so that `urlencode` can handle it. @@ -286,11 +286,11 @@ class StaticContent(object): # Only encode this if told to. Important so that we don't double encode # when working with paths that are in query parameters. - asset_path = asset_path.encode('utf-8') if encode: + asset_path = asset_path.encode('utf-8') asset_path = quote_plus(asset_path, '/:+@') - return urlunparse(('', base_url.encode('utf-8'), asset_path, params, urlencode(updated_query_params), '')) + return urlunparse(('', base_url, asset_path, params, urlencode(updated_query_params), '')) def stream_data(self): yield self._data diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index f3faa47146..7ab6c42d2d 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -99,7 +99,9 @@ class MongoContentStore(ContentStore): import_path=content.import_path, # getattr b/c caching may mean some pickled instances don't have attr locked=getattr(content, 'locked', False)) as fp: - if hasattr(content.data, '__iter__'): + # It seems that this code thought that only some specific object would have the `__iter__` attribute + # but the bytes object in python 3 has one and should not use the chunking logic. + if hasattr(content.data, '__iter__') and not isinstance(content.data, six.binary_type): for chunk in content.data: fp.write(chunk) else: diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 7993996fe7..1d72ef9169 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -1298,7 +1298,8 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): result = defaultdict(dict) if fields is None: return result - cls = self.mixologist.mix(XBlock.load_class(category, select=prefer_xmodules)) + classes = XBlock.load_class(category, select=prefer_xmodules) + cls = self.mixologist.mix(classes) for field_name, value in six.iteritems(fields): field = getattr(cls, field_name) result[field.scope][field_name] = value From 220759ee1de2fe8483e55b017107694f25be9b78 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 11:52:15 -0400 Subject: [PATCH 02/24] Remove unnecessary print statements. --- common/djangoapps/static_replace/test/test_static_replace.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index 04da1ad689..e49cbd81a7 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -588,8 +588,6 @@ class CanonicalContentTest(SharedModuleStoreTestCase): with check_mongo_calls(mongo_calls): asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, exts) - print(expected) - print(asset_path) self.assertIsNotNone(re.match(expected, asset_path)) @ddt.data( @@ -786,6 +784,4 @@ class CanonicalContentTest(SharedModuleStoreTestCase): with check_mongo_calls(mongo_calls): asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, exts) - print(expected) - print(asset_path) self.assertIsNotNone(re.match(expected, asset_path)) From 6f183831420b24c9695616622198d82f030b6ee9 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 11:59:24 -0400 Subject: [PATCH 03/24] Fix bytes/string handling in tracking middleware --- common/djangoapps/track/middleware.py | 10 +++++++--- .../djangoapps/track/tests/test_middleware.py | 20 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/track/middleware.py b/common/djangoapps/track/middleware.py index 6ccb7617af..77cfda2650 100644 --- a/common/djangoapps/track/middleware.py +++ b/common/djangoapps/track/middleware.py @@ -144,7 +144,10 @@ class TrackMiddleware(object): # HTTP headers may contain Latin1 characters. Decoding using Latin1 encoding here # avoids encountering UnicodeDecodeError exceptions when these header strings are # output to tracking logs. - context[context_key] = request.META.get(header_name, '').decode('latin1') + context_value = request.META.get(header_name, '') + if isinstance(context_value, six.binary_type): + context_value = context_value.decode('latin1') + context[context_key] = context_value # Google Analytics uses the clientId to keep track of unique visitors. A GA cookie looks like # this: _ga=GA1.2.1033501218.1368477899. The clientId is this part: 1033501218.1368477899. @@ -183,8 +186,9 @@ class TrackMiddleware(object): # Using a known-insecure hash to shorten is silly. # Also, why do we need same length? key_salt = "common.djangoapps.track" + self.__class__.__name__ - key = hashlib.md5(key_salt + settings.SECRET_KEY).digest() - encrypted_session_key = hmac.new(key, msg=session_key, digestmod=hashlib.md5).hexdigest() + key_bytes = (key_salt + settings.SECRET_KEY).encode('utf-8') + key = hashlib.md5(key_bytes).digest() + encrypted_session_key = hmac.new(key, msg=session_key.encode('utf-8'), digestmod=hashlib.md5).hexdigest() return encrypted_session_key def get_user_primary_key(self, request): diff --git a/common/djangoapps/track/tests/test_middleware.py b/common/djangoapps/track/tests/test_middleware.py index 8cf800f290..8fe672324b 100644 --- a/common/djangoapps/track/tests/test_middleware.py +++ b/common/djangoapps/track/tests/test_middleware.py @@ -49,9 +49,7 @@ class TrackMiddlewareTestCase(TestCase): request.META[meta_key] = 'test latin1 \xd3 \xe9 \xf1' # pylint: disable=no-member context = self.get_context_for_request(request) - # The bytes in the string on the right are utf8 encoded in the source file, so we decode them to construct - # a valid unicode string. - self.assertEqual(context[context_key], 'test latin1 Ó é ñ'.decode('utf8')) + self.assertEqual(context[context_key], u'test latin1 Ó é ñ') def test_default_filters_do_not_render_view(self): for url in ['/event', '/event/1', '/login', '/heartbeat']: @@ -79,7 +77,7 @@ class TrackMiddlewareTestCase(TestCase): def test_default_request_context(self): context = self.get_context_for_path('/courses/') - self.assertEquals(context, { + self.assertEqual(context, { 'accept_language': '', 'referer': '', 'user_id': '', @@ -101,7 +99,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['REMOTE_ADDR'] = remote_addr context = self.get_context_for_request(request) - self.assertEquals(context['ip'], remote_addr) + self.assertEqual(context['ip'], remote_addr) def test_single_forward_for_header_ip_context(self): request = self.request_factory.get('/courses/') @@ -112,7 +110,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip context = self.get_context_for_request(request) - self.assertEquals(context['ip'], forwarded_ip) + self.assertEqual(context['ip'], forwarded_ip) def test_multiple_forward_for_header_ip_context(self): request = self.request_factory.get('/courses/') @@ -123,7 +121,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip context = self.get_context_for_request(request) - self.assertEquals(context['ip'], '11.22.33.44') + self.assertEqual(context['ip'], '11.22.33.44') def get_context_for_path(self, path): """Extract the generated event tracking context for a given request for the given path.""" @@ -138,7 +136,7 @@ class TrackMiddlewareTestCase(TestCase): finally: self.track_middleware.process_response(request, None) - self.assertEquals( + self.assertEqual( tracker.get_tracker().resolve_context(), {} ) @@ -156,7 +154,7 @@ class TrackMiddlewareTestCase(TestCase): def assert_dict_subset(self, superset, subset): """Assert that the superset dict contains all of the key-value pairs found in the subset dict.""" for key, expected_value in six.iteritems(subset): - self.assertEquals(superset[key], expected_value) + self.assertEqual(superset[key], expected_value) def test_request_with_user(self): user_id = 1 @@ -177,7 +175,7 @@ class TrackMiddlewareTestCase(TestCase): request.session.save() session_key = request.session.session_key expected_session_key = self.track_middleware.encrypt_session_key(session_key) - self.assertEquals(len(session_key), len(expected_session_key)) + self.assertEqual(len(session_key), len(expected_session_key)) context = self.get_context_for_request(request) self.assert_dict_subset(context, { 'session': expected_session_key, @@ -188,7 +186,7 @@ class TrackMiddlewareTestCase(TestCase): session_key = '665924b49a93e22b46ee9365abf28c2a' expected_session_key = '3b81f559d14130180065d635a4f35dd2' encrypted_session_key = self.track_middleware.encrypt_session_key(session_key) - self.assertEquals(encrypted_session_key, expected_session_key) + self.assertEqual(encrypted_session_key, expected_session_key) def test_request_headers(self): ip_address = '10.0.0.0' From 42cfb3ec8fa3cbb361057d344d0fc17018fd9050 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 12:08:36 -0400 Subject: [PATCH 04/24] Fix last common/djangoapps/track test --- common/djangoapps/track/views/tests/test_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/track/views/tests/test_views.py b/common/djangoapps/track/views/tests/test_views.py index f5ce7539a8..a715d6cc5b 100644 --- a/common/djangoapps/track/views/tests/test_views.py +++ b/common/djangoapps/track/views/tests/test_views.py @@ -3,6 +3,7 @@ from __future__ import absolute_import import ddt +import six from django.contrib.auth.models import User from django.test.client import RequestFactory from django.test.utils import override_settings @@ -316,7 +317,7 @@ class TestTrackViews(EventTrackingTestCase): } task_info = { - sentinel.task_key: sentinel.task_value + six.text_type(sentinel.task_key): sentinel.task_value } expected_event_data = dict(task_info) expected_event_data.update(self.event) From f68c7e87f7d6d188140ffc727bdd8795ae4b92bd Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 12:59:32 -0400 Subject: [PATCH 05/24] Fix student.helpers logging tests --- common/djangoapps/student/helpers.py | 12 ++++++------ common/djangoapps/student/tests/test_helpers.py | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index fe82520fdb..366d6d33d5 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -327,14 +327,14 @@ def _get_redirect_to(request): mime_type, _ = mimetypes.guess_type(redirect_to, strict=False) if not is_safe_login_or_logout_redirect(request, redirect_to): log.warning( - u'Unsafe redirect parameter detected after login page: %(redirect_to)r', + u"Unsafe redirect parameter detected after login page: '%(redirect_to)s'", {"redirect_to": redirect_to} ) redirect_to = None elif 'text/html' not in header_accept: log.info( - u'Redirect to non html content %(content_type)r detected from %(user_agent)r' - u' after login page: %(redirect_to)r', + u"Redirect to non html content '%(content_type)s' detected from '%(user_agent)s'" + u" after login page: '%(redirect_to)s'", { "redirect_to": redirect_to, "content_type": header_accept, "user_agent": request.META.get('HTTP_USER_AGENT', '') @@ -343,13 +343,13 @@ def _get_redirect_to(request): redirect_to = None elif mime_type: log.warning( - u'Redirect to url path with specified filed type %(mime_type)r not allowed: %(redirect_to)r', + u"Redirect to url path with specified filed type '%(mime_type)s' not allowed: '%(redirect_to)s'", {"redirect_to": redirect_to, "mime_type": mime_type} ) redirect_to = None elif settings.STATIC_URL in redirect_to: log.warning( - u'Redirect to static content detected after login page: %(redirect_to)r', + u"Redirect to static content detected after login page: '%(redirect_to)s'", {"redirect_to": redirect_to} ) redirect_to = None @@ -359,7 +359,7 @@ def _get_redirect_to(request): for theme in themes: if theme.theme_dir_name in next_path: log.warning( - u'Redirect to theme content detected after login page: %(redirect_to)r', + u"Redirect to theme content detected after login page: '%(redirect_to)s'", {"redirect_to": redirect_to} ) redirect_to = None diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py index 5a7f195960..07ca1d319b 100644 --- a/common/djangoapps/student/tests/test_helpers.py +++ b/common/djangoapps/student/tests/test_helpers.py @@ -38,20 +38,20 @@ class TestLoginHelper(TestCase): @ddt.data( (logging.WARNING, "WARNING", "https://www.amazon.com", "text/html", None, - "Unsafe redirect parameter detected after login page: u'https://www.amazon.com'"), + "Unsafe redirect parameter detected after login page: 'https://www.amazon.com'"), (logging.WARNING, "WARNING", "testserver/edx.org/images/logo", "text/html", None, - "Redirect to theme content detected after login page: u'testserver/edx.org/images/logo'"), + "Redirect to theme content detected after login page: 'testserver/edx.org/images/logo'"), (logging.INFO, "INFO", "favicon.ico", "image/*", "test/agent", - "Redirect to non html content 'image/*' detected from 'test/agent' after login page: u'favicon.ico'"), + "Redirect to non html content 'image/*' detected from 'test/agent' after login page: 'favicon.ico'"), (logging.WARNING, "WARNING", "https://www.test.com/test.jpg", "image/*", None, - "Unsafe redirect parameter detected after login page: u'https://www.test.com/test.jpg'"), + "Unsafe redirect parameter detected after login page: 'https://www.test.com/test.jpg'"), (logging.INFO, "INFO", static_url + "dummy.png", "image/*", "test/agent", - "Redirect to non html content 'image/*' detected from 'test/agent' after login page: u'" + static_url + + "Redirect to non html content 'image/*' detected from 'test/agent' after login page: '" + static_url + "dummy.png" + "'"), (logging.WARNING, "WARNING", "test.png", "text/html", None, - "Redirect to url path with specified filed type 'image/png' not allowed: u'test.png'"), + "Redirect to url path with specified filed type 'image/png' not allowed: 'test.png'"), (logging.WARNING, "WARNING", static_url + "dummy.png", "text/html", None, - "Redirect to url path with specified filed type 'image/png' not allowed: u'" + static_url + "dummy.png" + "'"), + "Redirect to url path with specified filed type 'image/png' not allowed: '" + static_url + "dummy.png" + "'"), ) @ddt.unpack def test_next_failures(self, log_level, log_name, unsafe_url, http_accept, user_agent, expected_log): From 1e4fac644088859f24f1ae4a403068de59ce82e3 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 13:29:32 -0400 Subject: [PATCH 06/24] Better tests of stringifying objects. BOM-730 and BOM-731 --- .../management/commands/tests/test_dump_to_neo4j.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py b/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py index 40095b17ec..2279ffefa6 100644 --- a/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py +++ b/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py @@ -10,7 +10,6 @@ import ddt import mock import six from django.core.management import call_command -from django.utils import six from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -230,6 +229,12 @@ class TestDumpToNeo4jCommand(TestDumpToNeo4jCommandBase): ) +class SomeThing(object): + """Just to test the stringification of an object.""" + def __str__(self): + return "" + + @skip_unless_lms @ddt.ddt class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase): @@ -379,7 +384,7 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase): @ddt.data( (1, 1), - (object, ""), + (SomeThing(), ""), (1.5, 1.5), ("úñîçø∂é", "úñîçø∂é"), (b"plain string", b"plain string"), @@ -388,7 +393,8 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase): ((1,), "(1,)"), # list of elements should be coerced into a list of the # string representations of those elements - ([object, object], ["", ""]) + ([SomeThing(), SomeThing()], ["", ""]), + ([1, 2], ["1", "2"]), ) @ddt.unpack def test_coerce_types(self, original_value, coerced_expected): From b027437e8037eb28811b3f49241d6ec138d9a5e1 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 13:42:55 -0400 Subject: [PATCH 07/24] get_all_orgs returns a set, not a list. BOM-731 --- .../test/test_lms_filter_generator.py | 6 +++--- openedx/core/djangoapps/site_configuration/helpers.py | 2 +- openedx/core/djangoapps/site_configuration/models.py | 2 +- .../site_configuration/tests/test_models.py | 11 +++-------- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lms/lib/courseware_search/test/test_lms_filter_generator.py b/lms/lib/courseware_search/test/test_lms_filter_generator.py index a57e9e195c..84fd7da6a6 100644 --- a/lms/lib/courseware_search/test/test_lms_filter_generator.py +++ b/lms/lib/courseware_search/test/test_lms_filter_generator.py @@ -105,7 +105,7 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): self.assertEqual('LogistrationX', exclude_orgs[0]) self.assertEqual('TestSiteX', exclude_orgs[1]) - @patch('openedx.core.djangoapps.site_configuration.helpers.get_all_orgs', Mock(return_value=[])) + @patch('openedx.core.djangoapps.site_configuration.helpers.get_all_orgs', Mock(return_value=set())) def test_no_excludes_with_no_orgs(self): """ Test when no org is present - nothing to exclude """ _, _, exclude_dictionary = LmsSearchFilterGenerator.generate_field_filters(user=self.user) @@ -120,7 +120,7 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): @patch( 'openedx.core.djangoapps.site_configuration.helpers.get_all_orgs', - Mock(return_value=["TestSite1", "TestSite2", "TestSite3", "TestSite4"]) + Mock(return_value={"TestSite1", "TestSite2", "TestSite3", "TestSite4"}) ) def test_excludes_multi_orgs(self): _, _, exclude_dictionary = LmsSearchFilterGenerator.generate_field_filters(user=self.user) @@ -134,7 +134,7 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): @patch( 'openedx.core.djangoapps.site_configuration.helpers.get_all_orgs', - Mock(return_value=["TestSite1", "TestSite2", "TestSite3", "TestSite4"]) + Mock(return_value={"TestSite1", "TestSite2", "TestSite3", "TestSite4"}) ) @patch('openedx.core.djangoapps.site_configuration.helpers.get_value', Mock(return_value='TestSite3')) def test_excludes_multi_orgs_within(self): diff --git a/openedx/core/djangoapps/site_configuration/helpers.py b/openedx/core/djangoapps/site_configuration/helpers.py index eb4c0d737c..4af9bcd0f9 100644 --- a/openedx/core/djangoapps/site_configuration/helpers.py +++ b/openedx/core/djangoapps/site_configuration/helpers.py @@ -210,7 +210,7 @@ def get_all_orgs(): This can be used, for example, to do filtering. Returns: - A list of all organizations present in the site configuration. + A set of all organizations present in the site configuration. """ # Import is placed here to avoid model import at project startup. from openedx.core.djangoapps.site_configuration.models import SiteConfiguration diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index b20631fe93..6af78a171e 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -114,7 +114,7 @@ class SiteConfiguration(models.Model): for example, to do filtering. Returns: - A list of all organizations present in site configuration. + A set of all organizations present in site configuration. """ org_filter_set = set() diff --git a/openedx/core/djangoapps/site_configuration/tests/test_models.py b/openedx/core/djangoapps/site_configuration/tests/test_models.py index afd262d889..a753e6cbb7 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_models.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_models.py @@ -4,6 +4,7 @@ Tests for site configuration's django models. from __future__ import absolute_import from mock import patch +import six from django.test import TestCase from django.db import IntegrityError, transaction @@ -321,10 +322,7 @@ class SiteConfigurationTests(TestCase): ) # Test that the default value is returned if the value for the given key is not found in the configuration - self.assertListEqual( - list(SiteConfiguration.get_all_orgs()), - expected_orgs, - ) + six.assertCountEqual(self, SiteConfiguration.get_all_orgs(), expected_orgs) def test_get_all_orgs_returns_only_enabled(self): """ @@ -343,7 +341,4 @@ class SiteConfigurationTests(TestCase): ) # Test that the default value is returned if the value for the given key is not found in the configuration - self.assertListEqual( - list(SiteConfiguration.get_all_orgs()), - expected_orgs, - ) + six.assertCountEqual(self, SiteConfiguration.get_all_orgs(), expected_orgs) From 0c207859cdeba757d4116dbc6d2ee0a0266e4c50 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 13:43:01 -0400 Subject: [PATCH 08/24] Fix most tests in user_authn --- openedx/core/djangoapps/user_authn/views/login.py | 5 +++-- .../core/djangoapps/user_authn/views/tests/test_views.py | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index d730926602..a623f73792 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -8,6 +8,7 @@ from __future__ import absolute_import import logging +import six from django.conf import settings from django.contrib.auth import authenticate from django.contrib.auth import login as django_login @@ -110,11 +111,11 @@ def _enforce_password_policy_compliance(request, user): password_policy_compliance.enforce_compliance_on_login(user, request.POST.get('password')) except password_policy_compliance.NonCompliantPasswordWarning as e: # Allow login, but warn the user that they will be required to reset their password soon. - PageLevelMessages.register_warning_message(request, e.message) + PageLevelMessages.register_warning_message(request, six.text_type(e)) except password_policy_compliance.NonCompliantPasswordException as e: send_password_reset_email_for_user(user, request) # Prevent the login attempt. - raise AuthFailedError(e.message) + raise AuthFailedError(six.text_type(e)) def _generate_not_activated_message(user): diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_views.py b/openedx/core/djangoapps/user_authn/views/tests/test_views.py index f50ce173e6..3391bf0835 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_views.py @@ -323,7 +323,7 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto visible=True, enabled=True, icon_class='', - icon_image=SimpleUploadedFile('icon.svg', ''), + icon_image=SimpleUploadedFile('icon.svg', b''), ) self.hidden_enabled_provider = self.configure_linkedin_provider( visible=False, @@ -606,7 +606,7 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto tpa_hint = self.hidden_disabled_provider.provider_id params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))] response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html") - self.assertNotIn(response.content, tpa_hint) + self.assertNotIn(response.content.decode('utf-8'), tpa_hint) @ddt.data( ('signin_user', 'login'), @@ -650,7 +650,7 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto tpa_hint = self.hidden_disabled_provider.provider_id params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))] response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html") - self.assertNotIn(response.content, tpa_hint) + self.assertNotIn(response.content.decode('utf-8'), tpa_hint) @override_settings(FEATURES=dict(settings.FEATURES, THIRD_PARTY_AUTH_HINT='oa2-google-oauth2')) @ddt.data( From 5f2a6430feff38f8461c9a82787f579bfb939e04 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 13:56:11 -0400 Subject: [PATCH 09/24] Fix YouTube stub responses --- .../terrain/stubs/tests/test_youtube_stub.py | 8 ++++---- common/djangoapps/terrain/stubs/youtube.py | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/terrain/stubs/tests/test_youtube_stub.py b/common/djangoapps/terrain/stubs/tests/test_youtube_stub.py index 0ad715420e..ce8da2cd69 100644 --- a/common/djangoapps/terrain/stubs/tests/test_youtube_stub.py +++ b/common/djangoapps/terrain/stubs/tests/test_youtube_stub.py @@ -22,7 +22,7 @@ class StubYouTubeServiceTest(unittest.TestCase): def test_unused_url(self): response = requests.get(self.url + 'unused_url') - self.assertEqual("Unused url", response.content) + self.assertEqual(b"Unused url", response.content) @unittest.skip('Failing intermittently due to inconsistent responses from YT. See TE-871') def test_video_url(self): @@ -32,7 +32,7 @@ class StubYouTubeServiceTest(unittest.TestCase): # YouTube metadata for video `OEoXaMPEzfM` states that duration is 116. self.assertEqual( - 'callback_func({"data": {"duration": 116, "message": "I\'m youtube.", "id": "OEoXaMPEzfM"}})', + b'callback_func({"data": {"duration": 116, "message": "I\'m youtube.", "id": "OEoXaMPEzfM"}})', response.content ) @@ -46,7 +46,7 @@ class StubYouTubeServiceTest(unittest.TestCase): '', '', 'Equal transcripts' - ]), response.content + ]).encode('utf-8'), response.content ) def test_transcript_url_not_equal(self): @@ -60,7 +60,7 @@ class StubYouTubeServiceTest(unittest.TestCase): '', 'Transcripts sample, different that on server', '' - ]), response.content + ]).encode('utf-8'), response.content ) def test_transcript_not_found(self): diff --git a/common/djangoapps/terrain/stubs/youtube.py b/common/djangoapps/terrain/stubs/youtube.py index 9b3be70de5..73a8b6778a 100644 --- a/common/djangoapps/terrain/stubs/youtube.py +++ b/common/djangoapps/terrain/stubs/youtube.py @@ -65,7 +65,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): '', '', 'Equal transcripts' - ]) + ]).encode('utf-8') self.send_response( 200, content=status_message, headers={'Content-type': 'application/xml'} @@ -77,7 +77,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): '', 'Transcripts sample, different that on server', '' - ]) + ]).encode('utf-8') self.send_response( 200, content=status_message, headers={'Content-type': 'application/xml'} @@ -99,7 +99,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): # Delay the response to simulate network latency time.sleep(self.server.config.get('time_to_response', self.DEFAULT_DELAY_SEC)) if self.server.config.get('youtube_api_blocked'): - self.send_response(404, content='', headers={'Content-type': 'text/plain'}) + self.send_response(404, content=b'', headers={'Content-type': 'text/plain'}) else: # Get the response to send from YouTube. # We need to do this every time because Google sometimes sends different responses @@ -110,7 +110,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): else: self.send_response( - 404, content="Unused url", headers={'Content-type': 'text/plain'} + 404, content=b"Unused url", headers={'Content-type': 'text/plain'} ) def _send_video_response(self, youtube_id, message): @@ -134,7 +134,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): }) ) }) - response = "{cb}({data})".format(cb=callback, data=json.dumps(data)) + response = "{cb}({data})".format(cb=callback, data=json.dumps(data)).encode('utf-8') self.send_response(200, content=response, headers={'Content-type': 'text/html'}) self.log_message("Youtube: sent response {}".format(message)) @@ -158,7 +158,7 @@ class StubYouTubeHandler(StubHttpRequestHandler): "message": message, }) }) - response = "{cb}({data})".format(cb=callback, data=json.dumps(data)) + response = "{cb}({data})".format(cb=callback, data=json.dumps(data)).encode('utf-8') self.send_response(200, content=response, headers={'Content-type': 'text/html'}) self.log_message("Youtube: sent response {}".format(message)) From 8f6286a1a2e56a71523635fec901bd5ec6411baa Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 14:07:38 -0400 Subject: [PATCH 10/24] Fix a few user_api tests --- openedx/core/djangoapps/user_api/tests/test_helpers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index e62e5eb032..bb0378dab1 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -225,13 +225,13 @@ class StudentViewShimTest(TestCase): ) response = view(HttpRequest()) self.assertEqual(response.status_code, 403) - self.assertEqual(response.content, "third-party-auth") + self.assertEqual(response.content, b"third-party-auth") def test_non_json_response(self): view = self._shimmed_view(HttpResponse(content="Not a JSON dict")) response = view(HttpRequest()) self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, "Not a JSON dict") + self.assertEqual(response.content, b"Not a JSON dict") @ddt.data("redirect", "redirect_url") def test_ignore_redirect_from_json(self, redirect_key): @@ -254,7 +254,7 @@ class StudentViewShimTest(TestCase): ) response = view(HttpRequest()) self.assertEqual(response.status_code, 400) - self.assertEqual(response.content, "Error!") + self.assertEqual(response.content, b"Error!") def test_preserve_headers(self): view_response = HttpResponse() From cb64fed1af85f574ee89c126f63c687d00b7a8e3 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 12:18:14 -0400 Subject: [PATCH 11/24] BOM-489 Studio tests with byte/str mismatch. --- cms/djangoapps/contentstore/tests/test_export_git.py | 12 ++++++------ .../contentstore/tests/test_transcripts_utils.py | 4 ++-- cms/djangoapps/maintenance/tests.py | 4 ++-- common/lib/xmodule/xmodule/contentstore/mongo.py | 1 + .../xmodule/video_module/transcripts_utils.py | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_export_git.py b/cms/djangoapps/contentstore/tests/test_export_git.py index 27ae16da16..9b9c58b54b 100644 --- a/cms/djangoapps/contentstore/tests/test_export_git.py +++ b/cms/djangoapps/contentstore/tests/test_export_git.py @@ -69,7 +69,7 @@ class TestExportGit(CourseTestCase): self.assertIn( ('giturl must be defined in your ' 'course settings before you can export to git.'), - response.content + response.content.decode('utf-8') ) response = self.client.get('{}?action=push'.format(self.test_url)) @@ -77,7 +77,7 @@ class TestExportGit(CourseTestCase): self.assertIn( ('giturl must be defined in your ' 'course settings before you can export to git.'), - response.content + response.content.decode('utf-8') ) def test_course_export_failures(self): @@ -88,7 +88,7 @@ class TestExportGit(CourseTestCase): modulestore().update_item(self.course_module, self.user.id) response = self.client.get('{}?action=push'.format(self.test_url)) - self.assertIn('Export Failed:', response.content) + self.assertIn('Export Failed:', response.content.decode('utf-8')) def test_exception_translation(self): """ @@ -98,7 +98,7 @@ class TestExportGit(CourseTestCase): modulestore().update_item(self.course_module, self.user.id) response = self.client.get('{}?action=push'.format(self.test_url)) - self.assertNotIn('django.utils.functional.__proxy__', response.content) + self.assertNotIn('django.utils.functional.__proxy__', response.content.decode('utf-8')) def test_course_export_success(self): """ @@ -107,7 +107,7 @@ class TestExportGit(CourseTestCase): self.make_bare_repo_with_course('test_repo') response = self.client.get('{}?action=push'.format(self.test_url)) - self.assertIn('Export Succeeded', response.content) + self.assertIn('Export Succeeded', response.content.decode('utf-8')) def test_repo_with_dots(self): """ @@ -115,7 +115,7 @@ class TestExportGit(CourseTestCase): """ self.make_bare_repo_with_course('test.repo') response = self.client.get('{}?action=push'.format(self.test_url)) - self.assertIn('Export Succeeded', response.content) + self.assertIn('Export Succeeded', response.content.decode('utf-8')) def test_dirty_repo(self): """ diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index bba9984e76..dc0bfe373d 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -873,7 +873,7 @@ class TestGetTranscript(SharedModuleStoreTestCase): Verify that `get_transcript` function returns correct data when transcript is in content store. """ base_filename = 'video_101.srt' - self.upload_file(self.create_srt_file(self.subs_srt), self.video.location, base_filename) + self.upload_file(self.create_srt_file(self.subs_srt.encode('utf-8')), self.video.location, base_filename) self.create_transcript(subs_id, language, base_filename, youtube_id_1_0, html5_sources) content, file_name, mimetype = transcripts_utils.get_transcript( self.video, @@ -935,7 +935,7 @@ class TestGetTranscript(SharedModuleStoreTestCase): """ Verify that `get_transcript` function returns correct exception when transcript content is empty. """ - self.upload_file(self.create_srt_file(''), self.video.location, 'ur_video_101.srt') + self.upload_file(self.create_srt_file(b''), self.video.location, 'ur_video_101.srt') self.create_transcript('', 'ur', 'ur_video_101.srt') with self.assertRaises(NotFoundError) as no_content_exception: diff --git a/cms/djangoapps/maintenance/tests.py b/cms/djangoapps/maintenance/tests.py index 2b529792fc..9887caa733 100644 --- a/cms/djangoapps/maintenance/tests.py +++ b/cms/djangoapps/maintenance/tests.py @@ -289,7 +289,7 @@ class TestAnnouncementsViews(MaintenanceViewTestCase): """ url = reverse("maintenance:announcement_index") response = self.client.get(url) - self.assertIn('
', response.content) + self.assertIn('
', response.content.decode('utf-8')) def test_create(self): """ @@ -308,7 +308,7 @@ class TestAnnouncementsViews(MaintenanceViewTestCase): announcement.save() url = reverse("maintenance:announcement_edit", kwargs={"pk": announcement.pk}) response = self.client.get(url) - self.assertIn('
', response.content) + self.assertIn('
', response.content.decode('utf-8')) self.client.post(url, {"content": "Test Edit Announcement", "active": True}) announcement = Announcement.objects.get(pk=announcement.pk) self.assertEquals(announcement.content, "Test Edit Announcement") diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 7ab6c42d2d..649e07bc7a 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -99,6 +99,7 @@ class MongoContentStore(ContentStore): import_path=content.import_path, # getattr b/c caching may mean some pickled instances don't have attr locked=getattr(content, 'locked', False)) as fp: + # It seems that this code thought that only some specific object would have the `__iter__` attribute # but the bytes object in python 3 has one and should not use the chunking logic. if hasattr(content.data, '__iter__') and not isinstance(content.data, six.binary_type): diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 5a513bca58..5556a7448b 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -126,7 +126,7 @@ def save_subs_to_store(subs, subs_id, item, language='en'): Returns: location of saved subtitles. """ - filedata = json.dumps(subs, indent=2) + filedata = json.dumps(subs, indent=2).encode('utf-8') filename = subs_filename(subs_id, language) return save_to_store(filedata, filename, 'application/json', item.location) From 7281fed719729a2fd407a997679d70e2eaf61bee Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 14:24:37 -0400 Subject: [PATCH 12/24] Test JSON data without relying on arbitrary ordering. BOM-735 --- .../zendesk_proxy/tests/test_v1_views.py | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/zendesk_proxy/tests/test_v1_views.py b/openedx/core/djangoapps/zendesk_proxy/tests/test_v1_views.py index d66db7acd3..d7f5525a45 100644 --- a/openedx/core/djangoapps/zendesk_proxy/tests/test_v1_views.py +++ b/openedx/core/djangoapps/zendesk_proxy/tests/test_v1_views.py @@ -1,16 +1,19 @@ """Tests for zendesk_proxy views.""" + from __future__ import absolute_import -import json + from copy import deepcopy +import json import ddt from django.urls import reverse from django.test.utils import override_settings from mock import MagicMock, patch +import six +from six.moves import range from openedx.core.djangoapps.zendesk_proxy.v1.views import ZendeskProxyThrottle from openedx.core.lib.api.test_utils import ApiTestCase -from six.moves import range @ddt.ddt @@ -53,14 +56,30 @@ class ZendeskProxyTestCase(ApiTestCase): self.assertHttpCreated(response) (mock_args, mock_kwargs) = mock_post.call_args self.assertEqual(mock_args, ('https://www.superrealurlsthataredefinitelynotfake.com/api/v2/tickets.json',)) + six.assertCountEqual(self, mock_kwargs.keys(), ['headers', 'data']) self.assertEqual( - mock_kwargs, + mock_kwargs['headers'], { - 'headers': { - 'content-type': 'application/json', - 'Authorization': 'Bearer abcdefghijklmnopqrstuvwxyz1234567890' + 'content-type': 'application/json', + 'Authorization': 'Bearer abcdefghijklmnopqrstuvwxyz1234567890' + } + ) + self.assertEqual( + json.loads(mock_kwargs['data']), + { + 'ticket': { + 'comment': { + 'body': "Help! I'm trapped in a unit test factory and I can't get out!", + 'uploads': None, + }, + 'custom_fields': [{'id': '001', 'value': 'demo-course'}], + 'requester': { + 'email': 'JohnQStudent@example.com', + 'name': 'John Q. Student', + }, + 'subject': 'Python Unit Test Help Request', + 'tags': ['python_unit_test'], }, - 'data': '{"ticket": {"comment": {"body": "Help! I\'m trapped in a unit test factory and I can\'t get out!", "uploads": null}, "tags": ["python_unit_test"], "subject": "Python Unit Test Help Request", "custom_fields": [{"id": "001", "value": "demo-course"}], "requester": {"name": "John Q. Student", "email": "JohnQStudent@example.com"}}}' # pylint: disable=line-too-long } ) From 07ab555d05721e7defa777ff99c6651f215ae37e Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 14:26:41 -0400 Subject: [PATCH 13/24] Fix most common/djangoapps/util tests --- common/djangoapps/util/tests/test_file.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/util/tests/test_file.py b/common/djangoapps/util/tests/test_file.py index bca74aacee..57f89703d2 100644 --- a/common/djangoapps/util/tests/test_file.py +++ b/common/djangoapps/util/tests/test_file.py @@ -85,7 +85,7 @@ class StoreUploadedFileTestCase(TestCase): def setUp(self): super(StoreUploadedFileTestCase, self).setUp() self.request = Mock(spec=HttpRequest) - self.file_content = "test file content" + self.file_content = b"test file content" self.stored_file_name = None self.file_storage = None self.default_max_size = 2000000 @@ -149,7 +149,7 @@ class StoreUploadedFileTestCase(TestCase): def exception_validator(storage, filename): """ Validation test function that throws an exception """ self.assertEqual("error_file.csv", os.path.basename(filename)) - with storage.open(filename, 'rU') as f: + with storage.open(filename, 'rb') as f: self.assertEqual(self.file_content, f.read()) store_file_data(storage, filename) raise FileValidationException("validation failed") @@ -190,7 +190,7 @@ class StoreUploadedFileTestCase(TestCase): """ Tests uploading a file with upper case extension. Verifies that the stored file contents are correct. """ - file_content = "uppercase" + file_content = b"uppercase" self.request.FILES = {"uploaded_file": SimpleUploadedFile("tempfile.CSV", file_content)} file_storage, stored_file_name = store_uploaded_file( self.request, "uploaded_file", [".gif", ".csv"], "second_stored_file", self.default_max_size @@ -202,7 +202,7 @@ class StoreUploadedFileTestCase(TestCase): Test that the file storage method will create a unique filename if the file already exists. """ requested_file_name = "nonunique_store" - file_content = "copy" + file_content = b"copy" self.request.FILES = {"nonunique_file": SimpleUploadedFile("nonunique.txt", file_content)} _, first_stored_file_name = store_uploaded_file( @@ -220,7 +220,7 @@ class StoreUploadedFileTestCase(TestCase): def _verify_successful_upload(self, storage, file_name, expected_content): """ Helper method that checks that the stored version of the uploaded file has the correct content """ self.assertTrue(storage.exists(file_name)) - with storage.open(file_name, 'r') as f: + with storage.open(file_name, 'rb') as f: self.assertEqual(expected_content, f.read()) From 1c6ed4026e8d4b4a17d7ae40b82dd47625c4cca4 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 14:34:16 -0400 Subject: [PATCH 14/24] Test JSON data without relying on arbitrary ordering. BOM-734 --- .../zendesk_proxy/tests/test_v0_views.py | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/zendesk_proxy/tests/test_v0_views.py b/openedx/core/djangoapps/zendesk_proxy/tests/test_v0_views.py index 9da1919020..69a648e084 100644 --- a/openedx/core/djangoapps/zendesk_proxy/tests/test_v0_views.py +++ b/openedx/core/djangoapps/zendesk_proxy/tests/test_v0_views.py @@ -1,16 +1,18 @@ """Tests for zendesk_proxy views.""" from __future__ import absolute_import -import json + from copy import deepcopy +import json import ddt from django.urls import reverse from django.test.utils import override_settings from mock import MagicMock, patch +import six +from six.moves import range from openedx.core.djangoapps.zendesk_proxy.v0.views import ZENDESK_REQUESTS_PER_HOUR from openedx.core.lib.api.test_utils import ApiTestCase -from six.moves import range @ddt.ddt @@ -45,14 +47,30 @@ class ZendeskProxyTestCase(ApiTestCase): self.assertHttpCreated(response) (mock_args, mock_kwargs) = mock_post.call_args self.assertEqual(mock_args, ('https://www.superrealurlsthataredefinitelynotfake.com/api/v2/tickets.json',)) + six.assertCountEqual(self, mock_kwargs.keys(), ['headers', 'data']) self.assertEqual( - mock_kwargs, + mock_kwargs['headers'], { - 'headers': { - 'content-type': 'application/json', - 'Authorization': 'Bearer abcdefghijklmnopqrstuvwxyz1234567890' + 'content-type': 'application/json', + 'Authorization': 'Bearer abcdefghijklmnopqrstuvwxyz1234567890' + } + ) + self.assertEqual( + json.loads(mock_kwargs['data']), + { + 'ticket': { + 'comment': { + 'body': "Help! I'm trapped in a unit test factory and I can't get out!", + 'uploads': None, + }, + 'custom_fields': None, + 'requester': { + 'email': 'JohnQStudent@example.com', + 'name': 'John Q. Student', + }, + 'subject': 'Python Unit Test Help Request', + 'tags': ['python_unit_test'], }, - 'data': '{"ticket": {"comment": {"body": "Help! I\'m trapped in a unit test factory and I can\'t get out!", "uploads": null}, "tags": ["python_unit_test"], "subject": "Python Unit Test Help Request", "custom_fields": null, "requester": {"name": "John Q. Student", "email": "JohnQStudent@example.com"}}}' # pylint: disable=line-too-long } ) From 456ab6898636cc770dd6faf105bb176f16aa5f79 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 14:49:03 -0400 Subject: [PATCH 15/24] Check for ascii with an explicit ascii encode. BOM-728 --- openedx/core/djangoapps/credit/signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/credit/signature.py b/openedx/core/djangoapps/credit/signature.py index 47aebfc468..371d4bf941 100644 --- a/openedx/core/djangoapps/credit/signature.py +++ b/openedx/core/djangoapps/credit/signature.py @@ -36,7 +36,7 @@ def get_shared_secret_key(provider_id): if isinstance(secret, six.text_type): try: - secret = str(secret) + secret.encode('ascii') # pylint: disable=pointless-statement except UnicodeEncodeError: secret = None log.error(u'Shared secret key for credit provider "%s" contains non-ASCII unicode.', provider_id) From de0eafa0baf5142ff257e492a79026d228cf8db9 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 14:57:10 -0400 Subject: [PATCH 16/24] BOM-618 Decode test client content. The django test client returns bytes, and many of our tests start using it like a string. This was fine in python 2 but not in python3. --- .../discussion/django_comment_client/tests/test_middleware.py | 4 ++-- lms/djangoapps/instructor/tests/test_tools.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py b/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py index 49f2d950fc..6e23ef2881 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py @@ -30,7 +30,7 @@ class AjaxExceptionTestCase(TestCase): self.assertEqual(self.exception1.status_code, response1.status_code) self.assertEqual( {"errors": json.loads(text_type(self.exception1))}, - json.loads(response1.content) + json.loads(response1.content.decode('utf-8')) ) response2 = self.a.process_exception(self.request1, self.exception2) @@ -38,7 +38,7 @@ class AjaxExceptionTestCase(TestCase): self.assertEqual(self.exception2.status_code, response2.status_code) self.assertEqual( {"errors": [text_type(self.exception2)]}, - json.loads(response2.content) + json.loads(response2.content.decode('utf-8')) ) self.assertIsNone(self.a.process_exception(self.request1, self.exception0)) diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 9ab7ac689d..1072f2acea 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -33,7 +33,7 @@ class TestDashboardError(unittest.TestCase): """ def test_response(self): error = tools.DashboardError(u'Oh noes!') - response = json.loads(error.response().content) + response = json.loads(error.response().content.decode('utf-8')) self.assertEqual(response, {'error': 'Oh noes!'}) @@ -50,7 +50,7 @@ class TestHandleDashboardError(unittest.TestCase): """ raise tools.DashboardError("Oh noes!") - response = json.loads(view(None, None).content) + response = json.loads(view(None, None).content.decode('utf-8')) self.assertEqual(response, {'error': 'Oh noes!'}) def test_no_error(self): From 90838d27aa7414117c35b4c83ca10500ccc54860 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 15:14:07 -0400 Subject: [PATCH 17/24] BOM-618 update dependency to fix test mixins. --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 750489e21c..bc0ed171c6 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -109,7 +109,7 @@ edx-drf-extensions==2.4.0 edx-enterprise==1.10.1 edx-i18n-tools==0.4.8 edx-milestones==0.2.3 -edx-oauth2-provider==1.3.0 +edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 9755955774..03150c3246 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -133,7 +133,7 @@ edx-enterprise==1.10.1 edx-i18n-tools==0.4.8 edx-lint==1.3.0 edx-milestones==0.2.3 -edx-oauth2-provider==1.3.0 +edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 71186c536e..d18765c611 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -130,7 +130,7 @@ edx-enterprise==1.10.1 edx-i18n-tools==0.4.8 edx-lint==1.3.0 edx-milestones==0.2.3 -edx-oauth2-provider==1.3.0 +edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 From 6408f96e1832d1729f2d88bbc3d9ab5c2129aa90 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 16:13:40 -0400 Subject: [PATCH 18/24] Use explicit values in test assertions. BOM-683 Python 3 changed how rounding is performed. In Python 2, .625 rounded to .63 (as our grading code rounds it, so the test passed). In Python 3, .625 rounds to .62. This fixes the test by avoiding round() to calculate the expected value, and instead simply using the value we expect. --- .../tests/test_submitting_problems.py | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 38d502766f..6b690fa8da 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -1187,10 +1187,11 @@ class TestConditionalContent(TestSubmittingProblems): self.assertEqual(self.score_for_hw('homework2'), [1.0, 2.0]) self.assertEqual(self.earned_hw_scores(), [1.0, 3.0]) - # Grade percent is .63. Here is the calculation - homework_1_score = 1.0 / 2 - homework_2_score = (1.0 + 2.0) / 4 - self.check_grade_percent(round((homework_1_score + homework_2_score) / 2, 2)) + # Grade percent is .63. Here is the calculation: + # homework_1_score = 1.0 / 2 + # homework_2_score = (1.0 + 2.0) / 4 + # round((homework_1_score + homework_2_score) / 2) == .63 + self.check_grade_percent(.63) def test_split_different_problems_group_1(self): """ @@ -1205,10 +1206,11 @@ class TestConditionalContent(TestSubmittingProblems): self.assertEqual(self.score_for_hw('homework2'), [1.0]) self.assertEqual(self.earned_hw_scores(), [1.0, 1.0]) - # Grade percent is .75. Here is the calculation - homework_1_score = 1.0 / 2 - homework_2_score = 1.0 / 1 - self.check_grade_percent(round((homework_1_score + homework_2_score) / 2, 2)) + # Grade percent is .75. Here is the calculation: + # homework_1_score = 1.0 / 2 + # homework_2_score = 1.0 / 1 + # round((homework_1_score + homework_2_score) / 2) == .75 + self.check_grade_percent(.75) def split_one_group_no_problems_setup(self, user_partition_group): """ @@ -1238,10 +1240,11 @@ class TestConditionalContent(TestSubmittingProblems): self.assertEqual(self.score_for_hw('homework2'), []) self.assertEqual(self.earned_hw_scores(), [1.0]) - # Grade percent is .25. Here is the calculation. - homework_1_score = 1.0 / 2 - homework_2_score = 0.0 - self.check_grade_percent(round((homework_1_score + homework_2_score) / 2, 2)) + # Grade percent is .25. Here is the calculation: + # homework_1_score = 1.0 / 2 + # homework_2_score = 0.0 + # round((homework_1_score + homework_2_score) / 2) == .25 + self.check_grade_percent(.25) def test_split_one_group_no_problems_group_1(self): """ @@ -1256,6 +1259,7 @@ class TestConditionalContent(TestSubmittingProblems): self.assertEqual(self.earned_hw_scores(), [1.0, 1.0]) # Grade percent is .75. Here is the calculation. - homework_1_score = 1.0 / 2 - homework_2_score = 1.0 / 1 - self.check_grade_percent(round((homework_1_score + homework_2_score) / 2, 2)) + # homework_1_score = 1.0 / 2 + # homework_2_score = 1.0 / 1 + # round((homework_1_score + homework_2_score) / 2) == .75 + self.check_grade_percent(.75) From 3427c1abf36f274f24efeee33d6ad5b034d62c3f Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 16:26:55 -0400 Subject: [PATCH 19/24] BOM-618 Pass unicode to json.loads --- lms/djangoapps/courseware/tests/test_video_handlers.py | 2 +- lms/djangoapps/courseware/tests/test_word_cloud.py | 2 +- lms/djangoapps/verify_student/tests/test_views.py | 4 ++-- lms/djangoapps/verify_student/views.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 6853d078b8..249ecfb54c 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -973,7 +973,7 @@ class TestStudioTranscriptTranslationPostDispatch(TestVideo): request = Request.blank('/translation', POST=post_data) response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') self.assertEqual(response.status, '201 Created') - response = json.loads(response.body) + response = json.loads(response.text) self.assertTrue(response["language_code"], "uk") self.assertDictEqual(self.item_descriptor.transcripts, {}) self.assertTrue(edxval_api.get_video_transcript_data(video_id=response["edx_video_id"], language_code="uk")) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 5e91234cdf..709895568a 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -243,7 +243,7 @@ class TestWordCloud(BaseTestXmodule): for user in self.users: self.assertDictEqual( - json.loads(responses[user.username].content), + json.loads(responses[user.username].content.decode('utf-8')), { 'status': 'fail', 'error': 'Unknown Command!' diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index ca70b38786..4e51ad86af 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -1298,7 +1298,7 @@ class TestCheckoutWithEcommerceService(ModuleStoreTestCase): self.assertTrue(mock_audit_log.called) # Check the api call - self.assertEqual(json.loads(httpretty.last_request().body), { + self.assertEqual(json.loads(httpretty.last_request().body.decode('utf-8')), { 'products': [{'sku': 'test-sku'}], 'checkout': True, 'payment_processor_name': 'test-processor', @@ -1845,7 +1845,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): self.assertEqual(attempt.status, u'denied') self.assertEqual(attempt.error_code, u'Your photo doesn\'t meet standards.') self.assertEqual(attempt.error_msg, u'[{"photoIdReasons": ["Not provided"]}]') - self.assertEquals(response.content, 'OK!') + self.assertEquals(response.content.decode('utf-8'), 'OK!') self.assertEqual(len(mail.outbox), 1) @patch( diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 9dec48b2f8..28cdf2e8ab 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -1113,7 +1113,7 @@ def results_callback(request): body = request.body try: - body_dict = json.loads(body) + body_dict = json.loads(body.decode('utf-8')) except ValueError: log.exception(u"Invalid JSON received from Software Secure:\n\n{}\n".format(body)) return HttpResponseBadRequest(u"Invalid JSON. Received:\n\n{}".format(body)) From 93142ebd2de62479deeb8815eb14adc1906117bd Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 16:27:11 -0400 Subject: [PATCH 20/24] Update the ssencrypt logic for python3. We updated the underlying methods to always be passing unicode strings aronud and only encode them to bytes when we need to hash them. --- lms/djangoapps/verify_student/ssencrypt.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/verify_student/ssencrypt.py b/lms/djangoapps/verify_student/ssencrypt.py index f6fce78bc2..0db7a231cd 100644 --- a/lms/djangoapps/verify_student/ssencrypt.py +++ b/lms/djangoapps/verify_student/ssencrypt.py @@ -175,8 +175,8 @@ def generate_signed_message(method, headers_dict, body_dict, access_key, secret_ message = signing_format_message(method, headers_dict, body_dict) # hmac needs a byte string for it's starting key, can't be unicode. - hashed = hmac.new(secret_key.encode('utf-8'), message, sha256) - signature = binascii.b2a_base64(hashed.digest()).rstrip('\n') + hashed = hmac.new(secret_key.encode('utf-8'), message.encode('utf-8'), sha256) + signature = binascii.b2a_base64(hashed.digest()).rstrip(b'\n') authorization_header = u"SSI {}:{}".format(access_key, signature) message += '\n' @@ -223,12 +223,12 @@ def body_string(body_dict, prefix=""): if isinstance(arr, dict): body_list.append(body_string(arr, u"{}.{}.".format(key, i))) else: - body_list.append(u"{}.{}:{}\n".format(key, i, arr).encode('utf-8')) + body_list.append(u"{}.{}:{}\n".format(key, i, arr)) elif isinstance(value, dict): body_list.append(body_string(value, key + ":")) else: if value is None: value = "null" - body_list.append(u"{}{}:{}\n".format(prefix, key, value).encode('utf-8')) + body_list.append(u"{}{}:{}\n".format(prefix, key, value)) return "".join(body_list) # Note that trailing \n's are important From 83eb9e7f1a4931e635c0a3615fdfeac2b62a9dc0 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 16:37:52 -0400 Subject: [PATCH 21/24] Fix auth_exchange tests BOM-648 --- .../core/djangoapps/auth_exchange/tests/test_forms.py | 2 +- .../core/djangoapps/auth_exchange/tests/test_views.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/auth_exchange/tests/test_forms.py b/openedx/core/djangoapps/auth_exchange/tests/test_forms.py index 3aafbe989e..775d8ac989 100644 --- a/openedx/core/djangoapps/auth_exchange/tests/test_forms.py +++ b/openedx/core/djangoapps/auth_exchange/tests/test_forms.py @@ -50,7 +50,7 @@ class AccessTokenExchangeFormTest(AccessTokenExchangeTestMixin): self.assertTrue(form.is_valid()) self.assertEqual(form.cleaned_data["user"], self.user) self.assertEqual(form.cleaned_data["client"], self.oauth_client) - self.assertEqual(scope.to_names(form.cleaned_data["scope"]), expected_scopes) + self.assertEqual(set(scope.to_names(form.cleaned_data["scope"])), set(expected_scopes)) # This is necessary because cms does not implement third party auth diff --git a/openedx/core/djangoapps/auth_exchange/tests/test_views.py b/openedx/core/djangoapps/auth_exchange/tests/test_views.py index 15692391a7..2ad2829111 100644 --- a/openedx/core/djangoapps/auth_exchange/tests/test_views.py +++ b/openedx/core/djangoapps/auth_exchange/tests/test_views.py @@ -63,11 +63,16 @@ class AccessTokenExchangeViewTest(AccessTokenExchangeTestMixin): timedelta(seconds=int(content["expires_in"])), provider.constants.EXPIRE_DELTA_PUBLIC ) - self.assertEqual(content["scope"], ' '.join(expected_scopes)) + actual_scopes = content["scope"] + if actual_scopes: + actual_scopes = actual_scopes.split(' ') + else: + actual_scopes = [] + self.assertEqual(set(actual_scopes), set(expected_scopes)) token = self.oauth2_adapter.get_access_token(token_string=content["access_token"]) self.assertEqual(token.user, self.user) self.assertEqual(self.oauth2_adapter.get_client_for_token(token), self.oauth_client) - self.assertEqual(self.oauth2_adapter.get_token_scope_names(token), expected_scopes) + self.assertEqual(set(self.oauth2_adapter.get_token_scope_names(token)), set(expected_scopes)) def test_single_access_token(self): def extract_token(response): @@ -184,7 +189,7 @@ class TestLoginWithAccessTokenView(TestCase): Calls the login_with_access_token endpoint and verifies the response given the expected values. """ url = reverse("login_with_access_token") - response = self.client.post(url, HTTP_AUTHORIZATION=b"Bearer {0}".format(access_token)) + response = self.client.post(url, HTTP_AUTHORIZATION=u"Bearer {0}".format(access_token).encode('utf-8')) self.assertEqual(response.status_code, expected_status_code) if expected_cookie_name: self.assertIn(expected_cookie_name, response.cookies) From f08a2ed88da0836cba2dd0717362b7161c19f11a Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 16:55:00 -0400 Subject: [PATCH 22/24] Fix text representation of VerifiedTrackCohortedCourse BOM-733 --- openedx/core/djangoapps/verified_track_content/models.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/verified_track_content/models.py b/openedx/core/djangoapps/verified_track_content/models.py index 9b17dd9f60..3a80b70485 100644 --- a/openedx/core/djangoapps/verified_track_content/models.py +++ b/openedx/core/djangoapps/verified_track_content/models.py @@ -10,6 +10,7 @@ from config_models.models import ConfigurationModel from django.db import models from django.db.models.signals import post_save, pre_save from django.dispatch import receiver +from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy from edx_django_utils.cache import RequestCache from opaque_keys.edx.django.models import CourseKeyField @@ -92,6 +93,7 @@ def pre_save_callback(sender, instance, **kwargs): # pylint: disable=unused-arg instance._old_mode = None # pylint: disable=protected-access +@python_2_unicode_compatible class VerifiedTrackCohortedCourse(models.Model): """ Tracks which courses have verified track auto-cohorting enabled. @@ -109,7 +111,7 @@ class VerifiedTrackCohortedCourse(models.Model): CACHE_NAMESPACE = u"verified_track_content.VerifiedTrackCohortedCourse.cache." - def __unicode__(self): + def __str__(self): return u"Course: {}, enabled: {}".format(six.text_type(self.course_key), self.enabled) @classmethod From 10d9bb22b5613928ef542f7e5d08aeef1054967e Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 17:02:38 -0400 Subject: [PATCH 23/24] Fix oauth_dispatch tests BOM-732 --- openedx/core/djangoapps/oauth_dispatch/models.py | 10 +++++++--- .../core/djangoapps/oauth_dispatch/tests/test_views.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/oauth_dispatch/models.py b/openedx/core/djangoapps/oauth_dispatch/models.py index 42a58e2918..fb383ee4cf 100644 --- a/openedx/core/djangoapps/oauth_dispatch/models.py +++ b/openedx/core/djangoapps/oauth_dispatch/models.py @@ -8,6 +8,7 @@ from datetime import datetime import six from django.db import models +from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from django_mysql.models import ListCharField from oauth2_provider.settings import oauth2_settings @@ -19,6 +20,7 @@ from openedx.core.djangolib.markup import HTML from openedx.core.lib.request_utils import get_request_or_stub +@python_2_unicode_compatible class RestrictedApplication(models.Model): """ This model lists which django-oauth-toolkit Applications are considered 'restricted' @@ -35,7 +37,7 @@ class RestrictedApplication(models.Model): class Meta: app_label = 'oauth_dispatch' - def __unicode__(self): + def __str__(self): """ Return a unicode representation of this object """ @@ -59,6 +61,7 @@ class RestrictedApplication(models.Model): return access_token.expires == datetime(1970, 1, 1, tzinfo=utc) +@python_2_unicode_compatible class ApplicationAccess(models.Model): """ Specifies access control information for the associated Application. @@ -81,7 +84,7 @@ class ApplicationAccess(models.Model): def get_scopes(cls, application): return cls.objects.get(application=application).scopes - def __unicode__(self): + def __str__(self): """ Return a unicode representation of this object. """ @@ -91,6 +94,7 @@ class ApplicationAccess(models.Model): ) +@python_2_unicode_compatible class ApplicationOrganization(models.Model): """ Associates a DOT Application to an Organization. @@ -129,7 +133,7 @@ class ApplicationOrganization(models.Model): queryset = queryset.filter(relation_type=relation_type) return [r.organization.name for r in queryset] - def __unicode__(self): + def __str__(self): """ Return a unicode representation of this object. """ diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py index 7722872aad..25e4585e28 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py @@ -63,7 +63,7 @@ class AccessTokenLoginMixin(object): return self.client.post( self.login_with_access_token_url, - HTTP_AUTHORIZATION=b"Bearer {0}".format(access_token if access_token else self.access_token) + HTTP_AUTHORIZATION=u"Bearer {0}".format(access_token if access_token else self.access_token).encode('utf-8') ) def _assert_access_token_is_valid(self, access_token=None): From bab6b87eb8407a7370b36a5e9820676aa46b29a2 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 17:28:12 -0400 Subject: [PATCH 24/24] Fix pylint error. --- openedx/core/djangoapps/credit/signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/credit/signature.py b/openedx/core/djangoapps/credit/signature.py index 371d4bf941..928968fff2 100644 --- a/openedx/core/djangoapps/credit/signature.py +++ b/openedx/core/djangoapps/credit/signature.py @@ -36,7 +36,7 @@ def get_shared_secret_key(provider_id): if isinstance(secret, six.text_type): try: - secret.encode('ascii') # pylint: disable=pointless-statement + secret.encode('ascii') except UnicodeEncodeError: secret = None log.error(u'Shared secret key for credit provider "%s" contains non-ASCII unicode.', provider_id)