Commit a561166a authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot

[WPT Importer] Switch to new rotation endpoint for TBR

This also changes to use full email addresses instead of usernames,
which is (coincidentally) a concern raised by Pinpoint team in
issue 1045089.

This change is being deliberately TBR'd (after review!) to confirm
that TBR-ing an @google.com account works.

TBR=kyleju@google.com

Bug: None
Change-Id: I472c8fcdfba0aae9d2ff75d34a59e287905b50f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2226769Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774645}
parent fbe2e324
...@@ -38,8 +38,8 @@ POLL_DELAY_SECONDS = 2 * 60 ...@@ -38,8 +38,8 @@ POLL_DELAY_SECONDS = 2 * 60
TIMEOUT_SECONDS = 210 * 60 TIMEOUT_SECONDS = 210 * 60
# Sheriff calendar URL, used for getting the ecosystem infra sheriff to TBR. # Sheriff calendar URL, used for getting the ecosystem infra sheriff to TBR.
ROTATIONS_URL = 'https://rota-ng.appspot.com/legacy/all_rotations.js' ROTATIONS_URL = 'https://chrome-ops-rotation-proxy.appspot.com/current/grotation:chrome-ecosystem-infra'
TBR_FALLBACK = 'robertma' TBR_FALLBACK = 'robertma@google.com'
_log = logging.getLogger(__file__) _log = logging.getLogger(__file__)
...@@ -562,42 +562,34 @@ class TestImporter(object): ...@@ -562,42 +562,34 @@ class TestImporter(object):
return '\n'.join(message_lines) return '\n'.join(message_lines)
def tbr_reviewer(self): def tbr_reviewer(self):
"""Returns the user name or email address to use as the reviewer. """Returns the email address to use as the reviewer.
This tries to fetch the current ecosystem infra sheriff, but falls back This tries to fetch the current ecosystem infra sheriff, but falls back
in case of error. in case of error.
Either a user name (which is assumed to have a chromium.org email
address) or a full email address (for other cases) is returned.
""" """
username = '' email = ''
try: try:
username = self._fetch_ecosystem_infra_sheriff_username() email = self._fetch_ecosystem_infra_sheriff_email()
except (IOError, KeyError, ValueError) as error: except (IOError, KeyError, ValueError) as error:
_log.error('Exception while fetching current sheriff: %s', error) _log.error('Exception while fetching current sheriff: %s', error)
if username in ['kyleju']: if email in ['kyleju@google.com']:
_log.warning('Cannot TBR by %s: not a committer', username) _log.warning('Cannot TBR by %s: not a committer', email)
username = '' email = ''
return username or TBR_FALLBACK return email or TBR_FALLBACK
def _fetch_ecosystem_infra_sheriff_username(self): def _fetch_ecosystem_infra_sheriff_email(self):
try: try:
content = self.host.web.get_binary(ROTATIONS_URL) content = self.host.web.get_binary(ROTATIONS_URL)
except NetworkTimeout: except NetworkTimeout:
_log.error('Cannot fetch %s', ROTATIONS_URL) _log.error('Cannot fetch %s', ROTATIONS_URL)
return '' return ''
data = json.loads(content) data = json.loads(content)
today = datetime.date.fromtimestamp(self.host.time()).isoformat() if not data.get('emails'):
index = data['rotations'].index('ecosystem_infra') _log.error(
calendar = data['calendar'] 'No email found for current sheriff. Retrieved content: %s',
for entry in calendar: content)
if entry['date'] == today:
if not entry['participants'][index]:
_log.info('No sheriff today.')
return ''
return entry['participants'][index][0]
_log.error('No entry found for date %s in rotations table.', today)
return '' return ''
return data['emails'][0]
def fetch_new_expectations_and_baselines(self): def fetch_new_expectations_and_baselines(self):
"""Modifies expectation lines and baselines based on try job results. """Modifies expectation lines and baselines based on try job results.
......
...@@ -465,43 +465,30 @@ class TestImporterTest(LoggingTestCase): ...@@ -465,43 +465,30 @@ class TestImporterTest(LoggingTestCase):
'No JSON object could be decoded\n' 'No JSON object could be decoded\n'
]) ])
def test_tbr_reviewer_date_not_found(self): def test_tbr_reviewer_no_emails_field(self):
host = MockHost() host = MockHost()
yesterday = (datetime.date.fromtimestamp(host.time()) - host.web.urls[ROTATIONS_URL] = json.dumps(
datetime.timedelta(days=1)).isoformat() {'updated_unix_timestamp': '1591108191'})
host.web.urls[ROTATIONS_URL] = json.dumps({
'calendar': [
{
'date': yesterday,
'participants': [['some-sheriff'], ['other-sheriff']],
},
],
'rotations': ['ecosystem_infra', 'other_rotation']
})
importer = TestImporter(host) importer = TestImporter(host)
self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer()) self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer())
# Use a variable here, otherwise we get different values depending on
# the machine's time zone settings (e.g. "1969-12-31" vs "1970-01-01").
today = datetime.date.fromtimestamp(host.time()).isoformat()
self.assertLog([ self.assertLog([
'ERROR: No entry found for date %s in rotations table.\n' % today 'ERROR: No email found for current sheriff. Retrieved content: %s\n'
% host.web.urls[ROTATIONS_URL]
]) ])
def test_tbr_reviewer_nobody_on_rotation(self): def test_tbr_reviewer_nobody_on_rotation(self):
host = MockHost() host = MockHost()
today = datetime.date.fromtimestamp(host.time()).isoformat()
host.web.urls[ROTATIONS_URL] = json.dumps({ host.web.urls[ROTATIONS_URL] = json.dumps({
'calendar': [ 'emails': [],
{ 'updated_unix_timestamp':
'date': today, '1591108191'
'participants': [[], ['some-sheriff']],
},
],
'rotations': ['ecosystem_infra', 'other-rotation']
}) })
importer = TestImporter(host) importer = TestImporter(host)
self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer()) self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer())
self.assertLog(['INFO: No sheriff today.\n']) self.assertLog([
'ERROR: No email found for current sheriff. Retrieved content: %s\n'
% host.web.urls[ROTATIONS_URL]
])
def test_tbr_reviewer_rotations_url_unavailable(self): def test_tbr_reviewer_rotations_url_unavailable(self):
def raise_exception(*_): def raise_exception(*_):
...@@ -515,47 +502,23 @@ class TestImporterTest(LoggingTestCase): ...@@ -515,47 +502,23 @@ class TestImporterTest(LoggingTestCase):
def test_tbr_reviewer(self): def test_tbr_reviewer(self):
host = MockHost() host = MockHost()
today = datetime.date.fromtimestamp(host.time())
yesterday = today - datetime.timedelta(days=1)
host.web.urls[ROTATIONS_URL] = json.dumps({ host.web.urls[ROTATIONS_URL] = json.dumps({
'calendar': [ 'emails': ['current-sheriff@chromium.org'],
{ 'updated_unix_timestamp':
'date': yesterday.isoformat(), '1591108191',
'participants': [['other-sheriff'], ['last-sheriff']],
},
{
'date': today.isoformat(),
'participants': [['other-sheriff'], ['current-sheriff']],
},
],
'rotations': ['other-rotation', 'ecosystem_infra']
})
importer = TestImporter(host)
self.assertEqual('current-sheriff', importer.tbr_reviewer())
self.assertLog([])
def test_tbr_reviewer_with_full_email_address(self):
host = MockHost()
today = datetime.date.fromtimestamp(host.time()).isoformat()
host.web.urls[ROTATIONS_URL] = json.dumps({
'calendar': [
{
'date': today,
'participants': [['external@example.com']],
},
],
'rotations': ['ecosystem_infra']
}) })
importer = TestImporter(host) importer = TestImporter(host)
self.assertEqual('external@example.com', importer.tbr_reviewer()) self.assertEqual('current-sheriff@chromium.org',
importer.tbr_reviewer())
self.assertLog([]) self.assertLog([])
def test_tbr_reviewer_skips_non_committer(self): def test_tbr_reviewer_skips_non_committer(self):
host = MockHost() host = MockHost()
importer = TestImporter(host) importer = TestImporter(host)
importer._fetch_ecosystem_infra_sheriff_username = lambda: 'kyleju' importer._fetch_ecosystem_infra_sheriff_email = lambda: 'kyleju@google.com'
self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer()) self.assertEqual(TBR_FALLBACK, importer.tbr_reviewer())
self.assertLog(['WARNING: Cannot TBR by kyleju: not a committer\n']) self.assertLog(
['WARNING: Cannot TBR by kyleju@google.com: not a committer\n'])
def test_generate_manifest_successful_run(self): def test_generate_manifest_successful_run(self):
# This test doesn't test any aspect of the real manifest script, it just # This test doesn't test any aspect of the real manifest script, it just
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment