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

@@ -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
     )

test-requirements.txt

@@ -1,2 +1,4 @@
 cryptography
 httpretty==0.9.6
+mock==3.0.5; python_version < '3.3'
+parameterized==0.7.0

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,

tests/Connection.py

@@ -0,0 +1,114 @@
+# -*- coding: utf-8 -*-
+
+############################ Copyrights and license ############################
+#                                                                              #
+# Copyright 2019 Adam Baratz <adam.baratz@gmail.com>                           #
+#                                                                              #
+# 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 <http://www.gnu.org/licenses/>.             #
+#                                                                              #
+################################################################################
+
+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()

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):