[autotest] Isolate code for autofiled bug keyvals
There was a small amount of duplicated code shared between the code
that records job keyvals when a bug is filed, and the code that
extracts the bug information from keyvals when reporting suite
results. This pulls that code into a common location so it can
be shared, and modified more transparently.
BUG=None
TEST=run dummy suite on a local autotest instance, see expected results
Change-Id: I65193559447ffadbc02b781a3654743e5143c9ee
Reviewed-on: https://gerrit.chromium.org/gerrit/66592
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Prashanth Balasubramanian <beeps@chromium.org>
Commit-Queue: Richard Barnette <jrbarnette@chromium.org>
diff --git a/server/cros/dynamic_suite/suite.py b/server/cros/dynamic_suite/suite.py
index 1bd6ef3..48fbe8f 100644
--- a/server/cros/dynamic_suite/suite.py
+++ b/server/cros/dynamic_suite/suite.py
@@ -12,8 +12,9 @@
from autotest_lib.server.cros.dynamic_suite import control_file_getter
from autotest_lib.server.cros.dynamic_suite import frontend_wrappers
from autotest_lib.server.cros.dynamic_suite import job_status
-from autotest_lib.server.cros.dynamic_suite.job_status import Status
from autotest_lib.server.cros.dynamic_suite import reporting
+from autotest_lib.server.cros.dynamic_suite import tools
+from autotest_lib.server.cros.dynamic_suite.job_status import Status
class Suite(object):
"""
@@ -468,19 +469,10 @@
result)
bug_id = bug_reporter.report(failure, bug_template)
+ bug_keyvals = tools.create_bug_keyvals(
+ result.test_name, bug_id)
try:
- # Attempting to use the name of a job with special
- # characters as a keyval will throw a ValueError. One
- # such case is with aborted jobs. Luckily, we don't
- # really care about the name, since the same name we
- # have here is inserted into the results database we can
- # use it as a key to retrieve the bug id. An example key
- # for an aborted job after replacing the '/' with '_':
- # lumpy-release_R28-3947.0.0_dummy_experimental_dummy_\
- # Pass-Bug_Id=xxxx, where xxxx is the id of the bug.
- utils.write_keyval(self._results_dir, {
- (result.test_name.replace('/', '_')+
- constants.BUG_KEYVAL): bug_id})
+ utils.write_keyval(self._results_dir, bug_keyvals)
except ValueError:
logging.error('Unable to log keyval for test:%s '
'bugid: %s', result.test_name, bug_id)
diff --git a/server/cros/dynamic_suite/tools.py b/server/cros/dynamic_suite/tools.py
index 47d5636..2d789af 100644
--- a/server/cros/dynamic_suite/tools.py
+++ b/server/cros/dynamic_suite/tools.py
@@ -6,7 +6,10 @@
import random
import re
+import common
+
from autotest_lib.client.common_lib import global_config
+from autotest_lib.server.cros.dynamic_suite import constants
_CONFIG = global_config.global_config
@@ -160,3 +163,38 @@
True if the host is locked by the infra user.
"""
return (host.locked and host.locked_by != infrastructure_user())
+
+
+def _testname_to_keyval_key(testname):
+ """Make a test name acceptable as a keyval key.
+
+ @param testname Test name that must be converted.
+ @return A string with selected bad characters replaced
+ with allowable characters.
+ """
+ # Characters for keys in autotest keyvals are restricted; in
+ # particular, '/' isn't allowed. Alas, in the case of an
+ # aborted job, the test name will be a path that includes '/'
+ # characters. We want to file bugs for aborted jobs, so we
+ # apply a transform here to avoid trouble.
+ return testname.replace('/', '_') + constants.BUG_KEYVAL
+
+
+def create_bug_keyvals(testname, bug_id):
+ """Create keyvals to record a bug filed against a test failure.
+
+ @param testname Name of the test for which to record a bug.
+ @param bug_id Id of the bug to be recorded for the test.
+ @return Keyvals to be recorded for the given test.
+ """
+ return {_testname_to_keyval_key(testname): bug_id}
+
+
+def get_test_failure_bug_id(keyvals, testname):
+ """Extract the id for a bug filed against a test failure.
+
+ @param keyvals Keyvals associated with a suite job.
+ @param testname Name of a test from the suite.
+ @return Id of the bug file against the test's failure.
+ """
+ return keyvals.get(_testname_to_keyval_key(testname))
diff --git a/site_utils/run_suite.py b/site_utils/run_suite.py
index 0333d14..4492d42 100755
--- a/site_utils/run_suite.py
+++ b/site_utils/run_suite.py
@@ -24,6 +24,7 @@
from autotest_lib.server.cros.dynamic_suite import constants
from autotest_lib.server.cros.dynamic_suite import frontend_wrappers
from autotest_lib.server.cros.dynamic_suite import job_status
+from autotest_lib.server.cros.dynamic_suite import tools
from autotest_lib.server.cros.dynamic_suite.reimager import Reimager
from autotest_lib.site_utils.graphite import stats
@@ -634,16 +635,14 @@
options.name).ljust(width)
logging.info("%s%s", test_view, get_pretty_status(view['status']))
- # It's important that we:
- # a. Use the test name in the view and not the name returned by
- # full_test_name, as this was the name inserted after the test
- # ran. Eg: for an aborted test full_test_name will return
- # experimental_testname but the view and the bug_id keyval will
- # contain /bulid/suite/experimental_testname.
- # b. Apply the inverse function that was applied to record the bug
- # id as a keyval in dynamic_suite, by replacing all '/' with '_'.
- bug_id = view['job_keyvals'].get(
- view['test_name'].replace('/', '_')+constants.BUG_KEYVAL)
+ # It's important that we use the test name in the view
+ # and not the name returned by full_test_name, as this
+ # was the name inserted after the test ran, e.g. for an
+ # aborted test full_test_name will return
+ # 'experimental_testname' but the view and the bug_id
+ # keyval will use '/build/suite/experimental_testname'.
+ bug_id = tools.get_test_failure_bug_id(
+ view['job_keyvals'], view['test_name'])
link = LogLink(test_view, job_name, bug_id)
web_links.append(link)