From faa1bbd61fb6f5bb06c26a48a6fb6108d9f0af2f Mon Sep 17 00:00:00 2001 From: Adam Baratz Date: Tue, 22 Oct 2019 23:50:08 -0400 Subject: [PATCH] Handle unicode strings when recording responses (#1253) (#1254) * Handle unicode strings when recording responses (#1253) * Add new test requirements * Make requirement conditional * Fix flake8 error * Compare dict values without worrying about key order * Improve readability --- setup.py | 19 +++++-- test-requirements.txt | 2 + tests/AllTests.py | 3 ++ tests/Connection.py | 114 ++++++++++++++++++++++++++++++++++++++++++ tests/Framework.py | 35 +++++++------ 5 files changed, 150 insertions(+), 23 deletions(-) create mode 100644 tests/Connection.py diff --git a/setup.py b/setup.py index 66993e40..4ce2b3e2 100755 --- a/setup.py +++ b/setup.py @@ -41,11 +41,23 @@ # # ################################################################################ -import setuptools +import sys import textwrap +import setuptools + + version = "1.44" +tests_require = [ + "cryptography", + "httpretty>=0.9.6", + "parameterized==0.7.0", +] + +if sys.version_info < (3, 3): + tests_require.append("mock==3.0.5") + if __name__ == "__main__": setuptools.setup( @@ -108,8 +120,5 @@ if __name__ == "__main__": extras_require={ "integrations": ["cryptography"] }, - tests_require=[ - "cryptography", - "httpretty>=0.9.6" - ] + tests_require=tests_require ) diff --git a/test-requirements.txt b/test-requirements.txt index c9e5e042..414f91f5 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,2 +1,4 @@ cryptography httpretty==0.9.6 +mock==3.0.5; python_version < '3.3' +parameterized==0.7.0 diff --git a/tests/AllTests.py b/tests/AllTests.py index dc2f09e9..40de3ddd 100644 --- a/tests/AllTests.py +++ b/tests/AllTests.py @@ -108,6 +108,8 @@ from .Equality import Equality from .Search import Search from .Retry import Retry +from .Connection import Connection + from .Issue33 import Issue33 from .Issue50 import Issue50 from .Issue54 import Issue54 @@ -146,6 +148,7 @@ __all__ = [ CommitComment, CommitStatus, ConditionalRequestUpdate, + Connection, ContentFile, Download, Enterprise, diff --git a/tests/Connection.py b/tests/Connection.py new file mode 100644 index 00000000..31ed0eb8 --- /dev/null +++ b/tests/Connection.py @@ -0,0 +1,114 @@ +# -*- coding: utf-8 -*- + +############################ Copyrights and license ############################ +# # +# Copyright 2019 Adam Baratz # +# # +# This file is part of PyGithub. # +# http://pygithub.readthedocs.io/ # +# # +# PyGithub is free software: you can redistribute it and/or modify it under # +# the terms of the GNU Lesser General Public License as published by the Free # +# Software Foundation, either version 3 of the License, or (at your option) # +# any later version. # +# # +# PyGithub is distributed in the hope that it will be useful, but WITHOUT ANY # +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS # +# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more # +# details. # +# # +# You should have received a copy of the GNU Lesser General Public License # +# along with PyGithub. If not, see . # +# # +################################################################################ + +from __future__ import absolute_import + +import itertools +import unittest +from io import StringIO +try: + from unittest.mock import Mock +except ImportError: + from mock import Mock + +import httpretty +from parameterized import parameterized + +from . import Framework + + +PARAMETERS = itertools.product( + [ + (Framework.ReplayingHttpConnection, "http"), + (Framework.ReplayingHttpsConnection, "https"), + ], + [ + ( + "{\"body\":\"BODY TEXT\"}", + '\nGET\napi.github.com\nNone\n/user\n{\'Authorization\': \'Basic login_and_password_removed\', \'User-Agent\': \'PyGithub/Python\'}\nNone\n200\n[]\n{"body":"BODY TEXT"}\n\n', + ), + ( + u"{\"body\":\"BODY\xa0TEXT\"}", + u'\nGET\napi.github.com\nNone\n/user\n{\'Authorization\': \'Basic login_and_password_removed\', \'User-Agent\': \'PyGithub/Python\'}\nNone\n200\n[]\n{"body":"BODY\xa0TEXT"}\n\n', + ), + ( + "BODY TEXT", + '\nGET\napi.github.com\nNone\n/user\n{\'Authorization\': \'Basic login_and_password_removed\', \'User-Agent\': \'PyGithub/Python\'}\nNone\n200\n[]\nBODY TEXT\n\n', + ), + ( + u"BODY\xa0TEXT", + u'\nGET\napi.github.com\nNone\n/user\n{\'Authorization\': \'Basic login_and_password_removed\', \'User-Agent\': \'PyGithub/Python\'}\nNone\n200\n[]\nBODY\xa0TEXT\n\n', + ), + ], +) + + +class RecordingMockConnection(Framework.RecordingConnection): + def __init__(self, file, protocol, host, port, realConnection): + self._realConnection = realConnection + Framework.RecordingConnection.__init__(self, file, protocol, host, port) + + +class Connection(unittest.TestCase): + @parameterized.expand(itertools.chain(*p) for p in PARAMETERS) + def testRecordAndReplay(self, replaying_connection_class, protocol, response_body, expected_recording): + file = StringIO() + host = "api.github.com" + verb = "GET" + url = "/user" + headers = {'Authorization': 'Basic p4ssw0rd', 'User-Agent': 'PyGithub/Python'} + + response = Mock() + response.status = 200 + response.getheaders.return_value = {} + response.read.return_value = response_body + + connection = Mock() + connection.getresponse.return_value = response + + # write mock response to buffer + recording_connection = RecordingMockConnection(file, protocol, host, None, lambda *args, **kwds: connection) + recording_connection.request(verb, url, None, headers) + recording_connection.getresponse() + recording_connection.close() + + # validate contents of buffer + file_value_lines = file.getvalue().split("\n") + expected_recording_lines = (protocol + expected_recording).split("\n") + self.assertEquals(file_value_lines[:5], expected_recording_lines[:5]) + self.assertEquals(eval(file_value_lines[5]), eval(expected_recording_lines[5])) # dict literal, so keys not in guaranteed order + self.assertEquals(file_value_lines[6:], expected_recording_lines[6:]) + + # required for replay to work as expected + httpretty.enable(allow_net_connect=False) + + # rewind buffer and attempt to replay response from it + file.seek(0) + replaying_connection = replaying_connection_class(self, file, host=host, port=None) + replaying_connection.request(verb, url, None, headers) + replaying_connection.getresponse() + + # not necessarily required for subsequent tests + httpretty.disable() + httpretty.reset() diff --git a/tests/Framework.py b/tests/Framework.py index c81d3eb9..3416e6b4 100644 --- a/tests/Framework.py +++ b/tests/Framework.py @@ -38,6 +38,7 @@ from __future__ import absolute_import from __future__ import print_function +import io import json import os import traceback @@ -86,12 +87,14 @@ def fixAuthorizationHeader(headers): headers["Authorization"] = "Bearer jwt_removed" -class RecordingConnection: # pragma no cover (Class useful only when recording new tests, not used during automated tests) +class RecordingConnection: def __init__(self, file, protocol, host, port, *args, **kwds): + # write operations make the assumption that the file is not in binary mode + assert isinstance(file, io.TextIOBase) self.__file = file self.__protocol = protocol self.__host = host - self.__port = str(port) + self.__port = port self.__cnx = self._realConnection(host, port, *args, **kwds) def request(self, verb, url, input, headers): @@ -111,8 +114,8 @@ class RecordingConnection: # pragma no cover (Class useful only when recording self.__writeLine(self.__host) self.__writeLine(self.__port) self.__writeLine(url) - self.__writeLine(str(anonymous_headers)) - self.__writeLine(str(input).replace('\n', '').replace('\r', '')) + self.__writeLine(anonymous_headers) + self.__writeLine(six.text_type(input).replace('\n', '').replace('\r', '')) def getresponse(self): res = self.__cnx.getresponse() @@ -122,10 +125,8 @@ class RecordingConnection: # pragma no cover (Class useful only when recording headers = res.getheaders() output = res.read() - self.__writeLine(str(status)) - self.__writeLine(str(list(headers))) - if isinstance(output, bytes): - output = output.decode('utf-8') + self.__writeLine(status) + self.__writeLine(list(headers)) self.__writeLine(output) return FakeHttpResponse(status, headers, output) @@ -135,19 +136,17 @@ class RecordingConnection: # pragma no cover (Class useful only when recording return self.__cnx.close() def __writeLine(self, line): - if isinstance(line, bytes): - line = line.decode('utf-8') - self.__file.write(line + '\n') + self.__file.write(six.text_type(line) + u'\n') -class RecordingHttpConnection(RecordingConnection): # pragma no cover (Class useful only when recording new tests, not used during automated tests) +class RecordingHttpConnection(RecordingConnection): _realConnection = github.Requester.HTTPRequestsConnectionClass def __init__(self, file, *args, **kwds): RecordingConnection.__init__(self, file, "http", *args, **kwds) -class RecordingHttpsConnection(RecordingConnection): # pragma no cover (Class useful only when recording new tests, not used during automated tests) +class RecordingHttpsConnection(RecordingConnection): _realConnection = github.Requester.HTTPSRequestsConnectionClass def __init__(self, file, *args, **kwds): @@ -259,8 +258,8 @@ class BasicTestCase(unittest.TestCase): self.__file = None if self.recordMode: # pragma no cover (Branch useful only when recording new tests, not used during automated tests) github.Requester.Requester.injectConnectionClasses( - lambda ignored, *args, **kwds: RecordingHttpConnection(self.__openFile("wb"), *args, **kwds), - lambda ignored, *args, **kwds: RecordingHttpsConnection(self.__openFile("wb"), *args, **kwds) + lambda ignored, *args, **kwds: RecordingHttpConnection(self.__openFile("w"), *args, **kwds), + lambda ignored, *args, **kwds: RecordingHttpsConnection(self.__openFile("w"), *args, **kwds) ) import GithubCredentials self.login = GithubCredentials.login @@ -272,8 +271,8 @@ class BasicTestCase(unittest.TestCase): # self.client_secret = GithubCredentials.client_secret else: github.Requester.Requester.injectConnectionClasses( - lambda ignored, *args, **kwds: ReplayingHttpConnection(self, self.__openFile("rb"), *args, **kwds), - lambda ignored, *args, **kwds: ReplayingHttpsConnection(self, self.__openFile("rb"), *args, **kwds) + lambda ignored, *args, **kwds: ReplayingHttpConnection(self, self.__openFile("r"), *args, **kwds), + lambda ignored, *args, **kwds: ReplayingHttpsConnection(self, self.__openFile("r"), *args, **kwds) ) self.login = "login" self.password = "password" @@ -299,7 +298,7 @@ class BasicTestCase(unittest.TestCase): if fileName != self.__fileName: self.__closeReplayFileIfNeeded() self.__fileName = fileName - self.__file = open(self.__fileName, mode) + self.__file = io.open(self.__fileName, mode, encoding="utf-8") return self.__file def __closeReplayFileIfNeeded(self):