[Telemetry] Fix C-State stat reading for Note 3 and S5.

[Telemetry] Fix C-State stat reading for Note 3 and S5.

On some Android devices, the sysfs-based C-State info contains extra blank
lines which will cause an exception in C-State stat parser in Android platform
backend if the power metric and a Monsoon Power Monitor is in use. More
precisely, /sys/devices/system/cpu/cpu*/cpuidle/state*/name contains two
trailing newlines on at least Samsung Galaxy Note 3 and S5.

This patch filters out these blank lines from the sysfs output and fixes nearby
style.

BUG=421008
TEST=SysfsPowerMonitorMonitorTest.testGetCpuStateForAndroidDevices

Committed: https://crrev.com/a30ca3cd77a8a33b839c59973c414692b19f872b
Cr-Commit-Position: refs/heads/master@{#300347}

[v.putkinen, 2014-10-08 11:06:48]

v.putkinen@partner.samsung.com changed reviewers:
+ dtu@chromium.org, ernstm@chromium.org, nednguyen@google.com,
tmandel@chromium.org, tonyg@chromium.org

[v.putkinen, 2014-10-08 11:09:22]

Hi, I'd like you to review my patch.

[nednguyen, 2014-10-08 14:48:08]

Can you add this case to sysfs_power_monitor_unittest.py?

[v.putkinen, 2014-10-10 15:40:17]

On 2014/10/08 14:48:08, nednguyen wrote:
> Can you add this case to sysfs_power_monitor_unittest.py?

I added a test case for checking that SysfsPowerMonitor.GetCpuState returns a
valid string for LinuxBasedPlatformBackend.ParseCStateSample when given input
from Note 3's sysfs.

[nednguyen, 2014-10-10 16:11:53]

https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py
(right):

https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py:216:

Instead of the assertions below, how about asserting that
sysfsmon.ParseFreqSample(state) sysfsmon.ParseCStateSample(state) returns the
expected results ?

[v.putkinen, 2014-10-13 09:27:38]

On 2014/10/10 16:11:53, nednguyen wrote:
>
https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
> File
>
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py
> (right):
> 
>
https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
>
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py:216:
> 
> Instead of the assertions below, how about asserting that
> sysfsmon.ParseFreqSample(state) sysfsmon.ParseCStateSample(state) returns the
> expected results ?

SysfsPowerMonitor does not have a ParseCStateSample method, but I changed the
test to call AndroidPlatformBackend.ParseCStateSample and check the result. It
feels somehow wrong to depend on another module in unit tests, but I guess it is
justified here.

[nednguyen, 2014-10-13 16:20:35]

On 2014/10/13 09:27:38, v.putkinen wrote:
> On 2014/10/10 16:11:53, nednguyen wrote:
> >
>
https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
> > File
> >
>
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py
> > (right):
> > 
> >
>
https://codereview.chromium.org/635313002/diff/20001/tools/telemetry/telemetr...
> >
>
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor_unittest.py:216:
> > 
> > Instead of the assertions below, how about asserting that
> > sysfsmon.ParseFreqSample(state) sysfsmon.ParseCStateSample(state) returns
the
> > expected results ?
> 
> SysfsPowerMonitor does not have a ParseCStateSample method, but I changed the
> test to call AndroidPlatformBackend.ParseCStateSample and check the result. It
> feels somehow wrong to depend on another module in unit tests, but I guess it
is
> justified here.
It can be justified if the test case here is meant to cover the C-State stat
reading for Note 3 and S5, which are android devices. Maybe renaming the test
slightly different: testGetCpuStateForAndroidDevices?

LGTM

[v.putkinen, 2014-10-15 11:58:44]

On 2014/10/13 16:20:35, nednguyen wrote:
> Maybe renaming the test
> slightly different: testGetCpuStateForAndroidDevices?

Renamed.

[nednguyen, 2014-10-15 15:30:47]

On 2014/10/15 11:58:44, v.putkinen wrote:
> On 2014/10/13 16:20:35, nednguyen wrote:
> > Maybe renaming the test
> > slightly different: testGetCpuStateForAndroidDevices?
> 
> Renamed.

lgtm again

[v.putkinen, 2014-10-20 14:00:08]

v.putkinen@partner.samsung.com changed reviewers:
+ chrishenry@google.com, marja@chromium.org, nduca@chromium.org,
skyostil@chromium.org

[v.putkinen, 2014-10-20 14:00:09]

Please review.

[marja, 2014-10-20 15:21:28]

Hmm, what do you mean, you already have an l-g-t-m from an owner?

[v.putkinen, 2014-10-20 20:15:20]

The CQ bit was checked by v.putkinen@partner.samsung.com

[v.putkinen, 2014-10-20 20:15:26]

The CQ bit was unchecked by v.putkinen@partner.samsung.com

[v.putkinen, 2014-10-20 20:15:36]

The CQ bit was checked by v.putkinen@partner.samsung.com

[v.putkinen, 2014-10-20 20:15:53]

The CQ bit was unchecked by v.putkinen@partner.samsung.com

[v.putkinen, 2014-10-20 20:16:40]

The CQ bit was checked by v.putkinen@partner.samsung.com

[commit-bot: I haz the power, 2014-10-20 20:18:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/635313002/120001

[v.putkinen, 2014-10-20 20:30:38]

On 2014/10/20 15:21:28, marja wrote:
> Hmm, what do you mean, you already have an l-g-t-m from an owner?

Whoops, I didn't realize I could check the commit check box myself. Sorry about
the spam.

[commit-bot: I haz the power, 2014-10-20 22:04:27]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2014-10-20 22:06:02]

Patchset 4 (id:??) landed as
https://crrev.com/a30ca3cd77a8a33b839c59973c414692b19f872b
Cr-Commit-Position: refs/heads/master@{#300347}