From 41bf9201b5b634a68166f7345e61f5b75c64f16e Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Fri, 22 Jan 2021 00:18:34 +0100 Subject: [PATCH] ci: do not rely on TREZOR_PATH, use switch ports instead As the connected Trezors can be left in various weird states, trezorctl list may not always return what is expected or even fail. If it failed TREZOR_PATH was set to empty string which means random device got selected. For now let's avoid using incorrect device by powering down the other usb port. Fix log lines ordering. --- ci/hardware_tests/bootstrap.py | 8 +++-- ci/hardware_tests/device/device.py | 45 ++++++++++++++++++++-------- ci/hardware_tests/device/t1.py | 10 ++++--- ci/hardware_tests/device/tt.py | 6 ++-- ci/hardware_tests/get_trezor_path.sh | 9 ------ ci/hardware_tests/t1_hw_test.sh | 3 -- ci/test.yml | 3 -- 7 files changed, 48 insertions(+), 36 deletions(-) delete mode 100755 ci/hardware_tests/get_trezor_path.sh diff --git a/ci/hardware_tests/bootstrap.py b/ci/hardware_tests/bootstrap.py index ead7099f3..c2fe2fa82 100755 --- a/ci/hardware_tests/bootstrap.py +++ b/ci/hardware_tests/bootstrap.py @@ -16,12 +16,16 @@ def main(model: str, file: str = None): tt = TrezorT(config["tt"]["uhub_location"], config["tt"]["port"]) if model == "t1": - t1.update_firmware(file) + tt.power_off() + path = t1.update_firmware(file) elif model == "tt": - tt.update_firmware(file) + t1.power_off() + path = tt.update_firmware(file) else: raise ValueError("Unknown Trezor model.") + print(path) + if __name__ == "__main__": model = sys.argv[1] diff --git a/ci/hardware_tests/device/device.py b/ci/hardware_tests/device/device.py index a3631ee61..eee8ac455 100644 --- a/ci/hardware_tests/device/device.py +++ b/ci/hardware_tests/device/device.py @@ -1,6 +1,7 @@ import datetime -import os +import sys import time +from subprocess import run class Device: @@ -8,14 +9,28 @@ class Device: self.uhub_location = uhub_location self.device_port = device_port - def run_trezorctl(self, cmd: str): + @staticmethod + def log(msg): + print(msg, flush=True, file=sys.stderr) + + def run_trezorctl(self, cmd: str, **kwargs): full_cmd = "trezorctl " full_cmd += cmd - print("[software/trezorctl] Running '{}'".format(full_cmd)) - os.system(full_cmd) + self.log("[software/trezorctl] Running '{}'".format(full_cmd)) + return run(full_cmd, shell=True, check=True, **kwargs) - def check_version(self): + def check_model(self, model=None): + res = self.run_trezorctl("list", capture_output=True, text=True).stdout + self.log(res) self.run_trezorctl("get-features | grep version") + lines = res.splitlines() + if len(lines) != 1: + raise RuntimeError("{} trezors connected".format(len(lines))) + if model and model not in lines[0]: + raise RuntimeError( + "invalid trezor model connected (expected {})".format(model) + ) + return lines[0].split()[0] def reboot(self): self.power_off() @@ -23,19 +38,23 @@ class Device: def power_on(self): self.now() - print("[hardware/usb] Turning power on...") - os.system( - "uhubctl -l {} -p {} -a on".format(self.uhub_location, self.device_port) + self.log("[hardware/usb] Turning power on...") + run( + "uhubctl -l {} -p {} -a on".format(self.uhub_location, self.device_port), + shell=True, + check=True, ) self.wait(3) def power_off(self): self.now() - print("[hardware/usb] Turning power off...") - os.system( + self.log("[hardware/usb] Turning power off...") + run( "uhubctl -l {} -p {} -r 100 -a off".format( self.uhub_location, self.device_port - ) + ), + shell=True, + check=True, ) self.wait(3) @@ -45,9 +64,9 @@ class Device: @staticmethod def wait(seconds): Device.now() - print("[software] Waiting for {} seconds...".format(seconds)) + Device.log("[software] Waiting for {} seconds...".format(seconds)) time.sleep(seconds) @staticmethod def now(): - print("\n[timestamp] {}".format(datetime.datetime.now())) + Device.log("\n[timestamp] {}".format(datetime.datetime.now())) diff --git a/ci/hardware_tests/device/t1.py b/ci/hardware_tests/device/t1.py index 7ef145b35..8587b79c7 100644 --- a/ci/hardware_tests/device/t1.py +++ b/ci/hardware_tests/device/t1.py @@ -10,7 +10,7 @@ class TrezorOne(Device): def touch(self, location, action): self.now() - print( + self.log( "[hardware/trezor] Touching the {} button by {}...".format(location, action) ) self.serial.write(("{} {}\n".format(location, action)).encode()) @@ -19,14 +19,16 @@ class TrezorOne(Device): if file: unofficial = True trezorctlcmd = "firmware-update -s -f {} &".format(file) - print("[software] Updating the firmware to {}".format(file)) + self.log("[software] Updating the firmware to {}".format(file)) else: unofficial = False trezorctlcmd = "firmware-update &" - print("[software] Updating the firmware to latest") + self.log("[software] Updating the firmware to latest") self.wait(3) self._enter_bootloader() + self.run_trezorctl("list") + self.wait(3) self.run_trezorctl(trezorctlcmd) self.wait(3) @@ -42,7 +44,7 @@ class TrezorOne(Device): self.wait(5) self.touch("right", "click") self.wait(5) - self.check_version() + return self.check_model("Trezor 1") def _enter_bootloader(self): self.power_off() diff --git a/ci/hardware_tests/device/tt.py b/ci/hardware_tests/device/tt.py index 8f439ab92..c7660c0bd 100644 --- a/ci/hardware_tests/device/tt.py +++ b/ci/hardware_tests/device/tt.py @@ -7,15 +7,17 @@ class TrezorT(Device): self.power_off() self.power_on() + self.run_trezorctl("list") + if not file: raise ValueError( "Uploading production firmware will replace the bootloader, it is not allowed!" ) self.wait(5) - print("[software] Updating the firmware to {}".format(file)) + self.log("[software] Updating the firmware to {}".format(file)) self.run_trezorctl("firmware-update -s -f {}".format(file)) # after firmware-update finishes wait for reboot self.wait(15) - self.check_version() + return self.check_model("Trezor T") diff --git a/ci/hardware_tests/get_trezor_path.sh b/ci/hardware_tests/get_trezor_path.sh deleted file mode 100755 index a47492929..000000000 --- a/ci/hardware_tests/get_trezor_path.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env bash - -if [ $# -ne 1 ] - then - echo "Usage: $0 [model]" - exit 1 -fi - -nix-shell --run "pipenv run trezorctl list | grep '$1' | cut -d' ' -f1 | tr -d '\n'" diff --git a/ci/hardware_tests/t1_hw_test.sh b/ci/hardware_tests/t1_hw_test.sh index 621c88ffa..f89212b0e 100755 --- a/ci/hardware_tests/t1_hw_test.sh +++ b/ci/hardware_tests/t1_hw_test.sh @@ -12,9 +12,6 @@ set -x # trace commands ./record_video.sh ${CI_COMMIT_SHORT_SHA} start (cd ../.. && poetry install) -poetry run trezorctl -v list -export TREZOR_PATH=$(./get_trezor_path.sh 'Trezor 1') -echo $TREZOR_PATH poetry run python bootstrap.py t1 poetry run python bootstrap.py t1 ../../trezor-*.bin poetry run pytest ../../tests/device_tests diff --git a/ci/test.yml b/ci/test.yml index ea9f70bb1..ea89998ce 100644 --- a/ci/test.yml +++ b/ci/test.yml @@ -325,9 +325,6 @@ hardware core btconly device test: script: - cd ci/hardware_tests - nix-shell --run "cd ../.. && poetry install" - - nix-shell --run "poetry run trezorctl list" - - export TREZOR_PATH=$(./get_trezor_path.sh 'Trezor T') - - nix-shell --run "echo $TREZOR_PATH" - nix-shell --run "poetry run python bootstrap.py tt ../../trezor-*.bin" - nix-shell --run "poetry run pytest ../../tests/device_tests -m 'not sd_card' -k 'not 15_of_15 and not test_multisig_mismatch_inputs'" timeout: 4h