From fe5e8b0e81037cb6d61484ca0998da0689a301d9 Mon Sep 17 00:00:00 2001 From: Gustavo Romero Date: Mon, 18 Oct 2021 14:54:30 -0300 Subject: [PATCH] [microTVM][RVM] Always destroy the VM if all tests pass (#8739) Currently base-box-tool 'test' command will skip destroying the test VM if a single provider is specified (i.e. --provider virtualbox) even if all tests pass. This is confusing (no warning is displayed to the user) and that will leave host resources (like USB devices necessary to run the test) locked by the VM. So if the user tries to run a program that uses the locked resource (e.g. openocd) cryptic failures might happen. Moreoever, even if all tests pass and more than one provider is specified but the option '--skip-build' is set a VM will be left running without notice. This commit changes that behavior by: 1. Always destroying the VM if the release test pass 2. Always keeping the VM up and running if a test fails 1. guarantees no resource remains locked by the VM without necessity. A new flag '--skip-destroy' is introduced in case the user still wants to keep a VM up and running if the release tests pass. 2. guarantees the VM where the test failed is left running for further inspection of the test that failed. Finally, for both 1. and 2. cases a proper message is displayed to the user to inform if a VM was left running or not and about what actions the user can take next accordingly to the test result in the VM. Signed-off-by: Gustavo Romero --- apps/microtvm/reference-vm/base-box-tool.py | 48 +++++++++++++++------ 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/apps/microtvm/reference-vm/base-box-tool.py b/apps/microtvm/reference-vm/base-box-tool.py index 3a5fd18cede7..42b90c661704 100755 --- a/apps/microtvm/reference-vm/base-box-tool.py +++ b/apps/microtvm/reference-vm/base-box-tool.py @@ -388,13 +388,15 @@ def test_command(args): microtvm_test_config["microtvm_board"] = args.microtvm_board providers = args.provider - provider_passed = {p: False for p in providers} release_test_dir = os.path.join(THIS_DIR, f"release-test-{args.platform}") - if args.skip_build: - assert len(providers) == 1, "--skip-build was given, but >1 provider specified" + if args.skip_build or args.skip_destroy: + assert ( + len(providers) == 1 + ), "--skip-build and/or --skip-destroy was given, but >1 provider specified" + test_failed = False for provider_name in providers: try: if not args.skip_build: @@ -408,18 +410,27 @@ def test_command(args): microtvm_test_config, args.test_device_serial, ) - provider_passed[provider_name] = True + + except subprocess.CalledProcessError: + test_failed = True + sys.exit( + f"\n\nERROR: Provider '{provider_name}' failed the release test. " + "You can re-run it to reproduce the issue without building everything " + "again by passing the --skip-build and specifying only the provider that failed. " + "The VM is still running in case you want to connect it via SSH to " + "investigate further the issue, thus it's necessary to destroy it manually " + "to release the resources back to the host, like a USB device attached to the VM." + ) finally: - if not args.skip_build and len(providers) > 1: + # if we reached out here do_run_release_test() succeeded, hence we can + # destroy the VM and release the resources back to the host if user haven't + # requested to not destroy it. + if not (args.skip_destroy or test_failed): subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir) shutil.rmtree(release_test_dir) - if not all(provider_passed[p] for p in provider_passed.keys()): - sys.exit( - "some providers failed release test: " - + ",".join(name for name, passed in provider_passed if not passed) - ) + print(f'\n\nThe release tests passed on all specified providers: {", ".join(providers)}.') def release_command(args): @@ -493,9 +504,20 @@ def parse_args(): "--skip-build", action="store_true", help=( - "If given, assume a box has already been built in " - "the release-test subdirectory. Attach a USB device to this box and execute the " - "release test script--do not delete it." + "If given, assume a box has already been built in the release-test subdirectory, " + "so use that box to execute the release test script. If the tests fail the VM used " + "for testing will be left running for further investigation and will need to be " + "destroyed manually. If all tests pass on all specified providers no VM is left running, " + "unless --skip-destroy is given too." + ), + ) + parser_test.add_argument( + "--skip-destroy", + action="store_true", + help=( + "Skip destroying the test VM even if all tests pass. Can only be used if a single " + "provider is specified. Default is to destroy the VM if all tests pass (and always " + "skip destroying it if a test fails)." ), ) parser_test.add_argument(