From b0ef1909f9510029bad9f0f2206b9f96bd49ca49 Mon Sep 17 00:00:00 2001 From: Steve Kowalik Date: Wed, 11 Dec 2019 16:02:14 +1100 Subject: [PATCH] Refactor Logging tests (#1315) To stop skipping logging tests on Python 3, refactor them to mock out the logger and check arguments directly, rather than naively comparing strings. Allow a logger to be injected into Requester for this. --- github/Requester.py | 16 +++- tests/Logging_.py | 173 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 150 insertions(+), 39 deletions(-) diff --git a/github/Requester.py b/github/Requester.py index 7e5a93a6..e5082a66 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -169,6 +169,7 @@ class Requester: __httpsConnectionClass = HTTPSRequestsConnectionClass __connection = None __persist = True + __logger = None @classmethod def injectConnectionClasses(cls, httpConnectionClass, httpsConnectionClass): @@ -182,6 +183,14 @@ class Requester: cls.__httpConnectionClass = HTTPRequestsConnectionClass cls.__httpsConnectionClass = HTTPSRequestsConnectionClass + @classmethod + def injectLogger(cls, logger): + cls.__logger = logger + + @classmethod + def resetLogger(cls): + cls.__logger = None + ############################################################# # For Debug @classmethod @@ -565,8 +574,9 @@ class Requester: return self.__connection def __log(self, verb, url, requestHeaders, input, status, responseHeaders, output): - logger = logging.getLogger(__name__) - if logger.isEnabledFor(logging.DEBUG): + if self.__logger is None: + self.__logger = logging.getLogger(__name__) + if self.__logger.isEnabledFor(logging.DEBUG): if "Authorization" in requestHeaders: if requestHeaders["Authorization"].startswith("Basic"): requestHeaders[ @@ -580,7 +590,7 @@ class Requester: requestHeaders[ "Authorization" ] = "(unknown auth removed)" # pragma no cover (Cannot happen, but could if we add an authentication method => be prepared) - logger.debug( + self.__logger.debug( "%s %s://%s%s %s %s ==> %i %s %s", verb, self.__scheme, diff --git a/tests/Logging_.py b/tests/Logging_.py index 52c1fe80..d84d3312 100644 --- a/tests/Logging_.py +++ b/tests/Logging_.py @@ -31,61 +31,146 @@ from __future__ import absolute_import -import logging -import sys - import github from . import Framework -python2 = sys.hexversion < 0x03000000 - class Logging(Framework.BasicTestCase): - class MockHandler: + class MockLogger: def __init__(self): - self.level = logging.DEBUG - self.handled = None + self.verb = None + self.url = None + self.requestHeaders = None + self.input = None + self.status = None + self.responseHeaders = None + self.output = None - def handle(self, record): - self.handled = record.getMessage() + def isEnabledFor(self, kind): + return True + + def debug( + self, + format_string, + verb, + scheme, + hostname, + fragment, + requestHeaders, + input_, + status, + responseHeaders, + output, + ): + self.verb = verb + self.url = "%s://%s%s" % (scheme, hostname, fragment) + self.requestHeaders = requestHeaders + self.input = input_ + self.status = status + self.responseHeaders = responseHeaders + self.output = output def setUp(self): + self.logger = self.MockLogger() + github.Requester.Requester.injectLogger(self.logger) Framework.BasicTestCase.setUp(self) - logger = logging.getLogger("github") - logger.setLevel(logging.DEBUG) - self.__handler = self.MockHandler() - logger.addHandler(self.__handler) + + def tearDown(self): + github.Requester.Requester.resetLogger() + + def assertLogging(self, verb, url, requestHeaders, responseHeaders, output): + self.assertEqual(self.logger.verb, verb) + self.assertEqual(self.logger.url, url) + self.assertEqual(self.logger.requestHeaders, requestHeaders) + self.assertIsNone(self.logger.input) + self.assertEqual(self.logger.status, 200) + self.assertEqual(self.logger.responseHeaders, responseHeaders) + self.assertEqual(self.logger.output, output) def testLoggingWithBasicAuthentication(self): self.assertEqual( github.Github(self.login, self.password).get_user().name, "Vincent Jacques" ) - # In Python 3.4+, dicts are not output in the same order as in Python 2.7. - # So, logging is not deterministic and we cannot test it. - if python2: - self.assertEqual( - self.__handler.handled, - 'GET https://api.github.com/user {\'Authorization\': \'Basic (login and password removed)\', \'User-Agent\': \'PyGithub/Python\'} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'806\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept, Authorization, Cookie\', \'x-ratelimit-remaining\': \'4993\', \'server\': \'nginx\', \'last-modified\': \'Fri, 14 Sep 2012 18:47:46 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"434dfe5d3f50558fe3cea087cb95c401"\', \'cache-control\': \'private, s-maxage=60, max-age=60\', \'date\': \'Mon, 17 Sep 2012 17:12:32 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"owned_private_repos":3,"disk_usage":18612,"following":28,"type":"User","public_repos":13,"location":"Paris, France","company":"Criteo","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","plan":{"space":614400,"private_repos":5,"name":"micro","collaborators":1},"blog":"http://vincent-jacques.net","login":"jacquev6","public_gists":3,"html_url":"https://github.com/jacquev6","hireable":false,"created_at":"2010-07-09T06:10:06Z","private_gists":5,"followers":13,"name":"Vincent Jacques","email":"vincent@vincent-jacques.net","bio":"","total_private_repos":3,"collaborators":0,"gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"url":"https://api.github.com/users/jacquev6"}', - ) + url = "https://api.github.com/user" + requestHeaders = { + "Authorization": "Basic (login and password removed)", + "User-Agent": "PyGithub/Python", + } + responseHeaders = { + "status": "200 OK", + "content-length": "806", + "x-github-media-type": "github.beta; format=json", + "x-content-type-options": "nosniff", + "x-ratelimit-limit": "5000", + "vary": "Accept, Authorization, Cookie", + "x-ratelimit-remaining": "4993", + "server": "nginx", + "last-modified": "Fri, 14 Sep 2012 18:47:46 GMT", + "connection": "keep-alive", + "etag": '"434dfe5d3f50558fe3cea087cb95c401"', + "cache-control": "private, s-maxage=60, max-age=60", + "date": "Mon, 17 Sep 2012 17:12:32 GMT", + "content-type": "application/json; charset=utf-8", + "DEBUG_FRAME": 0, + } + output = '{"owned_private_repos":3,"disk_usage":18612,"following":28,"type":"User","public_repos":13,"location":"Paris, France","company":"Criteo","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","plan":{"space":614400,"private_repos":5,"name":"micro","collaborators":1},"blog":"http://vincent-jacques.net","login":"jacquev6","public_gists":3,"html_url":"https://github.com/jacquev6","hireable":false,"created_at":"2010-07-09T06:10:06Z","private_gists":5,"followers":13,"name":"Vincent Jacques","email":"vincent@vincent-jacques.net","bio":"","total_private_repos":3,"collaborators":0,"gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"url":"https://api.github.com/users/jacquev6"}' + self.assertLogging("GET", url, requestHeaders, responseHeaders, output) def testLoggingWithOAuthAuthentication(self): self.assertEqual( github.Github(self.oauth_token).get_user().name, "Vincent Jacques" ) - if python2: - self.assertEqual( - self.__handler.handled, - "GET https://api.github.com/user {'Authorization': 'token (oauth token removed)', 'User-Agent': 'PyGithub/Python'} None ==> 200 {'status': '200 OK', 'x-ratelimit-remaining': '4993', 'x-github-media-type': 'github.beta; format=json', 'x-content-type-options': 'nosniff', 'vary': 'Accept, Authorization, Cookie', 'content-length': '628', 'server': 'nginx', 'last-modified': 'Tue, 25 Sep 2012 07:42:42 GMT', 'connection': 'keep-alive', 'x-ratelimit-limit': '5000', 'etag': '\"c23ad6b5815fc3d6ec6341c4a47afe85\"', 'cache-control': 'private, max-age=60, s-maxage=60', 'date': 'Tue, 25 Sep 2012 20:36:54 GMT', 'x-oauth-scopes': '', 'content-type': 'application/json; charset=utf-8', 'x-accepted-oauth-scopes': 'user'} {\"type\":\"User\",\"bio\":\"\",\"html_url\":\"https://github.com/jacquev6\",\"login\":\"jacquev6\",\"followers\":14,\"company\":\"Criteo\",\"blog\":\"http://vincent-jacques.net\",\"public_repos\":13,\"created_at\":\"2010-07-09T06:10:06Z\",\"avatar_url\":\"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png\",\"email\":\"vincent@vincent-jacques.net\",\"following\":29,\"name\":\"Vincent Jacques\",\"gravatar_id\":\"b68de5ae38616c296fa345d2b9df2225\",\"hireable\":false,\"id\":327146,\"public_gists\":3,\"location\":\"Paris, France\",\"url\":\"https://api.github.com/users/jacquev6\"}", - ) + url = "https://api.github.com/user" + requestHeaders = { + "Authorization": "token (oauth token removed)", + "User-Agent": "PyGithub/Python", + } + responseHeaders = { + "status": "200 OK", + "x-ratelimit-remaining": "4993", + "x-github-media-type": "github.beta; format=json", + "x-content-type-options": "nosniff", + "vary": "Accept, Authorization, Cookie", + "content-length": "628", + "server": "nginx", + "last-modified": "Tue, 25 Sep 2012 07:42:42 GMT", + "connection": "keep-alive", + "x-ratelimit-limit": "5000", + "etag": '"c23ad6b5815fc3d6ec6341c4a47afe85"', + "cache-control": "private, max-age=60, s-maxage=60", + "date": "Tue, 25 Sep 2012 20:36:54 GMT", + "x-oauth-scopes": "", + "content-type": "application/json; charset=utf-8", + "x-accepted-oauth-scopes": "user", + "DEBUG_FRAME": 0, + } + output = '{"type":"User","bio":"","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","blog":"http://vincent-jacques.net","public_repos":13,"created_at":"2010-07-09T06:10:06Z","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","email":"vincent@vincent-jacques.net","following":29,"name":"Vincent Jacques","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","hireable":false,"id":327146,"public_gists":3,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}' + self.assertLogging("GET", url, requestHeaders, responseHeaders, output) def testLoggingWithoutAuthentication(self): self.assertEqual(github.Github().get_user("jacquev6").name, "Vincent Jacques") - if python2: - self.assertEqual( - self.__handler.handled, - 'GET https://api.github.com/users/jacquev6 {\'User-Agent\': \'PyGithub/Python\'} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'628\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept\', \'x-ratelimit-remaining\': \'4989\', \'server\': \'nginx\', \'last-modified\': \'Tue, 25 Sep 2012 07:42:42 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"9bd085221a16b6d2ea95e72634c3c1ac"\', \'cache-control\': \'public, max-age=60, s-maxage=60\', \'date\': \'Tue, 25 Sep 2012 20:38:56 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}', - ) + url = "https://api.github.com/users/jacquev6" + requestHeaders = {"User-Agent": "PyGithub/Python"} + responseHeaders = { + "status": "200 OK", + "content-length": "628", + "x-github-media-type": "github.beta; format=json", + "x-content-type-options": "nosniff", + "x-ratelimit-limit": "5000", + "vary": "Accept", + "x-ratelimit-remaining": "4989", + "server": "nginx", + "last-modified": "Tue, 25 Sep 2012 07:42:42 GMT", + "connection": "keep-alive", + "etag": '"9bd085221a16b6d2ea95e72634c3c1ac"', + "cache-control": "public, max-age=60, s-maxage=60", + "date": "Tue, 25 Sep 2012 20:38:56 GMT", + "content-type": "application/json; charset=utf-8", + "DEBUG_FRAME": 0, + } + output = '{"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}' + self.assertLogging("GET", url, requestHeaders, responseHeaders, output) def testLoggingWithBaseUrl(self): # ReplayData forged, not recorded @@ -95,8 +180,24 @@ class Logging(Framework.BasicTestCase): .name, "Vincent Jacques", ) - if python2: - self.assertEqual( - self.__handler.handled, - 'GET http://my.enterprise.com/my/prefix/users/jacquev6 {\'User-Agent\': \'PyGithub/Python\'} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'628\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept\', \'x-ratelimit-remaining\': \'4989\', \'server\': \'nginx\', \'last-modified\': \'Tue, 25 Sep 2012 07:42:42 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"9bd085221a16b6d2ea95e72634c3c1ac"\', \'cache-control\': \'public, max-age=60, s-maxage=60\', \'date\': \'Tue, 25 Sep 2012 20:38:56 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}', - ) + url = "http://my.enterprise.com/my/prefix/users/jacquev6" + requestHeaders = {"User-Agent": "PyGithub/Python"} + responseHeaders = { + "status": "200 OK", + "content-length": "628", + "x-github-media-type": "github.beta; format=json", + "x-content-type-options": "nosniff", + "x-ratelimit-limit": "5000", + "vary": "Accept", + "x-ratelimit-remaining": "4989", + "server": "nginx", + "last-modified": "Tue, 25 Sep 2012 07:42:42 GMT", + "connection": "keep-alive", + "etag": '"9bd085221a16b6d2ea95e72634c3c1ac"', + "cache-control": "public, max-age=60, s-maxage=60", + "date": "Tue, 25 Sep 2012 20:38:56 GMT", + "content-type": "application/json; charset=utf-8", + "DEBUG_FRAME": 0, + } + output = '{"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}' + self.assertLogging("GET", url, requestHeaders, responseHeaders, output)