Commit 6cdda3e6 authored by Quinten Yearsley's avatar Quinten Yearsley Committed by Commit Bot

In wpt-importer, look only at the latest CQ job results.

Previously, the importer would sometimes abandon a potential import
even when it passed CQ if there were any failing (flaky) jobs.

This CL should make it so that when deciding whether the CQ passed,
importer looks only at the latest builds for each builder.

Bug: 739119
Change-Id: I126e33191ddc24797501f97ff0834e3f9e45d4bc
Reviewed-on: https://chromium-review.googlesource.com/578141
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: default avatarJeff Carpenter <jeffcarp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488453}
parent cde9497a
...@@ -82,7 +82,7 @@ class GitCL(object): ...@@ -82,7 +82,7 @@ class GitCL(object):
self._host.sleep(poll_delay_seconds) self._host.sleep(poll_delay_seconds)
try_results = self.try_job_results() try_results = self.try_job_results()
_log.debug('Fetched try results: %s', try_results) _log.debug('Fetched try results: %s', try_results)
if self.all_jobs_finished(try_results): if self.all_finished(try_results):
self._host.print_('All jobs finished.') self._host.print_('All jobs finished.')
return try_results return try_results
self._host.print_('Waiting. %d seconds passed.' % (self._host.time() - start)) self._host.print_('Waiting. %d seconds passed.' % (self._host.time() - start))
...@@ -91,7 +91,7 @@ class GitCL(object): ...@@ -91,7 +91,7 @@ class GitCL(object):
return None return None
def latest_try_jobs(self, builder_names=None): def latest_try_jobs(self, builder_names=None):
"""Returns a dict of Builds to results for the latest try jobs. """Fetches a dict of Build to TryJobStatus for the latest try jobs.
This includes jobs that are not yet finished and builds with infra This includes jobs that are not yet finished and builds with infra
failures, so if a build is in this list, that doesn't guarantee that failures, so if a build is in this list, that doesn't guarantee that
...@@ -101,14 +101,15 @@ class GitCL(object): ...@@ -101,14 +101,15 @@ class GitCL(object):
builder_names: Optional list of builders used to filter results. builder_names: Optional list of builders used to filter results.
Returns: Returns:
A dict mapping Build objects to TryJobSTatus objects, with A dict mapping Build objects to TryJobStatus objects, with
only the latest jobs included. only the latest jobs included.
""" """
try_results = self.try_job_results(builder_names) return self.filter_latest(self.try_job_results(builder_names))
builds = try_results.keys()
if builder_names: @staticmethod
builds = [b for b in builds if b.builder_name in builder_names] def filter_latest(try_results):
latest_builds = filter_latest_builds(builds) """Returns the latest entries from from a Build to TryJobStatus dict."""
latest_builds = filter_latest_builds(try_results.keys())
return {b: s for b, s in try_results.items() if b in latest_builds} return {b: s for b, s in try_results.items() if b in latest_builds}
def try_job_results(self, builder_names=None): def try_job_results(self, builder_names=None):
...@@ -158,9 +159,14 @@ class GitCL(object): ...@@ -158,9 +159,14 @@ class GitCL(object):
return TryJobStatus(result_dict['status'], result_dict['result']) return TryJobStatus(result_dict['status'], result_dict['result'])
@staticmethod @staticmethod
def all_jobs_finished(try_results): def all_finished(try_results):
return all(s.status == 'COMPLETED' for s in try_results.values()) return all(s.status == 'COMPLETED' for s in try_results.values())
@staticmethod @staticmethod
def has_failing_try_results(try_results): def all_success(try_results):
return all(s.status == 'COMPLETED' and s.result == 'SUCCESS'
for s in try_results.values())
@staticmethod
def some_failed(try_results):
return any(s.result == 'FAILURE' for s in try_results.values()) return any(s.result == 'FAILURE' for s in try_results.values())
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
from webkitpy.common.net.buildbot import filter_latest_builds from webkitpy.common.net.git_cl import GitCL
# TODO(qyearsley): Use this class in wpt_expectations_updater_unittest and rebaseline_cl_unittest. # TODO(qyearsley): Use this class in wpt_expectations_updater_unittest and rebaseline_cl_unittest.
...@@ -27,20 +27,27 @@ class MockGitCL(object): ...@@ -27,20 +27,27 @@ class MockGitCL(object):
def get_issue_number(self): def get_issue_number(self):
return '1234' return '1234'
def try_job_results(self, **_):
return self._results
def wait_for_try_jobs(self, **_): def wait_for_try_jobs(self, **_):
return self._results return self._results
def latest_try_jobs(self, **_): def latest_try_jobs(self, **_):
latest_builds = filter_latest_builds(self._results) return self.filter_latest(self._results)
return {b: s for b, s in self._results.items() if b in latest_builds}
def try_job_results(self, **_): @staticmethod
return self._results def filter_latest(try_results):
return GitCL.filter_latest(try_results)
@staticmethod
def all_finished(try_results):
return GitCL.all_finished(try_results)
@staticmethod @staticmethod
def all_jobs_finished(results): def all_success(try_results):
return all(s.status == 'COMPLETED' for s in results.values()) return GitCL.all_success(try_results)
@staticmethod @staticmethod
def has_failing_try_results(results): def some_failed(try_results):
return any(s.result == 'FAILURE' for s in results.values()) return GitCL.some_failed(try_results)
...@@ -90,19 +90,33 @@ class GitCLTest(unittest.TestCase): ...@@ -90,19 +90,33 @@ class GitCLTest(unittest.TestCase):
'All jobs finished.\n') 'All jobs finished.\n')
def test_has_failing_try_results_empty(self): def test_has_failing_try_results_empty(self):
self.assertFalse(GitCL.has_failing_try_results({})) self.assertFalse(GitCL.some_failed({}))
def test_has_failing_try_results_only_success_and_started(self): def test_has_failing_try_results_only_success_and_started(self):
self.assertFalse(GitCL.has_failing_try_results({ self.assertFalse(GitCL.some_failed({
Build('some-builder', 90): TryJobStatus('COMPLETED', 'SUCCESS'), Build('some-builder', 90): TryJobStatus('COMPLETED', 'SUCCESS'),
Build('some-builder', 100): TryJobStatus('STARTED'), Build('some-builder', 100): TryJobStatus('STARTED'),
})) }))
def test_has_failing_try_results_with_failing_results(self): def test_has_failing_try_results_with_failing_results(self):
self.assertTrue(GitCL.has_failing_try_results({ self.assertTrue(GitCL.some_failed({
Build('some-builder', 1): TryJobStatus('COMPLETED', 'FAILURE'), Build('some-builder', 1): TryJobStatus('COMPLETED', 'FAILURE'),
})) }))
def test_all_success_empty(self):
self.assertTrue(GitCL.all_success({}))
def test_all_success_true(self):
self.assertTrue(GitCL.all_success({
Build('some-builder', 1): TryJobStatus('COMPLETED', 'SUCCESS'),
}))
def test_all_success_with_started_build(self):
self.assertFalse(GitCL.all_success({
Build('some-builder', 1): TryJobStatus('COMPLETED', 'SUCCESS'),
Build('some-builder', 2): TryJobStatus('STARTED'),
}))
def test_latest_try_builds(self): def test_latest_try_builds(self):
git_cl = GitCL(MockHost()) git_cl = GitCL(MockHost())
git_cl.fetch_raw_try_job_results = lambda: [ git_cl.fetch_raw_try_job_results = lambda: [
...@@ -177,6 +191,20 @@ class GitCLTest(unittest.TestCase): ...@@ -177,6 +191,20 @@ class GitCLTest(unittest.TestCase):
Build('builder-b', 200): TryJobStatus('COMPLETED', 'FAILURE'), Build('builder-b', 200): TryJobStatus('COMPLETED', 'FAILURE'),
}) })
def test_filter_latest(self):
try_job_results = {
Build('builder-a', 100): TryJobStatus('COMPLETED', 'FAILURE'),
Build('builder-a', 200): TryJobStatus('COMPLETED', 'SUCCESS'),
Build('builder-b', 50): TryJobStatus('SCHEDULED'),
}
self.assertEqual(
GitCL.filter_latest(try_job_results),
{
Build('builder-a', 200): TryJobStatus('COMPLETED', 'SUCCESS'),
Build('builder-b', 50): TryJobStatus('SCHEDULED'),
})
def test_try_job_results_with_task_id_in_url(self): def test_try_job_results_with_task_id_in_url(self):
git_cl = GitCL(MockHost()) git_cl = GitCL(MockHost())
git_cl.fetch_raw_try_job_results = lambda: [ git_cl.fetch_raw_try_job_results = lambda: [
......
...@@ -63,6 +63,7 @@ class TestImporter(object): ...@@ -63,6 +63,7 @@ class TestImporter(object):
self.git_cl = GitCL(self.host, auth_refresh_token_json=options.auth_refresh_token_json) self.git_cl = GitCL(self.host, auth_refresh_token_json=options.auth_refresh_token_json)
_log.debug('Noting the current Chromium commit.') _log.debug('Noting the current Chromium commit.')
# TODO(qyearsley): Use Git (self.host.git) to run git commands.
_, show_ref_output = self.run(['git', 'show-ref', 'HEAD']) _, show_ref_output = self.run(['git', 'show-ref', 'HEAD'])
chromium_commit = show_ref_output.split()[0] chromium_commit = show_ref_output.split()[0]
...@@ -90,8 +91,6 @@ class TestImporter(object): ...@@ -90,8 +91,6 @@ class TestImporter(object):
_log.info('Checking out %s', options.revision) _log.info('Checking out %s', options.revision)
self.run(['git', 'checkout', options.revision], cwd=local_wpt.path) self.run(['git', 'checkout', options.revision], cwd=local_wpt.path)
self.run(['git', 'submodule', 'update', '--init', '--recursive'], cwd=local_wpt.path)
_log.info('Noting the revision we are importing.') _log.info('Noting the revision we are importing.')
_, show_ref_output = self.run(['git', 'show-ref', 'origin/master'], cwd=local_wpt.path) _, show_ref_output = self.run(['git', 'show-ref', 'origin/master'], cwd=local_wpt.path)
import_commit = 'wpt@%s' % show_ref_output.split()[0] import_commit = 'wpt@%s' % show_ref_output.split()[0]
...@@ -163,7 +162,7 @@ class TestImporter(object): ...@@ -163,7 +162,7 @@ class TestImporter(object):
self.git_cl.run(['set-close']) self.git_cl.run(['set-close'])
return False return False
if try_results and self.git_cl.has_failing_try_results(try_results): if try_results and self.git_cl.some_failed(try_results):
self.fetch_new_expectations_and_baselines() self.fetch_new_expectations_and_baselines()
if self.host.git().has_working_directory_changes(): if self.host.git().has_working_directory_changes():
message = 'Update test expectations and baselines.' message = 'Update test expectations and baselines.'
...@@ -178,14 +177,14 @@ class TestImporter(object): ...@@ -178,14 +177,14 @@ class TestImporter(object):
try_results = self.git_cl.wait_for_try_jobs( try_results = self.git_cl.wait_for_try_jobs(
poll_delay_seconds=POLL_DELAY_SECONDS, poll_delay_seconds=POLL_DELAY_SECONDS,
timeout_seconds=TIMEOUT_SECONDS) timeout_seconds=TIMEOUT_SECONDS)
try_results = self.git_cl.filter_latest(try_results)
if not try_results: if not try_results:
self.git_cl.run(['set-close']) self.git_cl.run(['set-close'])
_log.error('No CQ try job results, aborting.') _log.error('No CQ try job results, aborting.')
return False return False
# TODO(qyearsley): Change this to look only at the latest try jobs; crbug.com/739119 if try_results and self.git_cl.all_success(try_results):
if try_results and all(s == TryJobStatus('COMPLETED', 'SUCCESS') for _, s in try_results.iteritems()):
_log.info('CQ appears to have passed; trying to commit.') _log.info('CQ appears to have passed; trying to commit.')
self.git_cl.run(['upload', '-f', '--send-mail']) # Turn off WIP mode. self.git_cl.run(['upload', '-f', '--send-mail']) # Turn off WIP mode.
self.git_cl.run(['set-commit']) self.git_cl.run(['set-commit'])
......
...@@ -87,7 +87,7 @@ class TestImporterTest(LoggingTestCase): ...@@ -87,7 +87,7 @@ class TestImporterTest(LoggingTestCase):
'/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '') '/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '')
importer = TestImporter(host) importer = TestImporter(host)
importer.git_cl = MockGitCL(host, results={ importer.git_cl = MockGitCL(host, results={
Build('builder-a', 123): TryJobStatus('COMPLETED', 'SUCCESS') Build('builder-a', 123): TryJobStatus('COMPLETED', 'SUCCESS'),
}) })
success = importer.update_expectations_for_cl() success = importer.update_expectations_for_cl()
self.assertTrue(success) self.assertTrue(success)
...@@ -98,7 +98,7 @@ class TestImporterTest(LoggingTestCase): ...@@ -98,7 +98,7 @@ class TestImporterTest(LoggingTestCase):
'/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '') '/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '')
importer = TestImporter(host) importer = TestImporter(host)
importer.git_cl = MockGitCL(host, results={ importer.git_cl = MockGitCL(host, results={
Build('builder-a', 123): TryJobStatus('COMPLETED', 'FAILURE') Build('builder-a', 123): TryJobStatus('COMPLETED', 'FAILURE'),
}) })
importer.fetch_new_expectations_and_baselines = lambda: None importer.fetch_new_expectations_and_baselines = lambda: None
success = importer.update_expectations_for_cl() success = importer.update_expectations_for_cl()
...@@ -112,8 +112,10 @@ class TestImporterTest(LoggingTestCase): ...@@ -112,8 +112,10 @@ class TestImporterTest(LoggingTestCase):
host.filesystem.write_text_file( host.filesystem.write_text_file(
'/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '') '/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '')
importer = TestImporter(host) importer = TestImporter(host)
# Only the latest job for each builder is counted.
importer.git_cl = MockGitCL(host, results={ importer.git_cl = MockGitCL(host, results={
Build('builder-a', 123): TryJobStatus('COMPLETED', 'SUCCESS') Build('builder-a', 120): TryJobStatus('COMPLETED', 'FAILURE'),
Build('builder-a', 123): TryJobStatus('COMPLETED', 'SUCCESS'),
}) })
success = importer.run_commit_queue_for_cl() success = importer.run_commit_queue_for_cl()
self.assertTrue(success) self.assertTrue(success)
...@@ -134,7 +136,9 @@ class TestImporterTest(LoggingTestCase): ...@@ -134,7 +136,9 @@ class TestImporterTest(LoggingTestCase):
'/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '') '/mock-checkout/third_party/WebKit/LayoutTests/W3CImportExpectations', '')
importer = TestImporter(host) importer = TestImporter(host)
importer.git_cl = MockGitCL(host, results={ importer.git_cl = MockGitCL(host, results={
Build('builder-a', 123): TryJobStatus('COMPLETED', 'FAILURE') Build('builder-a', 120): TryJobStatus('COMPLETED', 'SUCCESS'),
Build('builder-a', 123): TryJobStatus('COMPLETED', 'FAILURE'),
Build('builder-b', 200): TryJobStatus('COMPLETED', 'SUCCESS'),
}) })
importer.fetch_new_expectations_and_baselines = lambda: None importer.fetch_new_expectations_and_baselines = lambda: None
success = importer.run_commit_queue_for_cl() success = importer.run_commit_queue_for_cl()
......
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