New issue
Advanced search Search tips

Issue 923426 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Run tast tests on chrome waterfalls via the host-side "tast" tool instead of the dut-side "local_test_runner"

Project Member Reported by bpastene@chromium.org, Jan 18 (4 days ago)

Issue description

Currently, tast tests don't fit in too well with chrome's test results reporting & recording. One run of chrome_all_tast_tests is considered just a single test, with only a pass or fail result. But we've got the ability to track a lot more than that (individual tests within a suite, run duration, output snippets). It's essentially everything outlined in:
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/json_test_results_format.md

eg: See https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/chromeos-amd64-generic-rel/24394 where video.WebRTCCamera and video.WebRTCPeerConnCameraVP8 failed, but the build page just says "chrome_all_tast_tests (status FAILURE)".

Compare that with cc_unittests in https://luci-milo.appspot.com/p/chromium/builders/luci.chromium.try/chromeos-amd64-generic-rel/171724. The suite failed 6 tests cases, each of which are printed in the build page and also have a link to a log of the test's output (https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8924022334701275504/+/steps/cc_unittests__with_patch_/0/logs/KeyframeModelTest.TrimTimeAlternateReverseTwoIterationsPlaybackNormalAlternate__status_FAILURE_/0)

It would be great if we could get tast tests to utilize the same result reporting. Right now that's a bit difficult since we're using local_test_runner on the device itself, so I think the only option there is to parse the stdout, which I'm not too wild about.

The tast binary is more feature-rich FWIU, so switching to that should make hooking up the results a lot easier. The only problem is getting access to the tool on chrome bots, which don't have a cros chroot checkout. One option is to package the tool into the simplechrome sdk we download (not sure how easy that would be). Another option is to package it up in CIPD, which our bots know well. It looks it was being CIPD'ed at one point, but stopped in https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1086160/. Not sure why it was reverted.. maybe we could add it back?

Tentatively assigning to dan so we can start discussing this when he's back from OOO
 

Comment 1 by derat@chromium.org, Jan 20 (2 days ago)

Cc: akes...@chromium.org derat@chromium.org ayatane@chromium.org nya@chromium.org hidehiko@chromium.org
Owner: bpastene@chromium.org
Yes, switching to using "tast run" instead of executing local_test_runner seems like the way to go here.

If you want to reenable the CIPD packages, that's okay with me. However, I deleted them in favor of using Server-Side Packaging since getting the tast binary and remote test bundle via CIPD was worse than SSP in several ways:

a) The version of remote tests didn't match the CrOS version on the DUT, which could result in test failures.

b) The version of the tast binary didn't match the local_test_runner version on the DUT, which could result in incompatibilities if/when the messages between the two processes are modified. I try to preserve compatibility in both directions for a while, but it's still a risk.

Also note that triggering the package builds was a bit tricky to get working, and I don't know if the relevant code has changed in the meantime. ayatane@ or akeshet@ may know. If you look at an old version of go/tast-infra (https://chrome-internal.googlesource.com/chromeos/chromeos-admin/+/785753280d757bab19124e3407b4a997096ce6ee/doc/tast_integration.md was where I added it), it used to explain how the CIPD stuff worked.

So packaging it in the SDK may be better, if that'll make the version match the system image on the DUT. I'm happy to make any Tast changes that'd help here (including massaging the output format if needed). I'm unlikely to have cycles to work on the packaging side of this in the near future, though.

Sign in to add a comment