[autotest] Only integer or None as num argument in create_suite_job
Cleanup to reduce confusion and type casting back and forth between int
and string.
BUG=chromium-os:37936
TEST=unit test that non-integer or None num arguments throw appropriate
exception; unit test that integer arguments get passed along correctly;
ran a suite locally to ensure rpc call still working
Change-Id: Id8b8e0dd5a08db2ebec67cdba13b2b1d8eb0b149
Reviewed-on: https://gerrit.chromium.org/gerrit/41791
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Tested-by: Aviv Keshet <akeshet@chromium.org>
Commit-Queue: David James <davidjames@chromium.org>
diff --git a/frontend/afe/site_rpc_interface.py b/frontend/afe/site_rpc_interface.py
index 2cdeb73..a50763d 100644
--- a/frontend/afe/site_rpc_interface.py
+++ b/frontend/afe/site_rpc_interface.py
@@ -7,8 +7,8 @@
import common
import datetime
import logging
-import sys
-from autotest_lib.client.common_lib import error, global_config
+
+from autotest_lib.client.common_lib import error
from autotest_lib.client.common_lib.cros import dev_server
from autotest_lib.server.cros.dynamic_suite import constants
from autotest_lib.server.cros.dynamic_suite import control_file_getter
@@ -83,7 +83,8 @@
@param pool: Specify the pool of machines to use for scheduling
purposes.
@param check_hosts: require appropriate live hosts to exist in the lab.
- @param num: Specify the number of machines to schedule across.
+ @param num: Specify the number of machines to schedule across (integer).
+ Leave unspecified or use None to use default sharding factor.
@param file_bugs: File a bug on each test failure in this suite.
@raises ControlFileNotFound: if a unique suite control file doesn't exist.
@@ -96,16 +97,13 @@
"""
# All suite names are assumed under test_suites/control.XX.
suite_name = canonicalize_suite_name(suite_name)
- try:
- if num is None: # Yes, specifically None
- numeric_num = None
- elif num == '0':
- logging.warning("Can't run on 0 hosts; using default.")
- numeric_num = None
- else:
- numeric_num = int(num)
- except (ValueError, TypeError) as e:
- raise error.SuiteArgumentException('Ill-specified num argument: %s' % e)
+
+ if type(num) is not int and num is not None:
+ raise error.SuiteArgumentException('Ill specified num argument. Must be'
+ ' an integer or None.')
+ if num == 0:
+ logging.warning("Can't run on 0 hosts; using default.")
+ num = None
timings = {}
# Ensure components of |build| necessary for installing images are staged
@@ -129,7 +127,7 @@
'build': build,
'check_hosts': check_hosts,
'pool': pool,
- 'num': numeric_num,
+ 'num': num,
'file_bugs': file_bugs}
control_file = tools.inject_vars(inject_dict, control_file_in)
diff --git a/frontend/afe/site_rpc_interface_unittest.py b/frontend/afe/site_rpc_interface_unittest.py
index 05d16c5..42b837f 100644
--- a/frontend/afe/site_rpc_interface_unittest.py
+++ b/frontend/afe/site_rpc_interface_unittest.py
@@ -56,11 +56,15 @@
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(self.getter)
- def _mockRpcUtils(self, to_return):
+ def _mockRpcUtils(self, to_return, control_file_substring=''):
"""Fake out the autotest rpc_utils module with a mockable class.
@param to_return: the value that rpc_utils.create_job_common() should
be mocked out to return.
+ @param control_file_substring: A substring that is expected to appear
+ in the control file output string that
+ is passed to create_job_common.
+ Default: ''
"""
download_started_time = constants.DOWNLOAD_STARTED_TIME
payload_finished_time = constants.PAYLOAD_FINISHED_TIME
@@ -70,7 +74,9 @@
priority='Medium',
control_type='Server',
control_file=mox.And(mox.StrContains(self._BOARD),
- mox.StrContains(self._BUILD)),
+ mox.StrContains(self._BUILD),
+ mox.StrContains(
+ control_file_substring)),
hostless=True,
keyvals=mox.And(mox.In(download_started_time),
mox.In(payload_finished_time))
@@ -142,6 +148,14 @@
self._BUILD,
None,
num=[])
+ self.assertRaises(error.SuiteArgumentException,
+ site_rpc_interface.create_suite_job,
+ self._NAME,
+ self._BOARD,
+ self._BUILD,
+ None,
+ num='5')
+
def testCreateSuiteJobFail(self):
@@ -187,9 +201,27 @@
job_id = 5
self._mockRpcUtils(job_id)
self.mox.ReplayAll()
- self.assertEquals(
- site_rpc_interface.create_suite_job(
- self._NAME, self._BOARD, self._BUILD, None, False),
+ self.assertEquals(site_rpc_interface.create_suite_job(self._NAME,
+ self._BOARD,
+ self._BUILD,
+ None, False),
+ job_id)
+
+ def testCreateSuiteIntegerNum(self):
+ """Ensures that success results in a successful RPC."""
+ self._mockDevServerGetter()
+ self.dev_server.trigger_download(self._BUILD,
+ synchronous=False).AndReturn(True)
+ self.getter.get_control_file_contents_by_name(
+ self._SUITE_NAME).AndReturn('f')
+ job_id = 5
+ self._mockRpcUtils(job_id, control_file_substring='num=17')
+ self.mox.ReplayAll()
+ self.assertEquals(site_rpc_interface.create_suite_job(self._NAME,
+ self._BOARD,
+ self._BUILD,
+ None, False,
+ num=17),
job_id)
diff --git a/site_utils/suite_scheduler/deduping_scheduler.py b/site_utils/suite_scheduler/deduping_scheduler.py
index 550c5dc..fd99196 100644
--- a/site_utils/suite_scheduler/deduping_scheduler.py
+++ b/site_utils/suite_scheduler/deduping_scheduler.py
@@ -4,7 +4,6 @@
import logging
from autotest_lib.server.cros.dynamic_suite import frontend_wrappers
-from autotest_lib.server import frontend
class DedupingSchedulerException(Exception):
@@ -73,6 +72,7 @@
@param pool: the pool of machines to use for scheduling purposes.
Default: None
@param num: the number of devices across which to shard the test suite.
+ Type: integer or None
Default: None (uses sharding factor in global_config.ini).
@return True if the suite got scheduled
@raise ScheduleException if an error occurs while scheduling.
@@ -107,6 +107,7 @@
x86-alex-release/R18-1655.0.0-a1-b1584.
@param pool: the pool of machines to use for scheduling purposes.
@param num: the number of devices across which to shard the test suite.
+ Type: integer or None
@param force: Always schedule the suite.
@return True if the suite got scheduled, False if not
@raise DedupException if we can't check for dups.
diff --git a/site_utils/suite_scheduler/task.py b/site_utils/suite_scheduler/task.py
index c53938a..4e32fed 100644
--- a/site_utils/suite_scheduler/task.py
+++ b/site_utils/suite_scheduler/task.py
@@ -4,7 +4,7 @@
import logging, re
-import deduping_scheduler, forgiving_config_parser
+import deduping_scheduler
from distutils import version
from constants import Labels
@@ -136,13 +136,14 @@
@param pool: the pool of machines to use for scheduling purposes.
Default: None
@param num: the number of devices across which to shard the test suite.
+ Type: integer or None
Default: None
"""
self._name = name
self._suite = suite
self._branch_specs = branch_specs
self._pool = pool
- self._num = '%d' % num if num else None
+ self._num = num
self._bare_branches = []
if not branch_specs:
@@ -159,7 +160,7 @@
# Since we expect __hash__() and other comparitor methods to be used
# frequently by set operations, and they use str() a lot, pre-compute
# the string representation of this object.
- self._str = '%s: %s on %s with pool %s, across %r machines' % (
+ self._str = '%s: %s on %s with pool %s, across %d machines' % (
self.__class__.__name__, suite, branch_specs, pool, num)