[autotest] Check the database even on job abort.
If a job aborts either during or just after it's PreJobTasks
the tko_test_view_2 table won't contain a row representing the
job. If the job aborts anytime after, the mentioned table usually
contains relevant information, in spite of the fact that the job
was aborted. This cl modifies dynamic_suite to check the database
on all types of failure, and only go the old route of yielding
an insipid status if the database doesn't contain an entry for
the job.
This cl also includes links to the failed job in the bug
description, as several people have expressed the desire
to have this.
TEST=Ran suites, aborted jobs, checked bugs.
Server job abort: https://code.google.com/p/autotest-bug-filing-test/issues/detail?id=1855
Premature abort: https://code.google.com/p/autotest-bug-filing-test/issues/detail?id=1854
BUG=chromium:188217, chromium:271036, chromium:318679
Change-Id: Ia8d1d3dd889c8dc2303371bfb522f44b7c52593a
Reviewed-on: https://chromium-review.googlesource.com/176753
Commit-Queue: Prashanth B <beeps@chromium.org>
Tested-by: Prashanth B <beeps@chromium.org>
Reviewed-by: Dan Shi <dshi@chromium.org>
diff --git a/global_config.ini b/global_config.ini
index aed4e56..25f83ad 100644
--- a/global_config.ini
+++ b/global_config.ini
@@ -194,6 +194,7 @@
debug_dir: debug/
buildbot_builders: http://chromegw.corp.google.com/i/chromeos/builders/
build_prefix: build/
+cautotest_job_view = http://cautotest.corp.google.com/afe/#tab_id=view_job&object_id
tracker_url: https://code.google.com/p/chromium/issues/detail?id=
gs_file_prefix: gs://
chromium_email_address = @chromium.org
diff --git a/server/cros/dynamic_suite/job_status.py b/server/cros/dynamic_suite/job_status.py
index 2119bc5..72b7f17 100644
--- a/server/cros/dynamic_suite/job_status.py
+++ b/server/cros/dynamic_suite/job_status.py
@@ -322,23 +322,33 @@
@yields an iterator of Statuses, one per test.
"""
entries = afe.run('get_host_queue_entries', job=job.id)
- if reduce(_collate_aborted, entries, False):
+
+ # This query uses the job id to search through the tko_test_view_2
+ # table, for results of a test with a similar job_tag. The job_tag
+ # is used to store results, and takes the form job_id-owner/host.
+ # Many times when a job aborts during a test, the job_tag actually
+ # exists and the results directory contains valid logs. If the job
+ # was aborted prematurely i.e before it had a chance to create the
+ # job_tag, this query will return no results. When statuses is not
+ # empty it will contain frontend.TestStatus' with fields populated
+ # using the results of the db query.
+ statuses = tko.get_job_test_statuses_from_db(job.id)
+ if not statuses:
yield Status('ABORT', job.name)
- else:
- statuses = tko.get_job_test_statuses_from_db(job.id)
- for s in statuses:
- if _status_for_test(s):
- yield Status(s.status, s.test_name, s.reason,
- s.test_started_time, s.test_finished_time,
- job.id, job.owner, s.hostname, job.name)
- else:
- if s.status != 'GOOD':
- yield Status(s.status,
- '%s_%s' % (entries[0]['job']['name'],
- s.test_name),
- s.reason, s.test_started_time,
- s.test_finished_time, job.id,
- job.owner, s.hostname, job.name)
+
+ for s in statuses:
+ if _status_for_test(s):
+ yield Status(s.status, s.test_name, s.reason,
+ s.test_started_time, s.test_finished_time,
+ job.id, job.owner, s.hostname, job.name)
+ else:
+ if s.status != 'GOOD':
+ yield Status(s.status,
+ '%s_%s' % (entries[0]['job']['name'],
+ s.test_name),
+ s.reason, s.test_started_time,
+ s.test_finished_time, job.id,
+ job.owner, s.hostname, job.name)
def wait_for_child_results(afe, tko, parent_job_id):
diff --git a/server/cros/dynamic_suite/job_status_unittest.py b/server/cros/dynamic_suite/job_status_unittest.py
index e223bf7..0984c3d 100644
--- a/server/cros/dynamic_suite/job_status_unittest.py
+++ b/server/cros/dynamic_suite/job_status_unittest.py
@@ -430,10 +430,18 @@
FakeJob(2, [FakeStatus('TEST_NA', 'T0', 'no')]),
FakeJob(3, [FakeStatus('FAIL', 'T0', 'broken')]),
FakeJob(4, [FakeStatus('ERROR', 'SERVER_JOB', 'server error'),
- FakeStatus('GOOD', 'T0', '')]),
- FakeJob(5, [FakeStatus('ERROR', 'T0', 'gah', True)]),
+ FakeStatus('GOOD', 'T0', '')]),]
+
+ # TODO: Write a better test for the case where we yield
+ # results for aborts vs cannot yield results because of
+ # a premature abort. Currently almost all client aborts
+ # have been converted to failures, and when aborts do happen
+ # they result in server job failures for which we always
+ # want results.
+ # FakeJob(5, [FakeStatus('ERROR', 'T0', 'gah', True)]),
# The next job shouldn't be recorded in the results.
- FakeJob(6, [FakeStatus('GOOD', 'SERVER_JOB', '')])]
+ # FakeJob(6, [FakeStatus('GOOD', 'SERVER_JOB', '')])]
+
for status in jobs[4].statuses:
status.entry['job'] = {'name': 'broken_infra_job'}
@@ -473,12 +481,19 @@
parent_job_id=parent_job_id),
FakeJob(4, [FakeStatus('ERROR', 'SERVER_JOB', 'server error'),
FakeStatus('GOOD', 'T0', '')],
- parent_job_id=parent_job_id),
- FakeJob(5, [FakeStatus('ERROR', 'T0', 'gah', True)],
- parent_job_id=parent_job_id),
+ parent_job_id=parent_job_id),]
+
+ # TODO: Write a better test for the case where we yield
+ # results for aborts vs cannot yield results because of
+ # a premature abort. Currently almost all client aborts
+ # have been converted to failures and when aborts do happen
+ # they result in server job failures for which we always
+ # want results.
+ #FakeJob(5, [FakeStatus('ERROR', 'T0', 'gah', True)],
+ # parent_job_id=parent_job_id),
# The next job shouldn't be recorded in the results.
- FakeJob(6, [FakeStatus('GOOD', 'SERVER_JOB', '')],
- parent_job_id=12345)]
+ #FakeJob(6, [FakeStatus('GOOD', 'SERVER_JOB', '')],
+ # parent_job_id=12345)]
for status in jobs[4].statuses:
status.entry['job'] = {'name': 'broken_infra_job'}
diff --git a/server/cros/dynamic_suite/reporting.py b/server/cros/dynamic_suite/reporting.py
index af25c11..e3ab2f3 100644
--- a/server/cros/dynamic_suite/reporting.py
+++ b/server/cros/dynamic_suite/reporting.py
@@ -105,6 +105,10 @@
_debug_dir = global_config.global_config.get_config_value(
BUG_CONFIG_SECTION, 'debug_dir', default='')
+ # cautotest url used to generate the link to the job
+ _cautotest_job_view = global_config.global_config.get_config_value(
+ BUG_CONFIG_SECTION, 'cautotest_job_view', default='')
+
# gs prefix to perform file like operations (gs://)
_gs_file_prefix = global_config.global_config.get_config_value(
BUG_CONFIG_SECTION, 'gs_file_prefix', default='')
@@ -169,7 +173,8 @@
'Build: %(build)s.\n\nReason:\n%(reason)s.\n'
'build artifacts: %(build_artifacts)s.\n'
'results log: %(results_log)s.\n'
- 'buildbot stages: %(buildbot_stages)s.\n')
+ 'buildbot stages: %(buildbot_stages)s.\n'
+ 'job link: %(job)s.\n')
specifics = {
'test': self.name,
@@ -180,6 +185,7 @@
'build_artifacts': links.artifacts,
'results_log': links.results,
'buildbot_stages': links.buildbot,
+ 'job': links.job,
}
return template % specifics
@@ -204,7 +210,17 @@
self.hostname, self._debug_dir)
return (self._retrieve_logs_cgi + self._generic_results_bin +
path_to_object)
- return 'NA'
+
+ return ('Could not generate results log: the job with id %s, '
+ 'scheduled by: %s on host: %s did not run' %
+ (self.job_id, self.result_owner, self.hostname))
+
+
+ def _link_job(self):
+ """Returns an url to the job on cautotest."""
+ if not self.job_id:
+ return 'Job did not run, or was aborted prematurely'
+ return '%s=%s' % (self._cautotest_job_view, self.job_id)
def _get_metadata_dict(self):
@@ -257,10 +273,12 @@
"""Returns a named tuple of links related to this failure."""
links = collections.namedtuple('links', ('results,'
'artifacts,'
- 'buildbot'))
+ 'buildbot,'
+ 'job'))
return links(self._link_result_logs(),
self._link_build_artifacts(),
- self._link_buildbot_stages())
+ self._link_buildbot_stages(),
+ self._link_job())
class Reporter(object):
