[Telemetry] Consider linux distribution number is 0 when it cannot be parsed

[Telemetry] Consider linux distribution number is 0 when it cannot be parsed

On some linux distributions, the DISTRIB_RELEASE field of
/etc/lsb-release cannot be parsed as a float. In that case, consider it
will be 0

R=
BUG=393022
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291481

[tonyg, 2014-07-11 02:33:45]

https://codereview.chromium.org/381293003/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/linux_platform_backend.py (right):

https://codereview.chromium.org/381293003/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/linux_platform_backend.py:53: version =
float(value)
I think you meant to remove this line.

That kind of points out that this should be unittested. There are some examples
here of how to mock out the contents of /etc/lsb-release to test the output
you're concerned with.

https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[a.renevier, 2014-07-18 18:02:56]

On 2014/07/11 02:33:45, tonyg wrote:

> That kind of points out that this should be unittested. There are some
examples
> here of how to mock out the contents of /etc/lsb-release to test the output
> you're concerned with.
> 


I'm trying to test GetOSVersionName with different mock /etc/lsb-release files.
Let's say one for the current Ubuntu release. And one for the current Archlinux
release. But unfortunately, it doesn't work because GetOSVersionName is only
called once and then the results is cached (@decorators.Cache)
What would be the best way then: only test one version? create two unittests
files? something else?

[tonyg, 2014-07-24 15:23:59]

On 2014/07/18 18:02:56, a.renevier wrote:
> On 2014/07/11 02:33:45, tonyg wrote:
> 
> > That kind of points out that this should be unittested. There are some
> examples
> > here of how to mock out the contents of /etc/lsb-release to test the output
> > you're concerned with.
> > 
> 
> 
> I'm trying to test GetOSVersionName with different mock /etc/lsb-release
files.
> Let's say one for the current Ubuntu release. And one for the current
Archlinux
> release. But unfortunately, it doesn't work because GetOSVersionName is only
> called once and then the results is cached (@decorators.Cache)
> What would be the best way then: only test one version? create two unittests
> files? something else?

I think that the cache should be per-instance of LinuxPlatformBackend (if not,
something is broken). So I'd suggest the unittest just create separate instances
via linux_platform_backend.LinuxPlatformBackend().

[a.renevier, 2014-07-24 20:24:20]

On 2014/07/24 15:23:59, tonyg wrote:

> 
> I think that the cache should be per-instance of LinuxPlatformBackend (if not,
> something is broken). So I'd suggest the unittest just create separate
instances
> via linux_platform_backend.LinuxPlatformBackend().

Looks like things are (a little bit) broken. If I create two
LinuxPlatformBackend in two different functions, they end up having the same id
(probably because they are created at the same memory place by python
interpreter). So, the cache is shared by those two instances.
http://paste.debian.net/111493/
Here is a script that can reproducibly shows the problem

So, I can write the unittests by creating all the LinuxPlatformBackend instances
in the same function. But I'm wondering if should instead try to "fix"
decorators.Cache. For that, I'm thinking I could attach a __CacheUniqueId
property to each object, and use it as part of the hash key. What do you think?

[tonyg, 2014-07-24 22:25:11]

On 2014/07/24 20:24:20, a.renevier wrote:
> On 2014/07/24 15:23:59, tonyg wrote:
> 
> > 
> > I think that the cache should be per-instance of LinuxPlatformBackend (if
not,
> > something is broken). So I'd suggest the unittest just create separate
> instances
> > via linux_platform_backend.LinuxPlatformBackend().
> 
> Looks like things are (a little bit) broken. If I create two
> LinuxPlatformBackend in two different functions, they end up having the same
id
> (probably because they are created at the same memory place by python
> interpreter). So, the cache is shared by those two instances.
> http://paste.debian.net/111493/
> Here is a script that can reproducibly shows the problem
> 
> So, I can write the unittests by creating all the LinuxPlatformBackend
instances
> in the same function. But I'm wondering if should instead try to "fix"
> decorators.Cache. For that, I'm thinking I could attach a __CacheUniqueId
> property to each object, and use it as part of the hash key. What do you
think?

Fixing decorators.Cache would be very useful!! Please do that in a separate
patch though.

[a.renevier, 2014-07-25 20:44:14]

On 2014/07/24 22:25:11, tonyg wrote:

> Fixing decorators.Cache would be very useful!! Please do that in a separate
> patch though.

Submitted in https://codereview.chromium.org/419973004

[tonyg, 2014-07-29 14:20:17]

On 2014/07/25 20:44:14, a.renevier wrote:
> On 2014/07/24 22:25:11, tonyg wrote:
> 
> > Fixing decorators.Cache would be very useful!! Please do that in a separate
> > patch though.
> 
> Submitted in https://codereview.chromium.org/419973004

You probably noticed that I landed an alternate fix. Could you please sync up
and see if it fixes your issue?

[tonyg, 2014-08-05 15:49:16]

lgtm

[commit-bot: I haz the power, 2014-08-05 15:50:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/a.renevier@samsung.com/381293003/20001

[commit-bot: I haz the power, 2014-08-05 19:26:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/42660)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-05 19:30:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-05 19:30:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tonyg, 2014-08-05 19:40:36]

We have to make the presubmit happy:

** Presubmit ERRORS **
Pylint (381 files) (20.65s) failed
************* Module
/mnt/scratch0/b_used/build/slave/linux/build/src/tools/telemetry/telemetry/core/platform/linux_platform_backend_unittest.py
W0223: 13,0:TestBackend: Method 'StartRawDisplayFrameRateMeasurement' is
abstract in class 'LinuxPlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'GetRawDisplayFrameRateMeasurements' is abstract
in class 'LinuxPlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'HasBeenThermallyThrottled' is abstract in class
'LinuxPlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'StopRawDisplayFrameRateMeasurement' is abstract
in class 'LinuxPlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'IsThermallyThrottled' is abstract in class
'LinuxPlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'StartMonitoringPower' is abstract in class
'PlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'StopMonitoringPower' is abstract in class
'PlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'GetSystemCommitCharge' is abstract in class
'PlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'StopVideoCapture' is abstract in class
'PlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'StartVideoCapture' is abstract in class
'PlatformBackend' but is not overridden
W0223: 13,0:TestBackend: Method 'GetSystemTotalPhysicalMemory' is abstract in
class 'PlatformBackend' but is not overridden
W0611: 10,0: Unused import factory

[a.renevier, 2014-08-07 23:17:41]

On 2014/08/05 21:05:58, a.renevier wrote:

This is the same patch, witch stub functions added to TestBackend. Should I try
to resubmit it?

[a.renevier, 2014-08-22 18:53:42]

The CQ bit was checked by a.renevier@samsung.com

[commit-bot: I haz the power, 2014-08-22 18:54:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/a.renevier@samsung.com/381293003/40001

[commit-bot: I haz the power, 2014-08-22 19:50:22]

Committed patchset #3 (40001) as 291481