Skip to content

Commit

Permalink
[microTVM][RVM] Always destroy the VM if all tests pass (apache#8739)
Browse files Browse the repository at this point in the history
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 <gustavo.romero@linaro.org>
  • Loading branch information
gromero authored and ylc committed Jan 13, 2022
1 parent f8ef4a4 commit fe5e8b0
Showing 1 changed file with 35 additions and 13 deletions.
48 changes: 35 additions & 13 deletions apps/microtvm/reference-vm/base-box-tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit fe5e8b0

Please sign in to comment.