New issue
Advanced search Search tips

Issue 828607 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 832374



Sign in to add a comment

Create a wrapper (and generator) script around chromite's run_cros_vm_test

Project Member Reported by bpastene@chromium.org, Apr 3 2018

Issue description

The wrapper script will wrap https://codesearch.chromium.org/chromium/src/third_party/chromite/scripts/cros_run_vm_test.py and will handle all the args we'd pass down.

The generator script, when called, will create the executable $OUTDIR/bin/run_$TESTNAME for a given test at compile-time. The generated script will simply invoke the wrapper script, giving us a single call-point for any test.

This is similar to what android and fuchsia does, so should be pretty reasonable. Though we may not need the wrapper script. The generated script could instead call run_cros_vm_test.py directly. Not sure about that yet. Will figure it out as I implement this.

See https://codesearch.chromium.org/chromium/src/build/fuchsia/create_runner_script.py as an example of fuchsia's generator script.
 
If you need to add features to cros_run_vm_test.py, we can do that; you're welcome to add them yourself, or if you give me a spec, I can implement it. cros_run_vm_test is mainly intended to be used by the bot, so if we can avoid adding another layer of abstraction, we should do so.
Agreed that we should avoid abstraction. Though the script generator is nice to have regardless, the wrapper script may not be needed. I'll get a working demo up today and see what we'd have it do and if we can move that functionality into chromite.
I got a working end to end setup for running cros vm tests in swarming:
https://chromium-review.googlesource.com/c/chromium/src/+/994062 (still a WIP)

Ran it on
media_unittests: https://chromium-swarm.appspot.com/task?id=3cac2d61e7a2ed10
base_unittests: https://chromium-swarm.appspot.com/task?id=3cac2d49b2ab0410
sandbox_linux_unittests: https://chromium-swarm.appspot.com/task?id=3cac2d7bd148ea10
(Each one failed only a few tests, which is pretty cool.)

The small wrapper I added (build/chromeos/run_vm_test.py ) didn't end up doing much. The main things were path manipulation and pushing test data files to the device (via the --files arg in cros_run_vm_test). All that can go in the generated script I think, which makes things a bit lighter.

There were a few caveats/hacks that might need rethinking:
- chromite's vpython spec doesn't have oauthtuils (attempt at a fix in https://chromium-review.googlesource.com/c/chromiumos/chromite/+/990932)
- chromites venv/ dir contains infinite sym link loops which isolate doesn't like; I had to list everything but that dir in the deps
- the cros cache's qemu contains broken symlinks, which isolate also doesn't like; I had to manually list all the needed libs and bins in the deps
- the gn target for the wrapper grabs sdk version and board via env vars that the cros-chrome-sdk sets; not sure of a cleaner way to communicate between the two, but it works...
Cc: vapier@chromium.org
I think Mike may have some thoughts on the issues you ran into.

Sounds like isolate doesn't copy symlinks as they are, but instead tries to follow them? We could try cleaning up some of the symlinks.
I figured the broken links in the qemu directory was a consequence of whatever mechanism was used to package it up. (Something in chromeos land?) We could clean them up, but I also filed  bug 829466  on our end to gracefully handle broken links. Either fix would clear up that hack.

Oh, and it would be nice to stream the VM's stdout instead of dumping it all at the very end. Filed  bug 829481  for that.
Blocking: 832374
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c781d12d5489a26caf12f407bc5a0bc66cf7007e

commit c781d12d5489a26caf12f407bc5a0bc66cf7007e
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Mon Apr 16 23:16:12 2018

Fix data path to qemu in CrOS generate_vm_runner_script gn target.

It changed from -r2 to -r3 recently, so the path is no longer valid.

This just lists the parent dir instead. (qemu should be the only thing
in it).

Bug: 828607
Change-Id: I2bc72b8f4202fedadc5274031d395df72ddd4276
Reviewed-on: https://chromium-review.googlesource.com/1014489
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551164}
[modify] https://crrev.com/c781d12d5489a26caf12f407bc5a0bc66cf7007e/build/config/chromeos/rules.gni

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c781d12d5489a26caf12f407bc5a0bc66cf7007e

commit c781d12d5489a26caf12f407bc5a0bc66cf7007e
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Mon Apr 16 23:16:12 2018

Fix data path to qemu in CrOS generate_vm_runner_script gn target.

It changed from -r2 to -r3 recently, so the path is no longer valid.

This just lists the parent dir instead. (qemu should be the only thing
in it).

Bug: 828607
Change-Id: I2bc72b8f4202fedadc5274031d395df72ddd4276
Reviewed-on: https://chromium-review.googlesource.com/1014489
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551164}
[modify] https://crrev.com/c781d12d5489a26caf12f407bc5a0bc66cf7007e/build/config/chromeos/rules.gni

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67e8c110257594c18d66cbeff3a45321c7081f7e

commit 67e8c110257594c18d66cbeff3a45321c7081f7e
Author: Ben Pastene <bpastene@chromium.org>
Date: Tue Apr 24 23:18:16 2018

Add KVM-related checks to chromeos vm test runner.

Mostly for humans. The bots should all have kvm installed and available.

Also use logging instead of print.

Bug: 828607
Change-Id: Ifee1ead435932d6d2752c0a418cd38618a66228e
Reviewed-on: https://chromium-review.googlesource.com/1026569
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553350}
[modify] https://crrev.com/67e8c110257594c18d66cbeff3a45321c7081f7e/build/chromeos/run_vm_test.py

This is mostly done. Now when building chrome for chromeos via the simplechrome flow, any gtest targets you compile will generate a script at out/bin/run_*. (Similar to what we do for fuchsia and android). Invoking the run_* script will launch a vm, push the test and its deps, run it, and collect the results.

Currently working on getting telemetry's unittests working (they're unique since they run on the host and talk to the cros environment via ssh at test-time).
Project Member

Comment 13 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c60bf502e3e92c32c7c297a429563c83db02277e

commit c60bf502e3e92c32c7c297a429563c83db02277e
Author: Ben Pastene <bpastene@chromium.org>
Date: Fri May 04 01:10:33 2018

Create gn target that builds the cros-vm-launcher.

This "cros_vm_launcher" target generates a script at build time (located
at $out_dir/bin/launch_cros_vm) that you prepend to any other command
and it will ensure a cros VM is running for the duration of the command.

Also add it as a dependency to telemetry unittests. Invoking it in a bot's
tests will come in a later CL.

Bug: 828607,  832374 
Change-Id: I56ee169c81127bf8cac1114b719f1cdeeb2c3d31
Reviewed-on: https://chromium-review.googlesource.com/1040117
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555945}
[modify] https://crrev.com/c60bf502e3e92c32c7c297a429563c83db02277e/chromeos/BUILD.gn
[modify] https://crrev.com/c60bf502e3e92c32c7c297a429563c83db02277e/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a53981ff196802bba5e5436b4cb433a2fccc8c7

commit 5a53981ff196802bba5e5436b4cb433a2fccc8c7
Author: Ben Pastene <bpastene@chromium.org>
Date: Fri May 04 21:59:11 2018

Remove stdout result parsing from cros-vm test runner.

Instead, it just pulls the file from the VM directly, which was added in
CL:1021531.

Bug: 828607
Change-Id: I6c71f557bf91dafc8c5f638d0c07c49bde728ed4
Reviewed-on: https://chromium-review.googlesource.com/1044755
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556213}
[modify] https://crrev.com/5a53981ff196802bba5e5436b4cb433a2fccc8c7/build/chromeos/run_vm_test.py

Sign in to add a comment