New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847901 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 3
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-09-26
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 876587
issue 877605
issue 891470
issue 897540



Sign in to add a comment

Add mash login tests to VMs on Chrome waterfall

Project Member Reported by jamescook@chromium.org, May 30 2018

Issue description

The Chrome waterfall / CQ now has Chrome OS VM bots. See email from achuith to chromeos-ui

Achuith says adding mash login to the vm_sanity test is the right thing to do for now.

vm_sanity is currently informational, since there's some low grade flake, but it would be helpful to get coverage on the chrome waterfall.

 
Cc: xiy...@chromium.org steve...@chromium.org rcui@chromium.org
+some folks who might be interested in this

I think it would just involve copying part of autotest desktopui_MashLogin into third_party/autotest/files/client/bin/vm_sanity.py

Cc: sky@chromium.org
Talked to sky, we should probably wait on this until mash/oopash is using ws2.
Labels: -Proj-Mustash Proj-Mash-SingleProcess
Talking with single-process as I think we should add such a test for single-process-mash.
Owner: rcui@chromium.org
Status: Assigned (was: Untriaged)
I can take this since I'm already shifting things around on the waterfall ( crbug.com/874090 ).
Cc: achuith@chromium.org
A quick note that a login test for SingleProcessMash in the VM (similar to the one at https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1180520/7/client/bin/vm_sanity.py) has started failing on TOT and recent builds.  Not seeing the failure when running with linux chromeos=1 and in browser_tests.  Will dig into this further.
What's the in-VM login failure for SingleProcessMash? Do we have a bug?

It seems to be stuck at the login screen.  I see these messages in the log:

INFO:root:Devtools client not yet ready: [Errno 111] Connection refused
INFO:root:Devtools client not yet ready: [Errno 111] Connection refused

It's currently somewhat involved to get setup to run this test, and involves scp-ing your local vm_sanity.py to the VM.  achuith@'s CL:1187139 will make it easier to repro.  Will file a bug with instructions once that lands.  In the meantime I can go over the steps in person.
If you look at the VM, you'll see a mostly white screen, so I suspect chrome has crashed?

You can install an unstripped version of chrome and look in the logs for the stack trace:
deploy_chrome --build-dir=out_$SDK_BOARD/Release/ --to=localhost --port=9222 --nostrip --mount

scp vm_sanity to the device looks like this:
scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i /usr/local/google/home/achuith/code/chrome/src/third_party/chromite/ssh_keys/testing_rsa -P 9222 /usr/local/google/home/achuith/code/cros/src/third_party/autotest/files/client/bin/vm_sanity.py root@localhost:/usr/local/autotest/bin/vm_sanity.py


Blockedon: 877605
Thanks.  Filed crbug/877605 to track the failing test.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/4d74d75b5587c952a139e5ee7c2978979e1a5786

commit 4d74d75b5587c952a139e5ee7c2978979e1a5786
Author: Ryan Cui <rcui@chromium.org>
Date: Fri Aug 31 12:23:13 2018

Run Mash login test in vm_sanity

