diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 0a78af3c527a96c56992b94fe8579f474ded6f7b..29ee9c0479c1de6d044735f51ac02f6ba94fe3a9 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -655,8 +655,12 @@ def _Approvers(input_api, email_regexp): if not input_api.change.issue: return [] - path = '/api/%s?messages=true' - url = (input_api.host_url + path) % input_api.change.issue + # TODO(dpranke): Should figure out if input_api.host_url is supposed to + # be a host or a scheme+host and normalize it there. + host = input_api.host_url + if not host.startswith('http://') and not host.startswith('https://'): + host = 'http://' + host + url = '%s/api/%s?messages=true' % (host, input_api.change.issue) f = input_api.urllib2.urlopen(url) issue_props = input_api.json.load(f) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 1d390bd8dfad296413dfb996708ad8787fa70809..3bf7d79e317d9c848693e8f0f47f52160e2501aa 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1867,13 +1867,19 @@ mac|success|blew def OwnersTest(self, is_committing, tbr=False, change_tags=None, suggested_reviewers=None, approvers=None, - uncovered_files=None, expected_reviewers=None, expected_output=''): + uncovered_files=None, expected_reviewers=None, expected_output='', + host_url=None): affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile) affected_file.LocalPath().AndReturn('foo.cc') change = self.mox.CreateMock(presubmit.Change) change.AffectedFiles(None).AndReturn([affected_file]) input_api = self.MockInputApi(change, False) + expected_host = 'http://localhost' + if host_url: + input_api.host_url = host_url + if host_url.startswith('https'): + expected_host = host_url fake_db = self.mox.CreateMock(owners.Database) fake_db.email_regexp = input_api.re.compile(owners.BASIC_EMAIL_REGEXP) input_api.owners_db = fake_db @@ -1887,7 +1893,7 @@ mac|success|blew rietveld_response = ('{"owner": "john@example.com",' '"messages": [' + ','.join(messages) + ']}') input_api.urllib2.urlopen( - input_api.host_url + '/api/1?messages=true').AndReturn( + expected_host + '/api/1?messages=true').AndReturn( StringIO.StringIO(rietveld_response)) input_api.json = presubmit.json fake_db.files_not_covered_by(set(['foo.cc']), approvers).AndReturn( @@ -1937,6 +1943,17 @@ mac|success|blew uncovered_files=set(), expected_output='--tbr was specified, skipping OWNERS check\n') + def testCannedCheckOwners_MissingSchemeInHostURL(self): + self.OwnersTest(is_committing=True, + approvers=set(['ben@example.com']), + uncovered_files=set(), host_url='localhost') + + def testCannedCheckOwners_HTTPS_HostURL(self): + self.OwnersTest(is_committing=True, + approvers=set(['ben@example.com']), + uncovered_files=set(), host_url='https://localhost') + + if __name__ == '__main__': import unittest unittest.main()