From dab59635a7eb7244bf4817a6da4ebcfc0cf7ba7f Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 25 Sep 2019 12:01:27 -0400 Subject: [PATCH] Fix some failing xmodule tests (#21775) --- common/lib/xmodule/xmodule/lti_2_util.py | 6 +-- common/lib/xmodule/xmodule/lti_module.py | 4 +- .../lib/xmodule/xmodule/tests/test_content.py | 4 +- .../xmodule/tests/test_import_static.py | 8 ++-- .../xmodule/xmodule/tests/test_lti20_unit.py | 28 ++++++------- .../xmodule/xmodule/tests/test_lti_unit.py | 41 +++++++++++-------- 6 files changed, 49 insertions(+), 42 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_2_util.py b/common/lib/xmodule/xmodule/lti_2_util.py index adf5708fdd..0e15fd1f8b 100644 --- a/common/lib/xmodule/xmodule/lti_2_util.py +++ b/common/lib/xmodule/xmodule/lti_2_util.py @@ -174,12 +174,12 @@ class LTI20ModuleMixin(object): } self.system.rebind_noauth_module_to_user(self, real_user) if self.module_score is None: # In this case, no score has been ever set - return Response(json.dumps(base_json_obj), content_type=LTI_2_0_JSON_CONTENT_TYPE) + return Response(json.dumps(base_json_obj).encode('utf-8'), content_type=LTI_2_0_JSON_CONTENT_TYPE) # Fall through to returning grade and comment base_json_obj['resultScore'] = round(self.module_score, 2) base_json_obj['comment'] = self.score_comment - return Response(json.dumps(base_json_obj), content_type=LTI_2_0_JSON_CONTENT_TYPE) + return Response(json.dumps(base_json_obj).encode('utf-8'), content_type=LTI_2_0_JSON_CONTENT_TYPE) def _lti_2_0_result_del_handler(self, request, real_user): # pylint: disable=unused-argument """ @@ -211,7 +211,7 @@ class LTI20ModuleMixin(object): webob.response: response to this request. status 200 if success. 404 if body of PUT request is malformed """ try: - (score, comment) = self.parse_lti_2_0_result_json(request.body) + (score, comment) = self.parse_lti_2_0_result_json(request.body.decode('utf-8')) except LTIError: return Response(status=404) # have to do 404 due to spec, but 400 is better, with error msg in body diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 302c65413a..73f7199069 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -797,7 +797,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} lti_spec_namespace = "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0" namespaces = {'def': lti_spec_namespace} - data = body.strip().encode('utf-8') + data = body.strip() parser = etree.XMLParser(ns_clean=True, recover=True, encoding='utf-8') root = etree.fromstring(data, parser=parser) @@ -836,7 +836,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} sha1 = hashlib.sha1() sha1.update(request.body) - oauth_body_hash = base64.b64encode(sha1.digest()) + oauth_body_hash = base64.b64encode(sha1.digest()).decode('utf-8') oauth_params = signature.collect_parameters(headers=headers, exclude_oauth_signature=False) oauth_headers = dict(oauth_params) oauth_signature = oauth_headers.pop('oauth_signature') diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index 769cddc99c..99cc78d28e 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -140,7 +140,7 @@ class ContentTest(unittest.TestCase): content_store = ContentStore() content = Content(AssetLocator(CourseLocator(u'mitX', u'800', u'ignore_run'), u'asset', "monsters.jpg"), "image/jpeg") - content.data = 'mock data' + content.data = b'mock data' content_store.generate_thumbnail(content) self.assertTrue(image_class_mock.open.called, "Image.open not called") self.assertTrue(mock_image.close.called, "mock_image.close not called") @@ -153,7 +153,7 @@ class ContentTest(unittest.TestCase): thumbnail_filename = u'test.svg' content = Content(AssetLocator(CourseLocator(u'mitX', u'800', u'ignore_run'), u'asset', u'test.svg'), 'image/svg+xml') - content.data = 'mock svg file' + content.data = b'mock svg file' (thumbnail_content, thumbnail_file_location) = content_store.generate_thumbnail(content) self.assertEqual(thumbnail_content.data.read(), b'mock svg file') self.assertEqual( diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py index 2e4a87d438..728c1ca545 100644 --- a/common/lib/xmodule/xmodule/tests/test_import_static.py +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -19,7 +19,9 @@ from xmodule.tests import DATA_DIR class IgnoredFilesTestCase(unittest.TestCase): - "Tests for ignored files" + """ + Tests for ignored files + """ course_dir = DATA_DIR / "course_ignore" dict_list = [DOT_FILES_DICT, TILDA_FILES_DICT] @@ -47,8 +49,8 @@ class IgnoredFilesTestCase(unittest.TestCase): name_val = {sc.name: sc.data for sc in saved_static_content} self.assertIn("example.txt", name_val) self.assertIn(".example.txt", name_val) - self.assertIn("GREEN", name_val["example.txt"]) - self.assertIn("BLUE", name_val[".example.txt"]) + self.assertIn(b"GREEN", name_val["example.txt"]) + self.assertIn(b"BLUE", name_val[".example.txt"]) self.assertNotIn("._example.txt", name_val) self.assertNotIn(".DS_Store", name_val) self.assertNotIn("example.txt~", name_val) diff --git a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py index d16254e5a0..98c99e531f 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py @@ -112,7 +112,7 @@ class LTI20RESTResultServiceTest(LogicTest): fit the form user/ """ for ginput, expected in self.GOOD_DISPATCH_INPUTS: - self.assertEquals(self.xmodule.parse_lti_2_0_handler_suffix(ginput), expected) + self.assertEqual(self.xmodule.parse_lti_2_0_handler_suffix(ginput), expected) BAD_JSON_INPUTS = [ # (bad inputs, error message expected) @@ -248,7 +248,7 @@ class LTI20RESTResultServiceTest(LogicTest): self.xmodule.score_comment = COMMENT mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT_LIKE_DELETE) # Now call the handler - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") # Now assert there's no score self.assertEqual(response.status_code, 200) self.assertIsNone(self.xmodule.module_score) @@ -269,9 +269,9 @@ class LTI20RESTResultServiceTest(LogicTest): COMMENT = u"ಠ益ಠ" # pylint: disable=invalid-name self.xmodule.module_score = SCORE self.xmodule.score_comment = COMMENT - mock_request = self.get_signed_lti20_mock_request("", method=u'DELETE') + mock_request = self.get_signed_lti20_mock_request(b"", method=u'DELETE') # Now call the handler - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") # Now assert there's no score self.assertEqual(response.status_code, 200) self.assertIsNone(self.xmodule.module_score) @@ -290,7 +290,7 @@ class LTI20RESTResultServiceTest(LogicTest): self.setup_system_xmodule_mocks_for_lti20_request_test() mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) # Now call the handler - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") # Now assert self.assertEqual(response.status_code, 200) self.assertEqual(self.xmodule.module_score, 0.1) @@ -307,9 +307,9 @@ class LTI20RESTResultServiceTest(LogicTest): The happy path for LTI 2.0 GET when there's no score """ self.setup_system_xmodule_mocks_for_lti20_request_test() - mock_request = self.get_signed_lti20_mock_request("", method=u'GET') + mock_request = self.get_signed_lti20_mock_request(b"", method=u'GET') # Now call the handler - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") # Now assert self.assertEqual(response.status_code, 200) self.assertEqual(response.json, {"@context": "http://purl.imsglobal.org/ctx/lis/v2/Result", @@ -324,9 +324,9 @@ class LTI20RESTResultServiceTest(LogicTest): COMMENT = u"ಠ益ಠ" # pylint: disable=invalid-name self.xmodule.module_score = SCORE self.xmodule.score_comment = COMMENT - mock_request = self.get_signed_lti20_mock_request("", method=u'GET') + mock_request = self.get_signed_lti20_mock_request(b"", method=u'GET') # Now call the handler - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") # Now assert self.assertEqual(response.status_code, 200) self.assertEqual(response.json, {"@context": "http://purl.imsglobal.org/ctx/lis/v2/Result", @@ -344,7 +344,7 @@ class LTI20RESTResultServiceTest(LogicTest): mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) for bad_method in self.UNSUPPORTED_HTTP_METHODS: mock_request.method = bad_method - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") self.assertEqual(response.status_code, 404) def test_lti20_request_handler_bad_headers(self): @@ -354,7 +354,7 @@ class LTI20RESTResultServiceTest(LogicTest): self.setup_system_xmodule_mocks_for_lti20_request_test() self.xmodule.verify_lti_2_0_result_rest_headers = Mock(side_effect=LTIError()) mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") self.assertEqual(response.status_code, 401) def test_lti20_request_handler_bad_dispatch_user(self): @@ -373,7 +373,7 @@ class LTI20RESTResultServiceTest(LogicTest): self.setup_system_xmodule_mocks_for_lti20_request_test() self.xmodule.parse_lti_2_0_result_json = Mock(side_effect=LTIError()) mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") self.assertEqual(response.status_code, 404) def test_lti20_request_handler_bad_user(self): @@ -383,7 +383,7 @@ class LTI20RESTResultServiceTest(LogicTest): self.setup_system_xmodule_mocks_for_lti20_request_test() self.system.get_real_user = Mock(return_value=None) mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") self.assertEqual(response.status_code, 404) def test_lti20_request_handler_grade_past_due(self): @@ -394,5 +394,5 @@ class LTI20RESTResultServiceTest(LogicTest): self.xmodule.due = datetime.datetime.now(UTC) self.xmodule.accept_grades_past_due = False mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) - response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, u"user/abcd") self.assertEqual(response.status_code, 404) diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 861d183096..a82dd61f33 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -7,6 +7,7 @@ import datetime import textwrap from copy import copy +import six import six.moves.urllib.error import six.moves.urllib.parse import six.moves.urllib.request @@ -30,7 +31,7 @@ class LTIModuleTest(LogicTest): def setUp(self): super(LTIModuleTest, self).setUp() self.environ = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'POST'} - self.request_body_xml_template = textwrap.dedent(""" + self.request_body_xml_template = textwrap.dedent(u""" @@ -87,7 +88,7 @@ class LTIModuleTest(LogicTest): data = copy(self.defaults) data.update(params) - return self.request_body_xml_template.format(**data) + return self.request_body_xml_template.format(**data).encode('utf-8') def get_response_values(self, response): """Gets the values from the given response""" @@ -228,10 +229,14 @@ class LTIModuleTest(LogicTest): request.body = self.get_request_body(params={'grade': '0,5'}) response = self.xmodule.grade_handler(request, '') real_response = self.get_response_values(response) + if six.PY2: + msg = u'invalid literal for float(): 0,5' + else: + msg = u"could not convert string to float: '0,5'" expected_response = { 'action': None, 'code_major': 'failure', - 'description': 'Request body XML parsing error: invalid literal for float(): 0,5', + 'description': u'Request body XML parsing error: {}'.format(msg), 'messageIdentifier': 'unknown', } self.assertEqual(response.status_code, 200) @@ -396,13 +401,13 @@ class LTIModuleTest(LogicTest): """ mock_request = Mock() mock_request.headers = { - 'X-Requested-With': 'XMLHttpRequest', - 'Content-Type': 'application/x-www-form-urlencoded', - 'Authorization': ( + u'X-Requested-With': u'XMLHttpRequest', + u'Content-Type': u'application/x-www-form-urlencoded', + u'Authorization': ( u'OAuth realm="https://testurl/", oauth_body_hash="wwzA3s8gScKD1VpJ7jMt9b%2BMj9Q%3D",' - 'oauth_nonce="18821463", oauth_timestamp="1409321145", ' - 'oauth_consumer_key="__consumer_key__", oauth_signature_method="HMAC-SHA1", ' - 'oauth_version="1.0", oauth_signature="fHsE1hhIz76/msUoMR3Lyb7Aou4%3D"' + u'oauth_nonce="18821463", oauth_timestamp="1409321145", ' + u'oauth_consumer_key="__consumer_key__", oauth_signature_method="HMAC-SHA1", ' + u'oauth_version="1.0", oauth_signature="fHsE1hhIz76/msUoMR3Lyb7Aou4%3D"' ) } mock_request.url = u'https://testurl' @@ -410,15 +415,15 @@ class LTIModuleTest(LogicTest): mock_request.method = mock_request.http_method mock_request.body = ( - '\n' - '' - 'V1.0' - 'edX_fix' - '' - 'MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131' # pylint: disable=line-too-long - 'en0.32' - '' - ) + u'\n' + u'' + u'V1.0' + u'edX_fix' + u'' + u'MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131' # pylint: disable=line-too-long + u'en0.32' + u'' + ).encode('utf-8') return mock_request