Validate Mash mode (see //ash/README.md) during simple chrome builds on
the chromium waterfall.

BUG= chromium:847901 
TEST=cros_vm --start; cros_vm_run_test

Change-Id: I4d01d00e1a12f9e0837f56a503fcee42d53f0754
Reviewed-on: https://chromium-review.googlesource.com/1180520
Commit-Ready: Achuith Bhandarkar <achuith@chromium.org>
Commit-Ready: Ryan Cui <rcui@google.com>
Tested-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Ryan Cui <rcui@google.com>

[modify] https://crrev.com/4d74d75b5587c952a139e5ee7c2978979e1a5786/client/bin/vm_sanity.py

The mash test added in crrev.com/c/1180520 has been replaced by a tast test (see crrev.com/c/1220606).  We should add a tast test for SingleProcessMash as well.
Owner: jamescook@chromium.org
Cc: derat@chromium.org
Status: Started (was: Assigned)
Summary: Add mash login tests to vm_sanity on Chrome waterfall (was: Add mash login test to vm_sanity on Chrome waterfall)
We were in the Chrome and Chrome OS CQ for a few days, but that was reverted due to issue 882976.

Updated plan from email thread:

* Run mash login tests via tast ui.MashLogin in VMs in Chrome CQ. This will block chrome CLs on failure. I talked to dcastagna yesterday, we think that the VM is sufficient to catch interesting ozone and DRM issues.
* Keep tast ui.MashLogin where it is now in the Chrome OS hardware test lab (per-release-build, informational). I get summary emails of these failures so I'll see if something explodes.
* Take autotest desktopui_MashLogin out of bvt-perbuild since tast is giving us roughly the same coverage.
* Leave autotest desktopui_MashLogin in ToT chrome-informational for now. It's a nice early-warning signal for on-hardware failures, and it doesn't block CQ/PFQ. We can revisit removing it later
* Add another tast test for SingleProcessMash.

Issue 708002 has been merged into this issue.
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/0b4fa617f4023d1bf67bb7675bb4cadab976063f

commit 0b4fa617f4023d1bf67bb7675bb4cadab976063f
Author: James Cook <jamescook@chromium.org>
Date: Wed Sep 12 21:38:31 2018

autotest: Remove desktopui_MashLogin from bvt-perbuild

We have perbuild testing via tast ui.MashLogin.

Also update the bug label.

BUG= chromium:847901 
TEST=This is the test

Change-Id: I642ce1c8fe628cdf34da8b24079ce617751b893e
Reviewed-on: https://chromium-review.googlesource.com/1222255
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: James Cook <jamescook@chromium.org>

[modify] https://crrev.com/0b4fa617f4023d1bf67bb7675bb4cadab976063f/client/site_tests/desktopui_MashLogin/control

I think we also need a Chrome-side way to disable Tast ui.MashLogin if it fails or gets flaky. Otherwise we have to wait for a Chrome PFQ round-trip to uprev vm_sanity and/or ui.MashLogin.

Numbering action items for tracking:
1. Run Tast ui.MashLogin in VMs on chrome CQ (works today)
2. Don't run Tast ui.MashLogin on hardware via vm_sanity (in CQ)
3. Take autotest desktopui_MashLogin out of bvt-perbuild (done)
4. Add Tast ui.SingleProcessMashLogin (in CQ)
5. Chrome-side escape hatch to disable/skip mash tests

Cc: bpastene@chromium.org
Longer term maybe we should have a separate build target for ui.MashLogin that chrome sheriffs can mark as experimental or disable if it starts to fail.

I think vm_sanity should simply cover basic functionality that we don't expect to break? I think waterfall management will also be easier for chromium sheriffs. Wdyt?
If you only want to cover the most basic functionality, you could switch to a very short hardcoded list of tests that should be run, e.g.

  local_test_runner ui.ChromeLogin ui.MashLogin

From the perspective of preventing Chrome changes from breaking the Chrome PFQ, there's an argument that can be made in favor of using the same boolean expression that the PFQ builders use, though.
We have  bug 876587  to track having a separate target for Tast, so we'll pull out the Tast portion from vm_sanity as well:
https://bugs.chromium.org/p/chromium/issues/detail?id=876587

Yup, we should use the same expression in the chrome PFQ and chrome waterfall.
> Longer term maybe we should have a separate build target for ui.MashLogin that chrome sheriffs can mark as experimental or disable if it starts to fail.

I'm working on that now in  bug 876587 . Once done, it should be pretty easy to create new test targets for specific tast invocations.
Blockedon: 876587
NextAction: 2018-09-19
Status: Assigned (was: Started)
Summary: Add mash login tests to VMs on Chrome waterfall (was: Add mash login tests to vm_sanity on Chrome waterfall)
OK. With more thought I think we want Chrome CQ and Chrome PFQ to test the same thing. It's just too hard for developers to reason about why they might be different.

I'm OK with vm_sanity just being very basic tests that should never break. That sounds like basic chrome login and crash collection. Maybe we should take all the mash stuff out of vm_sanity?

It sounds like the right thing to do is to wait for a separate target for Tast tests. Marking this blocked on  bug 876587  and I'll follow along there.

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/e1b8beba8fa2861805cb2f8f21695b0f618b18a6

commit e1b8beba8fa2861805cb2f8f21695b0f618b18a6
Author: James Cook <jamescook@chromium.org>
Date: Sat Sep 15 23:21:48 2018

tast-tests: Add SingleProcessMashLogin test

This exercises chrome login with --enable-features=SingleProcessMash.
SingleProcessMash exercises the new mash window service mojo APIs,
but keeps ash system UI code running in the browser process. It's
the next milestone for the mustash team.

BUG= chromium:847901 
TEST=tast -verbose run ui.SingleProcessMashLogin on veyron_minnie and also amd64-generic in a VM

Change-Id: I6b4f6485da207aeb7373c1da33ee97e1ca3f979a
Reviewed-on: https://chromium-review.googlesource.com/1222157
Commit-Ready: James Cook <jamescook@chromium.org>
Tested-by: James Cook <jamescook@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[add] https://crrev.com/e1b8beba8fa2861805cb2f8f21695b0f618b18a6/src/chromiumos/tast/local/bundles/cros/ui/single_process_mash_login.go

The NextAction date has arrived: 2018-09-19
Labels: -M-69 Proj-Mash-MultiProcess
NextAction: 2018-09-26
Update:
* ui.MashLogin and ui.SingleProcessMashLogin are still informational
* Once we have a separate target for Tast tests (or some other way mash tests can be disabled from the Chrome tree) we can add them to the Chrome waterfall

> * Once we have a separate target for Tast tests

I got a chain of CLs up in the air for precisely that atm. They should be landed shortly (ie: ~days).
Project Member

Comment 26 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/d714e134851e37dbe740db2e625d4419d88f8724

commit d714e134851e37dbe740db2e625d4419d88f8724
Author: James Cook <jamescook@chromium.org>
Date: Fri Sep 21 07:51:18 2018

autotest: Add desktopui_SingleProcessMashLogin

This tests chrome OOBE and login similar to desktopui_MashLogin,
but with --enable-features=SingleProcessMash

I'm adding it so it can run on the chrome ToT builders so I get
early warnings of chrome-related failures.

BUG= chromium:847901 
TEST=this is the test
Change-Id: Ic2957627759e132d0e82a284fc36ce0c013a71aa
Reviewed-on: https://chromium-review.googlesource.com/1236659
Commit-Ready: James Cook <jamescook@chromium.org>
Tested-by: James Cook <jamescook@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>

[add] https://crrev.com/d714e134851e37dbe740db2e625d4419d88f8724/client/site_tests/desktopui_SingleProcessMashLogin/control
[add] https://crrev.com/d714e134851e37dbe740db2e625d4419d88f8724/client/site_tests/desktopui_SingleProcessMashLogin/desktopui_SingleProcessMashLogin.py

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/035cfe6ebc308a4f5cf6dc89274e15964f7134cc

commit 035cfe6ebc308a4f5cf6dc89274e15964f7134cc
Author: James Cook <jamescook@chromium.org>
Date: Fri Sep 21 20:34:12 2018

autotest: Add desktopui_SingleProcessMashLogin

This tests chrome OOBE and login similar to desktopui_MashLogin,
but with --enable-features=SingleProcessMash

I'm adding it so it can run on the chrome ToT builders so I get
early warnings of chrome-related failures.

BUG= chromium:847901 
TEST=test_that on veyron_minnie
CQ-DEPEND=CL:1236659
Change-Id: I017fdf31aa4fe141f317fc39addbaad9b0485f70
Reviewed-on: https://chromium-review.googlesource.com/1236220
Commit-Ready: James Cook <jamescook@chromium.org>
Tested-by: James Cook <jamescook@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>

[modify] https://crrev.com/035cfe6ebc308a4f5cf6dc89274e15964f7134cc/chromeos-base/autotest-chrome/autotest-chrome-9999.ebuild

The NextAction date has arrived: 2018-09-26
Cc: -derat@chromium.org jamescook@chromium.org
Owner: derat@chromium.org
Dan volunteered to take this over. The goal is still to run mash tests as closely as possible to Chrome ToT

My understanding of the current status:
* tast.ui.MashLogin and tast.ui.SingleProcessMashLogin are still informational
* They run both in Chrome VM tests and in Chrome OS hardware lab
* Once we have a way that Tast mash tests can be disabled from the Chrome-side tree we can make them important (non-informational)
* Once they are important we can delete the autotests desktopui_MashLogin and desktopui_SingleProcessMashLogin

Ben, once the work tracked in  issue 876587  is completed, will we have the ability to turn off individual Tast tests (e.g. by excluding them from the local_test_runner command line) in the Chrome CQ without needing to wait for an updated VM image?
Yes. The full cmd-line invocation of local_test_runner will be controlled from chromium test/build configs (and *not* a file on the cros VM image).
Blockedon: 891470
Status: Started (was: Assigned)
I think that we're pretty much good to go here now. Removing the "informational" attribute from ui.MashLogin and ui.SingleProcessMashLogin will make them run on the CrOS CQ, the Chrome PFQ, and the Chrome CQ (in chromeos-amd64-generic-rel as described at https://chrome-internal.googlesource.com/chromeos/chromeos-admin/+/master/doc/tast_integration.md#Chrome-Commit-Queue).

To disable them in a hurry in the Chrome CQ without waiting for a new OS image, update the expression within the outer set of parentheses in the chrome_all_tast_tests in //chromeos/BUILD.gn to end with e.g.

  && !"name:ui.*MashLogin"

(just added wildcard support)

You'll probably also want to re-add "informational" (or "disabled") to the test files in http://go/tast-tests so they stop running in the Chrome PFQ and CrOS CQ in that case.

But I still see a few (flaky?) failures in ui.MashLogin. I've reopened  issue 891470  to track that, as it probably needs to be fixed (or a board-specific cause needs to be identified) before this test becomes non-informational.
I'm OK with starting with ui.SingleProcessMashLogin for now.

I wonder if the tests to run should be a whitelist, such that a chrome dev can use cs.chromium.org to search for the test name to discover where to disable it.

Hmm. I think there's probably still some value to automatically running the same tests as on the Chrome PFQ by default, just so we don't need to worry about the two lists getting out of sync.

Ben, would it be easy to make a link with additional information get included if the chrome_all_tast_tests step fails? For example, I could update http://go/tast-failures to also explain how to disable tests in the Chrome CQ.
The chrome PFQ tests must be a superset of the chrome CQ tests. We don't want a situation where we validate LKGM on the PFQ and then the LKGM uprev fails in the chrome CQ. 
If chromium devs won't be able to find these tests by name by searching the chromium codebase, then I think the suite needs to print information about how to disable them.

For now, maybe //chromeos/BUILD.gn could have a comment with the names of some of the tests?

I really don't want to make Chrome sheriffs unhappy with Chrome OS by making our tests hard to disable.

#35: Yes, I think everyone is on the same page. We're just talking about providing a way to quickly disable tests on the Chrome CQ if they're failing there.
Project Member

Comment 38 by bugdroid1@chromium.org, Oct 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/057c8cac02b9d76cffd5860b74d37a1db385dc04

commit 057c8cac02b9d76cffd5860b74d37a1db385dc04
Author: Daniel Erat <derat@chromium.org>
Date: Mon Oct 15 23:02:01 2018

tast-tests: Make ui.SingleProcessMashLogin important.

Remove the "informational" attribute from the
ui.SingleProcessMashLogin test so it will start running on
the CrOS and Chrome CQs and the Chrome PFQ.

This test doesn't have any recent failures on -release
builders.

BUG= chromium:847901 
TEST=none

Change-Id: I014dfd04a500284ef5b7b992cdc65fc8ea77a857
Reviewed-on: https://chromium-review.googlesource.com/1279312
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>

[modify] https://crrev.com/057c8cac02b9d76cffd5860b74d37a1db385dc04/src/chromiumos/tast/local/bundles/cros/ui/single_process_mash_login.go

ui.MashLogin is passing consistently on -release builders after https://crrev.com/c/1281682 disabled the command-line check, so let's give it another try...
Project Member

Comment 40 by bugdroid1@chromium.org, Oct 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/a97923dd8617ddeccc462c3d4a77a98a63f7fe03

commit a97923dd8617ddeccc462c3d4a77a98a63f7fe03
Author: Daniel Erat <derat@chromium.org>
Date: Fri Oct 19 05:38:04 2018

tast-tests: Make ui.MashLogin important.

Similar to what was done earlier with
ui.SingleProcessMashLogin, remove the "informational"
attribute from the ui.MashLogin test so it will start
running on the CrOS and Chrome CQs and the Chrome PFQ.

This test has been consistently passing on -release builders
since it was updated to not check Chrome command lines.

BUG= chromium:847901 
TEST=none

Change-Id: I4240dfb7beeee1ec51652af865b86e19e0f96b43
Reviewed-on: https://chromium-review.googlesource.com/1287810
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>

[modify] https://crrev.com/a97923dd8617ddeccc462c3d4a77a98a63f7fe03/src/chromiumos/tast/local/bundles/cros/ui/mash_login.go

Status: Verified (was: Started)
Both ui.MashLogin and ui.SingleProcessMashLogin have been passing reliably, so I'm declaring success!
Note that only ui.SingleProcessMashLogin is running on chrome's CQ atm.  Bug 897540  likely needs to be fixed before ui.MashLogin starts running as well. (achuith's got a CL up, should be soon)
Blockedon: 897540
Status: Started (was: Verified)
Thanks, I'll keep this open to track reenabling ui.MashLogin.
Status: Verified (was: Started)
The test was re-enabled. (It was disabled again later, but that's tracked by issue 909736.)
Project Member

Comment 45 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/fb63c0cce8436542a06d2cac3ad07fa24de14080

commit fb63c0cce8436542a06d2cac3ad07fa24de14080
Author: Shuhei Takahashi <nya@chromium.org>
Date: Thu Dec 06 03:27:56 2018

Revert "tast-tests: Make ui.MashLogin important."

This reverts commit a97923dd8617ddeccc462c3d4a77a98a63f7fe03.

Reason for revert: Chrome OS CQ failed three times in a row. crbug.com/912228

Original change's description:
> tast-tests: Make ui.MashLogin important.
> 
> Similar to what was done earlier with
> ui.SingleProcessMashLogin, remove the "informational"
> attribute from the ui.MashLogin test so it will start
> running on the CrOS and Chrome CQs and the Chrome PFQ.
> 
> This test has been consistently passing on -release builders
> since it was updated to not check Chrome command lines.
> 
> BUG= chromium:847901 
> TEST=none
> 
> Change-Id: I4240dfb7beeee1ec51652af865b86e19e0f96b43
> Reviewed-on: https://chromium-review.googlesource.com/1287810
> Commit-Ready: Dan Erat <derat@chromium.org>
> Tested-by: Dan Erat <derat@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>

Bug:  chromium:847901 
Change-Id: I20c6cc8db233b24d66292b5955469368e5ec820e
Reviewed-on: https://chromium-review.googlesource.com/c/1364592
Reviewed-by: David Burger <dburger@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>
Tested-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/fb63c0cce8436542a06d2cac3ad07fa24de14080/src/chromiumos/tast/local/bundles/cros/ui/mash_login.go

Sign in to add a comment