[autotest] Make test faster by avoiding unnecessary servo check
Looking at dummy_Pass, you will see we do servo check
three times. They come from create_host in control segments of
client_wrapper, crashdumps, and verify_job_repo_url. If a test
choose to log sysinfo, in test.py there are two calls of
create_host which will trigger two more servo check.
None of them really need servo.
This CL allows code to explicitly skip servo when initializing
a host. Each servo check takes 10-20 seconds.
By doing this, I anticipate we can save 30~60 seconds per
test (which has servo), ~20secs on provision.
TEST=Run verify, repair, cleanup, provision
TEST=Test doesn't need servo don't check servo:
Run dummy_Pass, observe no servo check in the log
TEST=Test that need servo do check servo:
Run infra_ServoDiagnosis.py on a host with servo, servo check
was run.
TEST=Run infra_ServoDiagnosis.py on a host with servo,
manually pass servo_args={} in its control file,
fail the servo verify, and make sure servo repair is triggered.
TEST=Run a dummy suite with run_suite.
BUG=chromium:445677, chromium:426141
Change-Id: I06ec28e81b3abb20637f36c717f0d32e2310bd9d
Reviewed-on: https://chromium-review.googlesource.com/238223
Reviewed-by: Fang Deng <fdeng@chromium.org>
Commit-Queue: Fang Deng <fdeng@chromium.org>
Trybot-Ready: Fang Deng <fdeng@chromium.org>
Tested-by: Fang Deng <fdeng@chromium.org>
diff --git a/server/control_segments/cleanup b/server/control_segments/cleanup
index 53a3a04..cb6b637 100644
--- a/server/control_segments/cleanup
+++ b/server/control_segments/cleanup
@@ -15,7 +15,8 @@
timer = None
try:
job.record('START', None, 'cleanup')
- host = hosts.create_host(machine, initialize=False, auto_monitor=False)
+ host = hosts.create_host(machine, initialize=False, auto_monitor=False,
+ try_lab_servo=True)
timer = stats.Timer('cleanup_time')
timer.start()
diff --git a/server/control_segments/provision b/server/control_segments/provision
index cda4f6d..6fd2273 100644
--- a/server/control_segments/provision
+++ b/server/control_segments/provision
@@ -18,7 +18,7 @@
Run the appropriate provisioning tests to make the machine's labels match
those given in job_labels.
"""
- host = hosts.create_host(machine)
+ host = hosts.create_host(machine, try_lab_servo=True)
fixed, provisionable = provision.filter_labels(labels_list)
diff --git a/server/control_segments/repair b/server/control_segments/repair
index 01c4d76..665cfca 100644
--- a/server/control_segments/repair
+++ b/server/control_segments/repair
@@ -28,7 +28,8 @@
def repair(machine):
try:
job.record('START', None, 'repair')
- host = hosts.create_host(machine, initialize=False, auto_monitor=False)
+ host = hosts.create_host(machine, initialize=False, auto_monitor=False,
+ try_lab_servo=True)
# Collect logs before the repair, as it might destroy all useful logs.
local_log_dir = os.path.join(job.resultdir, machine, 'before_repair')
host.collect_logs('/var/log', local_log_dir, ignore_errors=True)
diff --git a/server/control_segments/verify b/server/control_segments/verify
index 1e4a8f4..ae54783 100644
--- a/server/control_segments/verify
+++ b/server/control_segments/verify
@@ -12,7 +12,10 @@
timer = None
try:
job.record('START', None, 'verify')
- host = hosts.create_host(machine, initialize=False, auto_monitor=False)
+ # We set try_lab_servo to True to trigger servo verify and
+ # servo update if needed.
+ host = hosts.create_host(machine, initialize=False, auto_monitor=False,
+ try_lab_servo=True)
timer = stats.Timer('verify_time')
timer.start()
diff --git a/server/hosts/cros_host.py b/server/hosts/cros_host.py
index 2bb9ef1..bd2a6e9 100644
--- a/server/hosts/cros_host.py
+++ b/server/hosts/cros_host.py
@@ -272,24 +272,27 @@
def _initialize(self, hostname, chameleon_args=None, servo_args=None,
- ssh_verbosity_flag='', ssh_options='',
+ try_lab_servo=False, ssh_verbosity_flag='', ssh_options='',
*args, **dargs):
"""Initialize superclasses, |self.chameleon|, and |self.servo|.
- This method checks whether a chameleon/servo (aka
- test-assistant objects) is required by checking whether
- chameleon_args/servo_args is None. This method will only
- attempt to create the test-assistant object when it is
- required by the test.
+ This method will attempt to create the test-assistant object
+ (chameleon/servo) when it is needed by the test. Check
+ the docstring of chameleon_host.create_chameleon_host and
+ servo_host.create_servo_host for how this is determined.
- For creating the test-assistant object, there are three
- possibilities: First, if the host is a lab system known to have
- a test-assistant board, we connect to that board unconditionally.
- Second, if we're called from a control file that requires
- test-assistant features for testing, it will pass settings from
- the arguments, like `servo_host`, `servo_port`. If neither of
- these cases apply, the test-assistant object will be `None`.
-
+ @param hostname: Hostname of the dut.
+ @param chameleon_args: A dictionary that contains args for creating
+ a ChameleonHost. See chameleon_host for details.
+ @param servo_args: A dictionary that contains args for creating
+ a ServoHost object. See servo_host for details.
+ @param try_lab_servo: Boolean, False indicates that ServoHost should
+ not be created for a device in Cros test lab.
+ See servo_host for details.
+ @param ssh_verbosity_flag: String, to pass to the ssh command to control
+ verbosity.
+ @param ssh_options: String, other ssh options to pass to the ssh
+ command.
"""
super(CrosHost, self)._initialize(hostname=hostname,
*args, **dargs)
@@ -304,8 +307,9 @@
# TODO(fdeng): We need to simplify the
# process of servo and servo_host initialization.
# crbug.com/298432
- self._servo_host = servo_host.create_servo_host(dut=self.hostname,
- servo_args=servo_args)
+ self._servo_host = servo_host.create_servo_host(
+ dut=self.hostname, servo_args=servo_args,
+ try_lab_servo=try_lab_servo)
# TODO(waihong): Do the simplication on Chameleon too.
self._chameleon_host = chameleon_host.create_chameleon_host(
dut=self.hostname, chameleon_args=chameleon_args)
diff --git a/server/hosts/servo_host.py b/server/hosts/servo_host.py
index 215cba3..7711e6e 100644
--- a/server/hosts/servo_host.py
+++ b/server/hosts/servo_host.py
@@ -607,20 +607,27 @@
return self._servo
-def create_servo_host(dut, servo_args):
+def create_servo_host(dut, servo_args, try_lab_servo=False):
"""Create a ServoHost object.
- There three possible cases:
- 1) If the DUT is in Cros Lab and has a beaglebone and a servo, then
- create a ServoHost object pointing to the beaglebone. servo_args
- is ignored.
- 2) If not case 1) and servo_args is neither None nor empty, then
- create a ServoHost object using servo_args.
- 3) If neither case 1) or 2) applies, return None.
- When the servo_args is not None, we assume the servo is required by the
- test. If servo failed to be verified, we will attempt to repair it. If servo
- is not required, we will initialize ServoHost without initializing a servo
- object.
+ The `servo_args` parameter is a dictionary specifying optional
+ Servo client parameter overrides (i.e. a specific host or port).
+ When specified, the caller requires that an exception be raised
+ unless both the ServoHost and the Servo are successfully
+ created.
+
+ There are three possible cases:
+ 1. If the DUT is in the Cros test lab then the ServoHost object
+ is only created for the host in the lab. Alternate host or
+ port settings in `servo_host` will be ignored.
+ 2. When not case 1., but `servo_args` is not `None`, then create
+ a ServoHost object using `servo_args`.
+ 3. Otherwise, return `None`.
+
+ When the `try_lab_servo` parameter is false, it indicates that a
+ ServoHost should not be created for a device in the Cros test
+ lab. The setting of `servo_args` takes precedence over the
+ setting of `try_lab_servo`.
@param dut: host name of the host that servo connects. It can be used to
lookup the servo in test lab using naming convention.
@@ -629,6 +636,8 @@
e.g. {'servo_host': '172.11.11.111',
'servo_port': 9999}.
See comments above.
+ @param try_lab_servo: Boolean. Whether to create ServoHost for a device
+ in test lab. See above.
@returns: A ServoHost object or None. See comments above.
@@ -636,7 +645,11 @@
lab_servo_hostname = make_servo_hostname(dut)
is_in_lab = utils.host_is_in_lab_zone(lab_servo_hostname)
- if is_in_lab:
+ if not is_in_lab:
+ if servo_args is None:
+ return None
+ return ServoHost(required_by_test=True, is_in_lab=False, **servo_args)
+ elif servo_args is not None or try_lab_servo:
# Technically, this duplicates the SSH ping done early in the servo
# proxy initialization code. However, this ping ends in a couple
# seconds when if fails, rather than the 60 seconds it takes to decide
@@ -653,7 +666,5 @@
if host_is_up:
return ServoHost(servo_host=lab_servo_hostname, is_in_lab=is_in_lab,
required_by_test=(servo_args is not None))
- elif servo_args is not None:
- return ServoHost(required_by_test=True, is_in_lab=False, **servo_args)
else:
return None
diff --git a/server/site_tests/infra_ServoDiagnosis/control b/server/site_tests/infra_ServoDiagnosis/control
index 3f08c32..ed08ffa 100644
--- a/server/site_tests/infra_ServoDiagnosis/control
+++ b/server/site_tests/infra_ServoDiagnosis/control
@@ -28,7 +28,7 @@
# servo_args = hosts.CrosHost.get_servo_arguments(args_dict)
def run(machine):
- host = hosts.create_host(machine)
+ host = hosts.create_host(machine, try_lab_servo=True)
job.run_test("infra_ServoDiagnosis", host=host)
parallel_simple(run, machines)