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

Issue 808085 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Regression: power.CheckStatus TastVMTest failing on betty-release

Project Member Reported by norvez@chromium.org, Feb 1 2018

Issue description


It started failing 2 days ago:
https://luci-milo.appspot.com/buildbot/chromeos/betty-release/

Assigning to sheriffs for triage.
 
Cc: derat@chromium.org
The actual error :

2018/02/01 05:49:18 Started test power.CheckStatus
2018/02/01 05:49:18 [07:49:18.309] Error at check_status.go:27: Failed to get power status: didn't find two fields in line "battery_status"
2018/02/01 05:49:18 [07:49:18.309] Stack trace:
chromiumos/tast/local/bundles/cros/power.CheckStatus(0xc42008c940)
	/build/betty/tmp/portage/chromeos-base/tast-local-tests-cros-0.0.1-r5/work/tast-local-tests-cros-0.0.1/src/chromiumos/tast/local/bundles/cros/power/check_status.go:27 +0xe9
chromiumos/tast/testing.(*Test).Run.func1(0xc42008c940, 0xc420134070, 0xc42008fce0)
	/build/betty/usr/lib/gopath/src/chromiumos/tast/testing/test.go:79 +0x66
created by chromiumos/tast/testing.(*Test).Run
	/build/betty/usr/lib/gopath/src/chromiumos/tast/testing/test.go:72 +0xb3
2018/02/01 05:49:18 Completed test power.CheckStatus in 20ms with 1 error(s)

Not having battery_status in a VM seems somewhat expected ...
I don't see recent changes in the Tast power code though.

But there is one in the dump_power_status exe called by the tast code at 
https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/master/src/chromiumos/tast/local/power/status.go?autodive=0%2F#34
it's :
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/886834
it definitely adding the battery_status.

Might just have nothing inside when lacking battery on the VM ?

Adding Dan the tastmaster and beta-taster
Owner: derat@chromium.org
Assigning to dan.

Comment 3 by derat@chromium.org, Feb 1 2018

Components: OS>Kernel>Power Tests>Tast
Status: Started (was: Unconfirmed)
Thanks for filing this and sending it my way!

I think I broke this with https://crrev.com/c/886834, which made dump_power_status log some additional data (including string battery status) for factory tests (see  issue 805275 ). I thought I'd verified that everyone who parses this would be okay with strings containing spaces, but it looks like I missed some of my own code. :-P Sorry, I'll update the test right now.

Comment 4 by derat@chromium.org, Feb 1 2018

Looking more closely, I think that the problem isn't the presence of spaces but rather that the battery status is empty in the VM. https://crrev.com/c/886834 should fix it, in any case.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 2 2018

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

commit d6be583ee93c7a2bdb799df32209f50b4aed69cf
Author: Daniel Erat <derat@chromium.org>
Date: Fri Feb 02 15:03:09 2018

tast-tests: Make power.GetStatus handle string values.

Update chromiumos/tast/power's GetStatus function to handle
string values that are now printed by dump_power_status.
Also add LinePowerCurrent, LinePowerType, and BatteryStatus
fields to the Status struct.

BUG= chromium:808085 
TEST=string values are reported correctly and
     power.CheckStatus test passes when battery status is
     empty (e.g. on a VM)

Change-Id: Iecf9d563721d4e90cad34837c922031aac17d3d6
Reviewed-on: https://chromium-review.googlesource.com/898222
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/d6be583ee93c7a2bdb799df32209f50b4aed69cf/src/chromiumos/tast/local/power/status.go

Status: Verified (was: Started)
betty-release cycled green.

Sign in to add a comment