From d83a2f68c1aa9865e521d1a3f99bbf788b96065d Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Thu, 1 Feb 2024 23:20:15 +0100 Subject: [PATCH 01/10] [runtests] Add a utility function to generate URLs --- runtests.py | 149 ++++++++++++++++++++++++++++------------------------ 1 file changed, 79 insertions(+), 70 deletions(-) diff --git a/runtests.py b/runtests.py index 0c35226..c86251e 100755 --- a/runtests.py +++ b/runtests.py @@ -45,7 +45,7 @@ ENV = 'test-trac-github' CONF = '%s/conf/trac.ini' % ENV HTDIGEST = '%s/passwd' % ENV -URL = 'http://localhost:8765/%s' % ENV +TRACD_URL = 'http://localhost:8765/%s' % ENV SECRET = 'test-secret' HEADERS = {'Content-Type': 'application/json', 'X-GitHub-Event': 'push'} UPDATEHOOK = '%s-mirror/hooks/trac-github-update' % GIT @@ -75,6 +75,15 @@ def d(*args): return os.path.join(TESTDIR, *args) +def u(*path_elements): + """ + Return an absolute URL where the given arguments are joined (with /) + and prepended with TRACD_URL. + """ + url = '/'.join([TRACD_URL] + list(path_elements)) + return url + + def git_check_output(*args, **kwargs): """ Run the given git command (`*args`), optionally on the given @@ -243,7 +252,7 @@ def startTracd(cls, **kwargs): waittime = 0.1 for _ in range(5): try: - urllib2.urlopen(URL) + urllib2.urlopen(u()) except urllib2.URLError: time.sleep(waittime) waittime *= 2 @@ -302,7 +311,7 @@ def makeGitHubHookPayload(n=1, reponame=''): def openGitHubHook(n=1, reponame='', payload=None): if not payload: payload = TracGitHubTests.makeGitHubHookPayload(n, reponame) - url = (URL + '/github/' + reponame) if reponame else URL + '/github' + url = u('github', reponame) if reponame else u('github') request = urllib2.Request(url, json.dumps(payload), HEADERS) return urllib2.urlopen(request) @@ -313,7 +322,7 @@ def testLinkToChangeset(self): self.makeGitCommit(GIT, 'myfile', 'for browser tests') changeset = self.openGitHubHook().read().rstrip()[-40:] try: - urllib2.urlopen(URL + '/changeset/' + changeset) + urllib2.urlopen(u('changeset', changeset)) except urllib2.HTTPError as exc: self.assertEqual(exc.code, 302) self.assertEqual(exc.headers['Location'], @@ -325,7 +334,7 @@ def testAlternateLinkToChangeset(self): self.makeGitCommit(ALTGIT, 'myfile', 'for browser tests') changeset = self.openGitHubHook(1, 'alt').read().rstrip()[-40:] try: - urllib2.urlopen(URL + '/changeset/' + changeset + '/alt') + urllib2.urlopen(u('changeset', changeset, 'alt')) except urllib2.HTTPError as exc: self.assertEqual(exc.code, 302) self.assertEqual(exc.headers['Location'], @@ -336,14 +345,14 @@ def testAlternateLinkToChangeset(self): def testNonGitHubLinkToChangeset(self): changeset = self.makeGitCommit(NOGHGIT, 'myfile', 'for browser tests') subprocess.check_output([TRAC_ADMIN_BIN, d(ENV), 'changeset', 'added', 'nogh', changeset]) - response = requests.get(URL + '/changeset/' + changeset + '/nogh', allow_redirects=False) + response = requests.get(u('changeset', changeset, 'nogh'), allow_redirects=False) self.assertEqual(response.status_code, 200) def testLinkToPath(self): self.makeGitCommit(GIT, 'myfile', 'for more browser tests') changeset = self.openGitHubHook().read().rstrip()[-40:] try: - urllib2.urlopen(URL + '/changeset/' + changeset + '/myfile') + urllib2.urlopen(u('changeset', changeset, 'myfile')) except urllib2.HTTPError as exc: self.assertEqual(exc.code, 302) self.assertEqual(exc.headers['Location'], @@ -355,7 +364,7 @@ def testAlternateLinkToPath(self): self.makeGitCommit(ALTGIT, 'myfile', 'for more browser tests') changeset = self.openGitHubHook(1, 'alt').read().rstrip()[-40:] try: - urllib2.urlopen(URL + '/changeset/' + changeset + '/alt/myfile') + urllib2.urlopen(u('changeset', changeset, 'alt/myfile')) except urllib2.HTTPError as exc: self.assertEqual(exc.code, 302) self.assertEqual(exc.headers['Location'], @@ -366,16 +375,16 @@ def testAlternateLinkToPath(self): def testNonGitHubLinkToPath(self): changeset = self.makeGitCommit(NOGHGIT, 'myfile', 'for more browser tests') subprocess.check_output([TRAC_ADMIN_BIN, d(ENV), 'changeset', 'added', 'nogh', changeset]) - response = requests.get(URL + '/changeset/' + changeset + '/nogh/myfile', allow_redirects=False) + response = requests.get(u('changeset', changeset, 'nogh/myfile'), allow_redirects=False) self.assertEqual(response.status_code, 200) def testBadChangeset(self): with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): - urllib2.urlopen(URL + '/changeset/1234567890') + urllib2.urlopen(u('changeset/1234567890')) def testBadUrl(self): with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): - urllib2.urlopen(URL + '/changesetnosuchurl') + urllib2.urlopen(u('changesetnosuchurl')) def testTimelineFiltering(self): self.makeGitBranch(GIT, 'stable/2.0') @@ -390,7 +399,7 @@ def testTimelineFiltering(self): self.makeGitCommit(ALTGIT, 'myfile', 'timeline 6\n', 'msg 6', 'unstable/2.0') self.openGitHubHook(3) self.openGitHubHook(3, 'alt') - html = urllib2.urlopen(URL + '/timeline').read() + html = urllib2.urlopen(u('timeline')).read() self.assertTrue('msg 1' in html) self.assertTrue('msg 2' in html) self.assertTrue('msg 3' in html) @@ -409,7 +418,7 @@ def startTracd(cls, **kwargs): super(GitHubLoginModuleTests, cls).startTracd(**kwargs) def testLogin(self): - response = requests.get(URL + '/github/login', allow_redirects=False) + response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) redirect_url = urlparse.urlparse(response.headers['Location']) @@ -420,7 +429,7 @@ def testLogin(self): state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': ['01234567890123456789'], - 'redirect_uri': [URL + '/github/oauth'], + 'redirect_uri': [u('github/oauth')], 'response_type': ['code'], 'scope': [''], 'state': [state], @@ -430,16 +439,16 @@ def testOauthInvalidState(self): session = requests.Session() # This adds a oauth_state parameter in the Trac session. - response = session.get(URL + '/github/login', allow_redirects=False) + response = session.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) response = session.get( - URL + '/github/oauth?code=01234567890123456789&state=wrong_state', + u('github/oauth') + '?code=01234567890123456789&state=wrong_state', allow_redirects=False) self.assertEqual(response.status_code, 302) - self.assertEqual(response.headers['Location'], URL) + self.assertEqual(response.headers['Location'], u()) - response = session.get(URL) + response = session.get(u()) self.assertEqual(response.status_code, 200) self.assertIn( "Invalid request. Please try to login again.", response.text) @@ -451,20 +460,20 @@ def testOauthInvalidStateWithoutSession(self): # OAuth callback requests without state must still fail. response = session.get( - URL + '/github/oauth?code=01234567890123456789', + u('github/oauth') + '?code=01234567890123456789', allow_redirects=False) self.assertEqual(response.status_code, 302) - self.assertEqual(response.headers['Location'], URL) + self.assertEqual(response.headers['Location'], u()) - response = session.get(URL) + response = session.get(u()) self.assertEqual(response.status_code, 200) self.assertIn( "Invalid request. Please try to login again.", response.text) def testLogout(self): - response = requests.get(URL + '/github/logout', allow_redirects=False) + response = requests.get(u('github/logout'), allow_redirects=False) self.assertEqual(response.status_code, 302) - self.assertEqual(response.headers['Location'], URL) + self.assertEqual(response.headers['Location'], u()) class GitHubLoginModuleConfigurationTests(TracGitHubTests): # Append custom failure messages to the automatically generated ones @@ -507,7 +516,7 @@ def tearDownClass(cls): def testLoginWithReqEmail(self): """Test that configuring request_email = true requests the user:email scope from GitHub""" with TracContext(self, request_email=True, resync=False): - response = requests.get(URL + '/github/login', allow_redirects=False) + response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) redirect_url = urlparse.urlparse(response.headers['Location']) @@ -518,7 +527,7 @@ def testLoginWithReqEmail(self): state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': ['01234567890123456789'], - 'redirect_uri': [URL + '/github/oauth'], + 'redirect_uri': [u('github/oauth')], 'response_type': ['code'], 'scope': ['user:email'], 'state': [state], @@ -529,7 +538,7 @@ def loginAndVerifyClientId(self, expected_client_id): Open the login page and check that the client_id in the redirect target matches the expected value. """ - response = requests.get(URL + '/github/login', allow_redirects=False) + response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) redirect_url = urlparse.urlparse(response.headers['Location']) @@ -540,7 +549,7 @@ def loginAndVerifyClientId(self, expected_client_id): state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': [expected_client_id], - 'redirect_uri': [URL + '/github/oauth'], + 'redirect_uri': [u('github/oauth')], 'response_type': ['code'], 'scope': [''], 'state': [state], @@ -576,7 +585,7 @@ def testLoginWithUnconfiguredClientId(self): with TracContext(self, client_id=''): session = requests.Session() - response = session.get(URL + '/github/login', allow_redirects=True) + response = session.get(u('github/login'), allow_redirects=True) self.assertEqual(response.status_code, 200) tree = html.fromstring(response.content) @@ -600,10 +609,10 @@ def attemptHttpAuth(self, testenv, **kwargs): # This logs into trac using HTTP authentication # This adds a oauth_state parameter in the Trac session. - response = session.get(URL + '/login', auth=requests.auth.HTTPDigestAuth('user', 'pass')) + response = session.get(u('login'), auth=requests.auth.HTTPDigestAuth('user', 'pass')) self.assertNotEqual(response.status_code, 403) - response = session.get(URL + '/newticket') # this should trigger IPermissionGroupProvider + response = session.get(u('newticket')) # this should trigger IPermissionGroupProvider self.assertEqual(response.status_code, 200) tree = html.fromstring(response.content) warning = ''.join(tree.xpath('//div[@id="warning"]/text()')).strip() @@ -637,7 +646,7 @@ def attemptValidOauth(self, testenv, callback, **kwargs): session = requests.Session() # This adds a oauth_state parameter in the Trac session. - response = session.get(URL + '/github/login', allow_redirects=False) + response = session.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) # Extract the state from the redirect @@ -645,7 +654,7 @@ def attemptValidOauth(self, testenv, callback, **kwargs): params = urlparse.parse_qs(redirect_url.query, keep_blank_values=True) state = params['state'][0] # this is a random value response = session.get( - URL + '/github/oauth', + u('github/oauth'), params={ 'code': '01234567890123456789', 'state': state @@ -653,7 +662,7 @@ def attemptValidOauth(self, testenv, callback, **kwargs): allow_redirects=False) self.assertEqual(response.status_code, 302) - response = session.get(URL + '/prefs') + response = session.get(u('prefs')) self.assertEqual(response.status_code, 200) tree = html.fromstring(response.content) warning = ''.join(tree.xpath('//div[@id="warning"]/text()')).strip() @@ -973,7 +982,7 @@ def testUnknownCommit(self): # Emulate self.openGitHubHook to use a non-existent commit id random_id = ''.join(random.choice('0123456789abcdef') for _ in range(40)) payload = {'commits': [{'id': random_id, 'message': '', 'distinct': True}]} - request = urllib2.Request(URL + '/github', json.dumps(payload), HEADERS) + request = urllib2.Request(u('github'), json.dumps(payload), HEADERS) output = urllib2.urlopen(request).read() self.assertRegexpMatches(output, r"Running hook on \(default\)\n" r"\* Updating clone\n" @@ -1045,32 +1054,32 @@ def testComplexNotification(self): def testPing(self): payload = {'zen': "Readability counts."} headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping'} - request = urllib2.Request(URL + '/github', json.dumps(payload), headers) + request = urllib2.Request(u('github'), json.dumps(payload), headers) output = urllib2.urlopen(request).read() self.assertEqual(output, "Readability counts.") def testUnknownEvent(self): headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'pull'} - request = urllib2.Request(URL + '/github', json.dumps({}), headers) + request = urllib2.Request(u('github'), json.dumps({}), headers) with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): urllib2.urlopen(request) def testBadMethod(self): with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 405: Method Not Allowed$'): - urllib2.urlopen(URL + '/github') + urllib2.urlopen(u('github')) def testBadPayload(self): - request = urllib2.Request(URL + '/github', 'foobar', HEADERS) + request = urllib2.Request(u('github'), 'foobar', HEADERS) with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): urllib2.urlopen(request) def testBadRepository(self): - request = urllib2.Request(URL + '/github/nosuchrepo', '{}', HEADERS) + request = urllib2.Request(u('github/nosuchrepo'), '{}', HEADERS) with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): urllib2.urlopen(request) def testBadUrl(self): - request = urllib2.Request(URL + '/githubnosuchurl', '{}', HEADERS) + request = urllib2.Request(u('githubnosuchurl'), '{}', HEADERS) with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): urllib2.urlopen(request) @@ -1087,7 +1096,7 @@ def setUpClass(cls): def testUnsignedPing(self): payload = {'zen': "Readability counts."} headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping'} - request = urllib2.Request(URL + '/github', json.dumps(payload), headers) + request = urllib2.Request(u('github'), json.dumps(payload), headers) with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 403: Forbidden$'): urllib2.urlopen(request).read() @@ -1099,7 +1108,7 @@ def testSignedPing(self): headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping', 'X-Hub-Signature': signature} - request = urllib2.Request(URL + '/github', json.dumps(payload) + '\n', headers) + request = urllib2.Request(u('github'), json.dumps(payload) + '\n', headers) output = urllib2.urlopen(request).read() self.assertEqual(output, "Echo me") @@ -1418,7 +1427,7 @@ def test_000_api_refuses_connection(self): Test that a request does not fail even if the API refuses connections. """ with TracContext(self, env=self.tracd_env_broken, **self.trac_env): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200, "Request with unresponsive API endpoint should not fail") self.assertEqual(response.json(), {}, @@ -1429,7 +1438,7 @@ def test_001_unconfigured(self): Test whether a request with an unconfigured GitHubGroupsProvider fails. """ with TracContext(self, resync=False): - response = requests.get(URL + '/newticket', allow_redirects=False) + response = requests.get(u('newticket'), allow_redirects=False) self.assertEqual(response.status_code, 200, "Unconfigured GitHubGroupsProvider caused requests to fail") @@ -1440,7 +1449,7 @@ def test_002_disabled_debugging(self): self.assertNotIn('TRAC_GITHUB_ENABLE_DEBUGGING', self.tracd_env, "tracd_env enables debugging, but should not; did you export TRAC_GITHUB_ENABLE_DEBUGGING?") with TracContext(self, env=self.tracd_env, resync=False): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 404, "Debugging API was not enabled, but did not return HTTP 404") @@ -1452,7 +1461,7 @@ def test_003_api_returns_500(self): '/orgs/%s/teams' % self.organization: {} }) with TracContext(self, env=self.tracd_env_debug, **self.trac_env): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200, "Request with failing API endpoint should not fail") self.assertEqual(response.json(), {}, @@ -1465,7 +1474,7 @@ def test_004_api_returns_404(self): updateMockData(self.mockdata, retcode=404, answers={}) with TracContext(self, env=self.tracd_env_debug, **self.trac_env): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200, "Request with 404 API endpoint should not fail") self.assertEqual(response.json(), {}, @@ -1480,7 +1489,7 @@ def test_005_org_has_no_teams(self): }) with TracContext(self, env=self.tracd_env_debug, **self.trac_env): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200, "Request with organization without teams should not fail") self.assertEqual(response.json(), {}, @@ -1533,7 +1542,7 @@ def test_006_normal_operation(self): }) with TracContext(self, env=self.tracd_env_debug, **self.trac_env): - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1598,12 +1607,12 @@ def test_007_hook_get_request(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env): - response = requests.get(URL + '/github-groups', allow_redirects=False) + response = requests.get(u('github-groups'), allow_redirects=False) self.assertEqual(response.status_code, 405, "GET /github-groups did not return HTTP 405") self.assertEqual(response.text, "Endpoint is ready to accept GitHub Organization membership notifications.\n") - response = requests.get(URL + '/github-groups/', allow_redirects=False) + response = requests.get(u('github-groups/'), allow_redirects=False) self.assertEqual(response.status_code, 405, "/github-groups/ did not return 405") self.assertEqual(response.text, @@ -1618,7 +1627,7 @@ def test_008_hook_unsupported_event(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'FooEvent'}) self.assertEqual(response.status_code, 400, @@ -1634,7 +1643,7 @@ def test_009_hook_ping_event(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'ping'}, json={'zen': 'Echo me!'}) @@ -1651,7 +1660,7 @@ def test_010_hook_ping_event_nonjson_payload(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'ping'}, data="Fail to parse as JSON") @@ -1668,7 +1677,7 @@ def test_011_hook_ping_event_invalid_json_payload(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'ping'}, json=[{'bar': 'baz'}]) @@ -1725,7 +1734,7 @@ def test_012_hook_membership_event_delete_team(self): with TracContext(self, env=self.tracd_env_debug, **self.trac_env): # Make sure the to-be-removed group exists - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1748,7 +1757,7 @@ def test_012_hook_membership_event_delete_team(self): }) # Send the delete event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) @@ -1757,7 +1766,7 @@ def test_012_hook_membership_event_delete_team(self): self.assertEqual(response.text, "success") # Check that the group is gone - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1789,7 +1798,7 @@ def test_013_hook_membership_event_delete_nonexistant_team(self): with TracContext(self, env=self.tracd_env_debug, **self.trac_env): # Send the delete event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) @@ -1833,7 +1842,7 @@ def test_014_hook_membership_event_add_team(self): }) # Send the update event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) @@ -1842,7 +1851,7 @@ def test_014_hook_membership_event_add_team(self): self.assertEqual(response.text, "success") # Check that the member and group were added - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1898,7 +1907,7 @@ def test_015_hook_membership_event_add_member(self): }) # Send the update event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) @@ -1907,7 +1916,7 @@ def test_015_hook_membership_event_add_member(self): self.assertEqual(response.text, "success") # Check that the member and group were added - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1963,7 +1972,7 @@ def test_016_hook_membership_event_remove_member(self): }) # Send the update event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) @@ -1972,7 +1981,7 @@ def test_016_hook_membership_event_remove_member(self): self.assertEqual(response.text, "success") # Check that the member and group were added - response = requests.get(URL + '/github-groups-dump', allow_redirects=False) + response = requests.get(u('github-groups-dump'), allow_redirects=False) self.assertEqual(response.status_code, 200) data = response.json() @@ -1989,7 +1998,7 @@ def test_017_hook_unsigned_ping_event(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env_secured): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'ping'}, json={'zen': 'Echo me!'}) @@ -2006,7 +2015,7 @@ def test_018_hook_unsupported_sig_algo_ping_event(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env_secured): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={ 'X-GitHub-Event': 'ping', @@ -2026,7 +2035,7 @@ def test_019_hook_invalid_sig_ping_event(self): }) with TracContext(self, env=self.tracd_env, **self.trac_env_secured): - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={ 'X-GitHub-Event': 'ping', @@ -2049,7 +2058,7 @@ def test_020_hook_signed_ping_event(self): # Correct signature can be generated with OpenSSL: # $> printf '{"zen": "Echo me"}\n' | openssl dgst -sha256 -hmac $webhook_secret signature = "sha256=cacc93c2df1b21313e16d8690fc21e56229b6a9525e7016db38bdf9bad708fed" - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={ 'X-GitHub-Event': 'ping', @@ -2081,7 +2090,7 @@ def test_021_hook_membership_event_api_failure(self): # Change the mock to always fail updateMockData(self.mockdata, retcode=403) # Send the delete event - response = requests.post(URL + '/github-groups', + response = requests.post(u('github-groups'), allow_redirects=False, headers={'X-GitHub-Event': 'membership'}, json=update) From 385fc8d6f438cb04213d7fac25590f281db58cd3 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Thu, 1 Feb 2024 23:59:23 +0100 Subject: [PATCH 02/10] [runtests] Created custom assertRequestRedirect test method This cuts down on repetition and will help migrate away from urllib2 in a future commit. --- runtests.py | 66 +++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/runtests.py b/runtests.py index c86251e..ea8b696 100755 --- a/runtests.py +++ b/runtests.py @@ -317,30 +317,34 @@ def openGitHubHook(n=1, reponame='', payload=None): class GitHubBrowserTests(TracGitHubTests): - - def testLinkToChangeset(self): - self.makeGitCommit(GIT, 'myfile', 'for browser tests') - changeset = self.openGitHubHook().read().rstrip()[-40:] + def assertRequestRedirects(self, url, redirection, status=302): + """ + A custom assertion to check that requesting the given url (GET) returns a redirection + response (302 by default) pointed to the given redirection URL. + """ try: - urllib2.urlopen(u('changeset', changeset)) + urllib2.urlopen(url) except urllib2.HTTPError as exc: - self.assertEqual(exc.code, 302) - self.assertEqual(exc.headers['Location'], - 'https://github.com/aaugustin/trac-github/commit/%s' % changeset) + self.assertEqual(exc.code, status) + self.assertEqual(exc.headers['Location'], redirection) else: self.fail("URL didn't redirect") + def testLinkToChangeset(self): + self.makeGitCommit(GIT, 'myfile', 'for browser tests') + changeset = self.openGitHubHook().rstrip()[-40:] + self.assertRequestRedirects( + u('changeset', changeset), + 'https://github.com/aaugustin/trac-github/commit/%s' % changeset + ) + def testAlternateLinkToChangeset(self): self.makeGitCommit(ALTGIT, 'myfile', 'for browser tests') - changeset = self.openGitHubHook(1, 'alt').read().rstrip()[-40:] - try: - urllib2.urlopen(u('changeset', changeset, 'alt')) - except urllib2.HTTPError as exc: - self.assertEqual(exc.code, 302) - self.assertEqual(exc.headers['Location'], - 'https://github.com/follower/trac-github/commit/%s' % changeset) - else: - self.fail("URL didn't redirect") + changeset = self.openGitHubHook(1, 'alt').rstrip()[-40:] + self.assertRequestRedirects( + u('changeset', changeset, 'alt'), + 'https://github.com/follower/trac-github/commit/%s' % changeset + ) def testNonGitHubLinkToChangeset(self): changeset = self.makeGitCommit(NOGHGIT, 'myfile', 'for browser tests') @@ -350,27 +354,19 @@ def testNonGitHubLinkToChangeset(self): def testLinkToPath(self): self.makeGitCommit(GIT, 'myfile', 'for more browser tests') - changeset = self.openGitHubHook().read().rstrip()[-40:] - try: - urllib2.urlopen(u('changeset', changeset, 'myfile')) - except urllib2.HTTPError as exc: - self.assertEqual(exc.code, 302) - self.assertEqual(exc.headers['Location'], - 'https://github.com/aaugustin/trac-github/blob/%s/myfile' % changeset) - else: - self.fail("URL didn't redirect") + changeset = self.openGitHubHook().rstrip()[-40:] + self.assertRequestRedirects( + u('changeset', changeset, 'myfile'), + 'https://github.com/aaugustin/trac-github/blob/%s/myfile' % changeset + ) def testAlternateLinkToPath(self): self.makeGitCommit(ALTGIT, 'myfile', 'for more browser tests') - changeset = self.openGitHubHook(1, 'alt').read().rstrip()[-40:] - try: - urllib2.urlopen(u('changeset', changeset, 'alt/myfile')) - except urllib2.HTTPError as exc: - self.assertEqual(exc.code, 302) - self.assertEqual(exc.headers['Location'], - 'https://github.com/follower/trac-github/blob/%s/myfile' % changeset) - else: - self.fail("URL didn't redirect") + changeset = self.openGitHubHook(1, 'alt').rstrip()[-40:] + self.assertRequestRedirects( + u('changeset', changeset, 'alt/myfile'), + 'https://github.com/follower/trac-github/blob/%s/myfile' % changeset + ) def testNonGitHubLinkToPath(self): changeset = self.makeGitCommit(NOGHGIT, 'myfile', 'for more browser tests') From bd8fd09ce57d0472cd972d593bde152d06f1bd97 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Fri, 2 Feb 2024 00:30:10 +0100 Subject: [PATCH 03/10] [runtests] Converted usage of urllib2 to requests This should help with the py3 support down the line. --- runtests.py | 111 ++++++++++++++++++++++------------------------------ 1 file changed, 46 insertions(+), 65 deletions(-) diff --git a/runtests.py b/runtests.py index ea8b696..11f4982 100755 --- a/runtests.py +++ b/runtests.py @@ -25,7 +25,6 @@ import time import traceback import unittest -import urllib2 import urlparse from lxml import html @@ -59,14 +58,6 @@ GIT_DEFAULT_BRANCH = 'main' -class HttpNoRedirectHandler(urllib2.HTTPRedirectHandler): - - def redirect_request(self, req, fp, code, msg, headers, newurl): - raise urllib2.HTTPError(req.get_full_url(), code, msg, headers, fp) - -urllib2.install_opener(urllib2.build_opener(HttpNoRedirectHandler())) - - def d(*args): """ Return an absolute path where the given arguments are joined and @@ -252,8 +243,8 @@ def startTracd(cls, **kwargs): waittime = 0.1 for _ in range(5): try: - urllib2.urlopen(u()) - except urllib2.URLError: + requests.get(u()) + except requests.ConnectionError: time.sleep(waittime) waittime *= 2 else: @@ -312,8 +303,9 @@ def openGitHubHook(n=1, reponame='', payload=None): if not payload: payload = TracGitHubTests.makeGitHubHookPayload(n, reponame) url = u('github', reponame) if reponame else u('github') - request = urllib2.Request(url, json.dumps(payload), HEADERS) - return urllib2.urlopen(request) + response = requests.post(url, json=payload, headers=HEADERS, allow_redirects=False) + response.raise_for_status() + return response.text class GitHubBrowserTests(TracGitHubTests): @@ -322,13 +314,9 @@ def assertRequestRedirects(self, url, redirection, status=302): A custom assertion to check that requesting the given url (GET) returns a redirection response (302 by default) pointed to the given redirection URL. """ - try: - urllib2.urlopen(url) - except urllib2.HTTPError as exc: - self.assertEqual(exc.code, status) - self.assertEqual(exc.headers['Location'], redirection) - else: - self.fail("URL didn't redirect") + response = requests.get(url, allow_redirects=False) + self.assertEqual(response.status_code, status) + self.assertEqual(response.headers['Location'], redirection) def testLinkToChangeset(self): self.makeGitCommit(GIT, 'myfile', 'for browser tests') @@ -375,12 +363,12 @@ def testNonGitHubLinkToPath(self): self.assertEqual(response.status_code, 200) def testBadChangeset(self): - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): - urllib2.urlopen(u('changeset/1234567890')) + response = requests.get(u('changeset/1234567890')) + self.assertEqual(response.status_code, 404) def testBadUrl(self): - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): - urllib2.urlopen(u('changesetnosuchurl')) + response = requests.get(u('changesetnosuchurl')) + self.assertEqual(response.status_code, 404) def testTimelineFiltering(self): self.makeGitBranch(GIT, 'stable/2.0') @@ -395,7 +383,7 @@ def testTimelineFiltering(self): self.makeGitCommit(ALTGIT, 'myfile', 'timeline 6\n', 'msg 6', 'unstable/2.0') self.openGitHubHook(3) self.openGitHubHook(3, 'alt') - html = urllib2.urlopen(u('timeline')).read() + html = requests.get(u('timeline')).text self.assertTrue('msg 1' in html) self.assertTrue('msg 2' in html) self.assertTrue('msg 3' in html) @@ -439,7 +427,8 @@ def testOauthInvalidState(self): self.assertEqual(response.status_code, 302) response = session.get( - u('github/oauth') + '?code=01234567890123456789&state=wrong_state', + u('github/oauth'), + params={'code': '01234567890123456789', 'state': 'wrong_state'}, allow_redirects=False) self.assertEqual(response.status_code, 302) self.assertEqual(response.headers['Location'], u()) @@ -456,7 +445,7 @@ def testOauthInvalidStateWithoutSession(self): # OAuth callback requests without state must still fail. response = session.get( - u('github/oauth') + '?code=01234567890123456789', + u('github/oauth'), params={'code': '01234567890123456789'}, allow_redirects=False) self.assertEqual(response.status_code, 302) self.assertEqual(response.headers['Location'], u()) @@ -934,20 +923,20 @@ def testPreferredEmailCaseInsensitive(self): class GitHubPostCommitHookTests(TracGitHubTests): def testDefaultRepository(self): - output = self.openGitHubHook(0).read() + output = self.openGitHubHook(0) self.assertEqual(output, "Running hook on (default)\n" "* Updating clone\n" "* Synchronizing with clone\n") def testAlternativeRepository(self): - output = self.openGitHubHook(0, 'alt').read() + output = self.openGitHubHook(0, 'alt') self.assertEqual(output, "Running hook on alt\n" "* Updating clone\n" "* Synchronizing with clone\n") def testCommit(self): self.makeGitCommit(GIT, 'foo', 'foo content\n') - output = self.openGitHubHook().read() + output = self.openGitHubHook() self.assertRegexpMatches(output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" @@ -956,7 +945,7 @@ def testCommit(self): def testMultipleCommits(self): self.makeGitCommit(GIT, 'bar', 'bar content\n') self.makeGitCommit(GIT, 'bar', 'more bar content\n') - output = self.openGitHubHook(2).read() + output = self.openGitHubHook(2) self.assertRegexpMatches(output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" @@ -967,7 +956,7 @@ def testCommitOnBranch(self): self.makeGitCommit(ALTGIT, 'stable', 'stable branch\n', branch='stable/1.0') self.makeGitBranch(ALTGIT, 'unstable/1.0') self.makeGitCommit(ALTGIT, 'unstable', 'unstable branch\n', branch='unstable/1.0') - output = self.openGitHubHook(2, 'alt').read() + output = self.openGitHubHook(2, 'alt') self.assertRegexpMatches(output, r"Running hook on alt\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" @@ -978,9 +967,8 @@ def testUnknownCommit(self): # Emulate self.openGitHubHook to use a non-existent commit id random_id = ''.join(random.choice('0123456789abcdef') for _ in range(40)) payload = {'commits': [{'id': random_id, 'message': '', 'distinct': True}]} - request = urllib2.Request(u('github'), json.dumps(payload), HEADERS) - output = urllib2.urlopen(request).read() - self.assertRegexpMatches(output, r"Running hook on \(default\)\n" + response = requests.post(u('github'), json=payload, headers=HEADERS) + self.assertRegexpMatches(response.text, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Unknown commit [0-9a-f]{40}\n") @@ -1050,34 +1038,29 @@ def testComplexNotification(self): def testPing(self): payload = {'zen': "Readability counts."} headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping'} - request = urllib2.Request(u('github'), json.dumps(payload), headers) - output = urllib2.urlopen(request).read() - self.assertEqual(output, "Readability counts.") + response = requests.post(u('github'), json=payload, headers=headers) + self.assertEqual(response.text, "Readability counts.") def testUnknownEvent(self): headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'pull'} - request = urllib2.Request(u('github'), json.dumps({}), headers) - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): - urllib2.urlopen(request) + response = requests.post(u('github'), json={}, headers=headers) + self.assertEqual(response.status_code, 400) def testBadMethod(self): - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 405: Method Not Allowed$'): - urllib2.urlopen(u('github')) + response = requests.get(u('github')) + self.assertEqual(response.status_code, 405) def testBadPayload(self): - request = urllib2.Request(u('github'), 'foobar', HEADERS) - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): - urllib2.urlopen(request) + response = requests.post(u('github'), data='foobar', headers=HEADERS) + self.assertEqual(response.status_code, 400) def testBadRepository(self): - request = urllib2.Request(u('github/nosuchrepo'), '{}', HEADERS) - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 400: Bad Request$'): - urllib2.urlopen(request) + response = requests.post(u('github/nosuchrepo'), data='{}', headers=HEADERS) + self.assertEqual(response.status_code, 400) def testBadUrl(self): - request = urllib2.Request(u('githubnosuchurl'), '{}', HEADERS) - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 404: Not Found$'): - urllib2.urlopen(request) + response = requests.post(u('githubnosuchurl'), data='{}', headers=HEADERS) + self.assertEqual(response.status_code, 404) class GitHubPostCommitHookWithSignedWebHookTests(TracGitHubTests): @@ -1092,21 +1075,19 @@ def setUpClass(cls): def testUnsignedPing(self): payload = {'zen': "Readability counts."} headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping'} - request = urllib2.Request(u('github'), json.dumps(payload), headers) - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 403: Forbidden$'): - urllib2.urlopen(request).read() + response = requests.post(u('github'), json=payload, headers=headers) + self.assertEqual(response.status_code, 403) def testSignedPing(self): # Correct signature can be generated with OpenSSL: - # $> printf '{"zen": "Echo me"}\n' | openssl dgst -sha256 -hmac $webhook_secret + # $> printf '{"zen": "Echo me"}' | openssl dgst -sha256 -hmac $webhook_secret payload = {'zen': "Echo me"} - signature = "sha256=cacc93c2df1b21313e16d8690fc21e56229b6a9525e7016db38bdf9bad708fed" + signature = "sha256=68aef42937ffd24dd0b98d00d84b1551784c00932ab4ae22055a916264e194df" headers = {'Content-Type': 'application/json', 'X-GitHub-Event': 'ping', 'X-Hub-Signature': signature} - request = urllib2.Request(u('github'), json.dumps(payload) + '\n', headers) - output = urllib2.urlopen(request).read() - self.assertEqual(output, "Echo me") + response = requests.post(u('github'), json=payload, headers=headers) + self.assertEqual(response.text, "Echo me") class GitHubPostCommitHookWithUpdateHookTests(TracGitHubTests): @@ -1145,7 +1126,7 @@ def tearDownClass(cls): def testUpdateHook(self): self.makeGitCommit(GIT, 'foo', 'foo content\n') payload = self.makeGitHubHookPayload() - output = self.openGitHubHook(payload=payload).read() + output = self.openGitHubHook(payload=payload) self.assertRegexpMatches(output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" @@ -1157,15 +1138,15 @@ def testUpdateHookExecFailure(self): os.chmod(d(UPDATEHOOK), 0o644) self.makeGitCommit(GIT, 'bar', 'bar content\n') payload = self.makeGitHubHookPayload() - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 500: Internal Server Error$'): - output = self.openGitHubHook(payload=payload).read() + with self.assertRaisesRegexp(requests.HTTPError, r'^500 Server Error: Internal Server Error'): + output = self.openGitHubHook(payload=payload) def testUpdateHookFailure(self): self.createFailingUpdateHook() self.makeGitCommit(GIT, 'baz', 'baz content\n') payload = self.makeGitHubHookPayload() - with self.assertRaisesRegexp(urllib2.HTTPError, r'^HTTP Error 500: Internal Server Error$'): - output = self.openGitHubHook(payload=payload).read() + with self.assertRaisesRegexp(requests.HTTPError, r'^500 Server Error: Internal Server Error'): + output = self.openGitHubHook(payload=payload) class GitHubBrowserWithCacheTests(GitHubBrowserTests): From c75efa5ae5235925a1f94d90ace62694c4912245 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 09:28:04 +0100 Subject: [PATCH 04/10] [py3-compat] Use PyGIT.Storage.repo_path instead of shelling out to git --- tracext/github/__init__.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tracext/github/__init__.py b/tracext/github/__init__.py index 0dddb2b..cb1e05e 100644 --- a/tracext/github/__init__.py +++ b/tracext/github/__init__.py @@ -951,11 +951,11 @@ def process_request(self, req): output = u'Running hook on %s\n' % (reponame or '(default)') output += u'* Updating clone\n' - try: - git = repos.git.repo # GitRepository - except AttributeError: - git = repos.repos.git.repo # GitCachedRepository - git.remote('update', '--prune') + try: # GitRepository + storage = repos.git + except AttributeError: # GitCachedRepository + storage = repos.repos.git + storage.repo.remote('update', '--prune') # Ensure that repos.get_changeset can find the new changesets. output += u'* Synchronizing with clone\n' @@ -988,12 +988,11 @@ def process_request(self, req): status = 200 - git_dir = git.rev_parse('--git-dir').rstrip('\n') - hook = os.path.join(git_dir, 'hooks', 'trac-github-update') + hook = os.path.join(storage.repo_path, 'hooks', 'trac-github-update') if os.path.isfile(hook): output += u'* Running trac-github-update hook\n' try: - p = Popen(hook, cwd=git_dir, + p = Popen(hook, cwd=storage.repo_path, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=trac.util.compat.close_fds) except Exception as e: From 100cc6bd7a0c2e50e9ba9d29ef50325c33df0790 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 10:11:35 +0100 Subject: [PATCH 05/10] [py3-compat] Used __future__ version of print() --- runtests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runtests.py b/runtests.py index 11f4982..69c0cf4 100755 --- a/runtests.py +++ b/runtests.py @@ -8,6 +8,7 @@ Trac's testing framework isn't well suited for plugins, so we NIH'd a bit. """ +from __future__ import print_function import argparse import BaseHTTPServer @@ -2175,8 +2176,8 @@ def get_parser(): COVERAGE_BIN = os.path.join(options.virtualenv, 'bin', COVERAGE_BIN) TESTDIR = tempfile.mkdtemp(prefix='trac-github-test-') - print "Starting tests using temporary directory %r" % TESTDIR - print "Using git version %s" % git_check_output('--version').strip() + print("Starting tests using temporary directory %r" % TESTDIR) + print("Using git version %s" % git_check_output('--version').strip()) try: test_program = unittest.main(argv=[sys.argv[0]] + unittest_argv, exit=False) From d02ea6c9f56d977bd1513094f9812f39f11f5e9d Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 10:50:36 +0100 Subject: [PATCH 06/10] [py3-compat] Only use str values for configparser --- runtests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtests.py b/runtests.py index 69c0cf4..72a25db 100755 --- a/runtests.py +++ b/runtests.py @@ -172,7 +172,7 @@ def createTracEnvironment(cls, **kwargs): conf.set('github', 'alt.repository', 'follower/trac-github') conf.set('github', 'alt.branches', '%s stable/*' % GIT_DEFAULT_BRANCH) if 'request_email' in kwargs: - conf.set('github', 'request_email', kwargs['request_email']) + conf.set('github', 'request_email', str(kwargs['request_email'])) if 'preferred_email_domain' in kwargs: conf.set('github', 'preferred_email_domain', kwargs['preferred_email_domain']) if 'organization' in kwargs: From 6dc5a3471bb2bcbcfd14463313a43c201be59326 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 11:29:33 +0100 Subject: [PATCH 07/10] [py3-compat] Remove usage of dict.iteritems() The slightly higher memory usage on py2 should be acceptable and will make py3 compatibility easier --- runtests.py | 4 ++-- tracext/github/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runtests.py b/runtests.py index 72a25db..895109f 100755 --- a/runtests.py +++ b/runtests.py @@ -1203,7 +1203,7 @@ def do_GET(self): prev_link = '; rel="prev"'.format( self.headers['Host'], path, - '&'.join(('='.join((str(k), str(v))) for k, v in prevparams.iteritems())) + '&'.join(('='.join((str(k), str(v))) for k, v in prevparams.items())) ) links.append(prev_link) if length >= end: @@ -1212,7 +1212,7 @@ def do_GET(self): next_link = '; rel="next"'.format( self.headers['Host'], path, - '&'.join(('='.join((str(k), str(v))) for k, v in nextparams.iteritems())) + '&'.join(('='.join((str(k), str(v))) for k, v in nextparams.items())) ) links.append(next_link) if len(links) > 0: diff --git a/tracext/github/__init__.py b/tracext/github/__init__.py index cb1e05e..a26b43e 100644 --- a/tracext/github/__init__.py +++ b/tracext/github/__init__.py @@ -698,7 +698,7 @@ def _fetch_groups(self): # Return data data = collections.defaultdict(list) - for tname, tmembers in members.iteritems(): + for tname, tmembers in members.items(): self.log.debug("Team members for group %r: %r", tname, tmembers) # pylint: disable=no-member for member in tmembers: data[member].append(tname) From 657fd66c88132f8d999c2372cf5efc722e9a7d64 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 10:03:13 +0100 Subject: [PATCH 08/10] [py3-compat] Added py3 compatibility :tada: Hybrid py2/py3 on a single codebase is achieved by using the `six` library here and there (mostly for imports) and a few `if six.PY2` conditionals. CI will test all currently supported versions of Python (that means 3.8 -> 3.12), plus 2.7 and 3.7 (supported in Ubuntu 20.04) --- .github/workflows/runtests.yml | 23 +++++----- noxfile.py | 5 ++- runtests.py | 79 +++++++++++++++++++--------------- setup.py | 3 ++ tracext/github/__init__.py | 14 +++--- 5 files changed, 72 insertions(+), 52 deletions(-) diff --git a/.github/workflows/runtests.yml b/.github/workflows/runtests.yml index 8a7446f..16a44c8 100644 --- a/.github/workflows/runtests.yml +++ b/.github/workflows/runtests.yml @@ -14,14 +14,15 @@ jobs: - run: git config --global user.email runtest@localhost - run: nox --non-interactive --error-on-missing-interpreter --session runtests -- --git-default-branch=master -# runtests-py3: -# runs-on: ubuntu-latest -# steps: -# - uses: wntrblm/nox@2022.8.7 -# with: -# python-versions: "3.7" -# - uses: actions/checkout@v4 -# - run: git config --global user.name runtest -# - run: git config --global user.email runtest@localhost -# - run: git config --global init.defaultBranch main -# - run: nox --non-interactive --error-on-missing-interpreter --session runtests + runtests-py3: + runs-on: ubuntu-latest + + steps: + - uses: wntrblm/nox@2022.8.7 + with: + python-versions: "3.7, 3.8, 3.9, 3.10, 3.11, 3.12" # duplicated in noxfile.py + - uses: actions/checkout@v4 + - run: git config --global user.name runtest + - run: git config --global user.email runtest@localhost + - run: git config --global init.defaultBranch main + - run: nox --non-interactive --error-on-missing-interpreter --session runtests diff --git a/noxfile.py b/noxfile.py index d6156fb..806f022 100644 --- a/noxfile.py +++ b/noxfile.py @@ -4,13 +4,16 @@ if sys.version_info.major == 2: TRAC_VERSIONS = ["1.4.4", "1.2.6"] + PYTHON_VERSIONS = ["2.7"] else: TRAC_VERSIONS = ["1.6"] + PYTHON_VERSIONS = ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"] # duplicated in runtests.yml -@nox.session +@nox.session(python=PYTHON_VERSIONS) @nox.parametrize("trac", TRAC_VERSIONS) def runtests(session, trac): session.install("-r", "requirements_test.txt") session.install("Trac==%s" % trac) + session.run("python", "--version") session.run("python", "runtests.py", *session.posargs) diff --git a/runtests.py b/runtests.py index 895109f..5474e1a 100755 --- a/runtests.py +++ b/runtests.py @@ -11,8 +11,6 @@ from __future__ import print_function import argparse -import BaseHTTPServer -import ConfigParser import json import os import random @@ -26,7 +24,11 @@ import time import traceback import unittest -import urlparse + + +from six.moves.BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer +from six.moves.configparser import ConfigParser +from six.moves.urllib.parse import parse_qs, urlparse from lxml import html @@ -35,6 +37,7 @@ from trac.util.translation import _ import requests +import six GIT = 'test-git-foo' @@ -83,6 +86,9 @@ def git_check_output(*args, **kwargs): as a string. """ repo = kwargs.pop('repo', None) + kwargs.setdefault('text', True) + if six.PY2: + del kwargs['text'] if repo is None: cmdargs = ["git"] + list(args) @@ -136,9 +142,12 @@ def createTracEnvironment(cls, **kwargs): subprocess.check_output([TRAC_ADMIN_BIN, d(ENV), 'permission', 'add', 'anonymous', 'TRAC_ADMIN']) - conf = ConfigParser.ConfigParser() - with open(d(CONF), 'rb') as fp: - conf.readfp(fp) + conf = ConfigParser() + with open(d(CONF), 'r') as fp: + if six.PY2: + conf.readfp(fp) + else: + conf.read_file(fp) conf.add_section('components') conf.set('components', 'trac.versioncontrol.web_ui.browser.BrowserModule', 'disabled') @@ -210,7 +219,7 @@ def createTracEnvironment(cls, **kwargs): conf.set('trac', 'permission_policies', 'GitHubPolicy, %s' % old_permission_policies) - with open(d(CONF), 'wb') as fp: + with open(d(CONF), 'w') as fp: conf.write(fp) with open(d(HTDIGEST), 'w') as fp: @@ -269,7 +278,7 @@ def makeGitCommit(repo, path, content, message='edit', branch=None): if branch != GIT_DEFAULT_BRANCH: git_check_output('checkout', branch, repo=repo) - with open(d(repo, path), 'wb') as fp: + with open(d(repo, path), 'w') as fp: fp.write(content) git_check_output('add', path, repo=repo) git_check_output('commit', '-m', message, repo=repo) @@ -406,11 +415,11 @@ def testLogin(self): response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) - redirect_url = urlparse.urlparse(response.headers['Location']) + redirect_url = urlparse(response.headers['Location']) self.assertEqual(redirect_url.scheme, 'https') self.assertEqual(redirect_url.netloc, 'github.com') self.assertEqual(redirect_url.path, '/login/oauth/authorize') - params = urlparse.parse_qs(redirect_url.query, keep_blank_values=True) + params = parse_qs(redirect_url.query, keep_blank_values=True) state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': ['01234567890123456789'], @@ -490,7 +499,7 @@ def setUpClass(cls): cls.trac_env_broken = trac_env_broken cls.trac_env_broken_api = trac_env_broken_api - with open(d(SECRET), 'wb') as fp: + with open(d(SECRET), 'w') as fp: fp.write('98765432109876543210') @@ -505,11 +514,11 @@ def testLoginWithReqEmail(self): response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) - redirect_url = urlparse.urlparse(response.headers['Location']) + redirect_url = urlparse(response.headers['Location']) self.assertEqual(redirect_url.scheme, 'https') self.assertEqual(redirect_url.netloc, 'github.com') self.assertEqual(redirect_url.path, '/login/oauth/authorize') - params = urlparse.parse_qs(redirect_url.query, keep_blank_values=True) + params = parse_qs(redirect_url.query, keep_blank_values=True) state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': ['01234567890123456789'], @@ -527,11 +536,11 @@ def loginAndVerifyClientId(self, expected_client_id): response = requests.get(u('github/login'), allow_redirects=False) self.assertEqual(response.status_code, 302) - redirect_url = urlparse.urlparse(response.headers['Location']) + redirect_url = urlparse(response.headers['Location']) self.assertEqual(redirect_url.scheme, 'https') self.assertEqual(redirect_url.netloc, 'github.com') self.assertEqual(redirect_url.path, '/login/oauth/authorize') - params = urlparse.parse_qs(redirect_url.query, keep_blank_values=True) + params = parse_qs(redirect_url.query, keep_blank_values=True) state = params['state'][0] # this is a random value self.assertEqual(params, { 'client_id': [expected_client_id], @@ -636,8 +645,8 @@ def attemptValidOauth(self, testenv, callback, **kwargs): self.assertEqual(response.status_code, 302) # Extract the state from the redirect - redirect_url = urlparse.urlparse(response.headers['Location']) - params = urlparse.parse_qs(redirect_url.query, keep_blank_values=True) + redirect_url = urlparse(response.headers['Location']) + params = parse_qs(redirect_url.query, keep_blank_values=True) state = params['state'][0] # this is a random value response = session.get( u('github/oauth'), @@ -938,7 +947,7 @@ def testAlternativeRepository(self): def testCommit(self): self.makeGitCommit(GIT, 'foo', 'foo content\n') output = self.openGitHubHook() - self.assertRegexpMatches(output, r"Running hook on \(default\)\n" + six.assertRegex(self, output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Adding commit [0-9a-f]{40}\n") @@ -947,7 +956,7 @@ def testMultipleCommits(self): self.makeGitCommit(GIT, 'bar', 'bar content\n') self.makeGitCommit(GIT, 'bar', 'more bar content\n') output = self.openGitHubHook(2) - self.assertRegexpMatches(output, r"Running hook on \(default\)\n" + six.assertRegex(self, output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Adding commits [0-9a-f]{40}, [0-9a-f]{40}\n") @@ -958,7 +967,7 @@ def testCommitOnBranch(self): self.makeGitBranch(ALTGIT, 'unstable/1.0') self.makeGitCommit(ALTGIT, 'unstable', 'unstable branch\n', branch='unstable/1.0') output = self.openGitHubHook(2, 'alt') - self.assertRegexpMatches(output, r"Running hook on alt\n" + six.assertRegex(self, output, r"Running hook on alt\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Adding commit [0-9a-f]{40}\n" @@ -969,7 +978,7 @@ def testUnknownCommit(self): random_id = ''.join(random.choice('0123456789abcdef') for _ in range(40)) payload = {'commits': [{'id': random_id, 'message': '', 'distinct': True}]} response = requests.post(u('github'), json=payload, headers=HEADERS) - self.assertRegexpMatches(response.text, r"Running hook on \(default\)\n" + six.assertRegex(self, response.text, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Unknown commit [0-9a-f]{40}\n") @@ -1095,13 +1104,13 @@ class GitHubPostCommitHookWithUpdateHookTests(TracGitHubTests): @classmethod def createUpdateHook(cls): - with open(d(UPDATEHOOK), 'wb') as fp: + with open(d(UPDATEHOOK), 'w') as fp: # simple shell script to echo back all input fp.write("""#!/bin/sh\nexec cat""") os.fchmod(fp.fileno(), 0o755) def createFailingUpdateHook(cls): - with open(d(UPDATEHOOK), 'wb') as fp: + with open(d(UPDATEHOOK), 'w') as fp: fp.write("""#!/bin/sh\nexit 1""") os.fchmod(fp.fileno(), 0o755) @@ -1128,7 +1137,7 @@ def testUpdateHook(self): self.makeGitCommit(GIT, 'foo', 'foo content\n') payload = self.makeGitHubHookPayload() output = self.openGitHubHook(payload=payload) - self.assertRegexpMatches(output, r"Running hook on \(default\)\n" + six.assertRegex(self, output, r"Running hook on \(default\)\n" r"\* Updating clone\n" r"\* Synchronizing with clone\n" r"\* Adding commit [0-9a-f]{40}\n" @@ -1139,14 +1148,14 @@ def testUpdateHookExecFailure(self): os.chmod(d(UPDATEHOOK), 0o644) self.makeGitCommit(GIT, 'bar', 'bar content\n') payload = self.makeGitHubHookPayload() - with self.assertRaisesRegexp(requests.HTTPError, r'^500 Server Error: Internal Server Error'): + with six.assertRaisesRegex(self, requests.HTTPError, r'^500 Server Error: Internal Server Error'): output = self.openGitHubHook(payload=payload) def testUpdateHookFailure(self): self.createFailingUpdateHook() self.makeGitCommit(GIT, 'baz', 'baz content\n') payload = self.makeGitHubHookPayload() - with self.assertRaisesRegexp(requests.HTTPError, r'^500 Server Error: Internal Server Error'): + with six.assertRaisesRegex(self, requests.HTTPError, r'^500 Server Error: Internal Server Error'): output = self.openGitHubHook(payload=payload) @@ -1160,7 +1169,7 @@ class GitHubPostCommitHookWithCacheTests(GitHubPostCommitHookTests): cached_git = True -class GitHubAPIMock(BaseHTTPServer.BaseHTTPRequestHandler): +class GitHubAPIMock(BaseHTTPRequestHandler): def log_message(self, format, *args): # Visibly differentiate GitHub API mock logging from tracd logs sys.stderr.write("%s [%s] %s\n" % @@ -1221,7 +1230,7 @@ def do_GET(self): self.send_header("Content-Type", contenttype) self.end_headers() - self.wfile.write(json.dumps(answer)) + self.wfile.write(json.dumps(answer, ensure_ascii=True).encode('ascii')) def do_POST(self): md = self.server.mockdata @@ -1239,9 +1248,9 @@ def do_POST(self): chunk = self.rfile.read(chunk_size) if not chunk: break - L.append(chunk) + L.append(chunk.decode('ascii')) size_remaining -= len(L[-1]) - args = urlparse.parse_qs(''.join(L)) + args = parse_qs(''.join(L)) retcode = 404 answer = {} @@ -1255,7 +1264,7 @@ def do_POST(self): self.send_response(retcode) self.send_header("Content-Type", contenttype) self.end_headers() - self.wfile.write(json.dumps(answer)) + self.wfile.write(json.dumps(answer, ensure_ascii=True).encode('ascii')) class TracContext(object): @@ -1836,7 +1845,7 @@ def test_014_hook_membership_event_add_team(self): self.assertGreater(len(data), 0, "No groups returned after update") self.assertIn(users[0]["login"], data, "User %s expected after update, but not present" % users[0]["login"]) - self.assertItemsEqual( + six.assertCountEqual(self, data[users[0]["login"]], (u"github-%s-justice-league" % self.organization, u"github-%s" % self.organization), "User %s does not have expected groups after update" % users[0]["login"]) @@ -1901,7 +1910,7 @@ def test_015_hook_membership_event_add_member(self): self.assertGreater(len(data), 0, "No groups returned after update") self.assertIn(users[1]["login"], data, "User %s expected after update, but not present" % users[1]["login"]) - self.assertItemsEqual( + six.assertCountEqual(self, data[users[1]["login"]], (u"github-%s-justice-league" % self.organization, u"github-%s" % self.organization), "User %s does not have expected groups after update" % users[1]["login"]) @@ -2113,7 +2122,7 @@ def updateMockData(md, retcode=None, contenttype=None, answers=None, JSON-encoded and returned for requests to the paths. :param postcallback: A callback function called for the next POST requests. Arguments are the requested path and a dict of POST - data as returned by `urlparse.parse_qs()`. The + data as returned by `parse_qs()`. The callback should return a tuple `(retcode, answer)` where `retcode` is the HTTP return code and `answer` will be JSON-encoded and sent to the client. Note that @@ -2148,7 +2157,7 @@ def apiMockServer(port, mockdata): be JSON-encoded and returned. Use `updateMockData()` to update the contents of the mockdata dict. """ - httpd = BaseHTTPServer.HTTPServer(('127.0.0.1', port), GitHubAPIMock) + httpd = HTTPServer(('127.0.0.1', port), GitHubAPIMock) # Make mockdata available to server httpd.mockdata = mockdata httpd.serve_forever() diff --git a/setup.py b/setup.py index 4000196..68dd80c 100644 --- a/setup.py +++ b/setup.py @@ -18,6 +18,9 @@ namespace_packages=['tracext'], platforms='all', license='BSD', + install_requires=[ + 'six==1.16.0', + ], extras_require={'oauth': ['requests_oauthlib >= 0.5']}, entry_points={'trac.plugins': [ 'github.browser = tracext.github:GitHubBrowser', diff --git a/tracext/github/__init__.py b/tracext/github/__init__.py index a26b43e..9dad4bd 100644 --- a/tracext/github/__init__.py +++ b/tracext/github/__init__.py @@ -29,6 +29,8 @@ from trac.web.auth import LoginModule from trac.web.chrome import add_warning +import six + def _config_secret(value): if re.match(r'[A-Z_]+', value): return os.environ.get(value, '') @@ -330,9 +332,11 @@ def __init__(self, api, env, fullname): self.api = api self.env = env self.name = fullname - # _fullname needs to be a string, not a unicode string, otherwise the - # cache object won't convert it into a hash. - self._fullname = fullname.encode('utf-8') + self._fullname = fullname + if six.PY2: + # Trac's @cached for py2 does an isinstance(..., str) so we need a native + # string type + self._fullname = fullname.encode('utf-8') # next try: immediately self._next_update = datetime.now() - timedelta(seconds=10) self._cached_result = self._apiresult_error() @@ -656,10 +660,10 @@ def github_api(self, url, *args): additional positional arguments. """ import requests - import urllib + from six.moves.urllib.parse import quote github_api_url = os.environ.get("TRAC_GITHUB_API_URL", "https://api.github.com/") - formatted_url = github_api_url + url.format(*(urllib.quote(str(x)) for x in args)) + formatted_url = github_api_url + url.format(*(quote(str(x)) for x in args)) access_token = _config_secret(self.access_token) self.log.debug("Hitting GitHub API endpoint %s with user %s", formatted_url, self.username) # pylint: disable=no-member results = [] From eccc5bac23bf7abd5e29505e5a422d4ad8b7d4d1 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 11:14:28 +0100 Subject: [PATCH 09/10] Added Python3 classifiers in setup.py --- setup.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/setup.py b/setup.py index 68dd80c..1162001 100644 --- a/setup.py +++ b/setup.py @@ -30,4 +30,16 @@ ]}, test_suite='runtests', tests_require='lxml', + classifiers=[ + 'Programming Language :: Python', + 'Programming Language :: Python :: 2.7', + 'Programming Language :: Python :: 3', + 'Programming Language :: Python :: 3.7', + 'Programming Language :: Python :: 3.8', + 'Programming Language :: Python :: 3.9', + 'Programming Language :: Python :: 3.10', + 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', + 'Framework :: Trac', + ], ) From e5e949dcf6f1213f3a70038d57737246bcd61cba Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 3 Feb 2024 12:39:37 +0100 Subject: [PATCH 10/10] Added release note about py3 support --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 35537d4..fe23d08 100644 --- a/README.md +++ b/README.md @@ -518,6 +518,7 @@ Changelog * Add configuration option for path prefix of login and logout. (#127) * Add `GitHubPolicy` permission policy to make `[timeline]` `changeset_show_file` option work correctly. (#126) +* Support for Python3 (tested with 3.7 - 3.12) (requires Trac >= 1.6) ### 2.3