Make test_net_info a bit more robust.

Make test_net_info a bit more robust.

Also add a get_all() method to metrics to return all field name/values and metric values.

BUG=624369
Committed: https://chromium.googlesource.com/infra/infra/+/4ebc706fd4f94daa887aca89c5ffda4485f0f62d

[dsansome, 2016-06-30 05:55:06]

dsansome@chromium.org changed reviewers:
+ sergeyberezin@chromium.org

[Sergey Berezin, 2016-06-30 19:14:04]

LGTM + comments. Thanks!

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
File infra/services/sysmon/test/system_metrics_test.py (right):

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:114: def test_net_info(self): 
# pragma: no cover
Remove the pragma - here and in system_metrics.py.

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:115:
system_metrics.get_net_info()
It worries me that we have to depend on the actual system network setup in this
test, and thus, have an inherent source of nondeterminism. OTOH, I can't think
of a better way to test that we got the keys correctly out of
psutil.net_io_counters. But short of that, I'd just mock it out.

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:125: any(x.startswith('Local
Area Connection') for x in interface_names),
Do we even need these platform-specific asserts? In the end, we are still
checking whatever happens to be the first interface for metrics. Maybe just
check that we have at least one interface?

[dsansome, 2016-07-01 00:17:54]

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
File infra/services/sysmon/test/system_metrics_test.py (right):

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:114: def test_net_info(self): 
# pragma: no cover
On 2016/06/30 19:14:04, Sergey Berezin wrote:
> Remove the pragma - here and in system_metrics.py.

Oops thanks

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:115:
system_metrics.get_net_info()
On 2016/06/30 19:14:04, Sergey Berezin wrote:
> It worries me that we have to depend on the actual system network setup in
this
> test, and thus, have an inherent source of nondeterminism. OTOH, I can't think
> of a better way to test that we got the keys correctly out of
> psutil.net_io_counters. But short of that, I'd just mock it out.

I think I mocked it originally but pgervais hates mocks :(

https://codereview.chromium.org/2109393002/diff/1/infra/services/sysmon/test/...
infra/services/sysmon/test/system_metrics_test.py:125: any(x.startswith('Local
Area Connection') for x in interface_names),
On 2016/06/30 19:14:04, Sergey Berezin wrote:
> Do we even need these platform-specific asserts? In the end, we are still
> checking whatever happens to be the first interface for metrics. Maybe just
> check that we have at least one interface?

I wanted to check the field values are actually interface names.  If we screwed
that up, or maybe set fields={'interface': ''} or something, we wouldn't notice
without this check.

[dsansome, 2016-07-01 00:17:59]

The CQ bit was checked by dsansome@chromium.org

[dsansome, 2016-07-01 00:17:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyberezin@chromium.org
Link to the patchset: https://codereview.chromium.org/2109393002/#ps20001
(title: "CL comments")

[commit-bot: I haz the power, 2016-07-01 00:18:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 00:21:44]

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

[commit-bot: I haz the power, 2016-07-01 00:21:45]

Try jobs failed on following builders:
  Infra Linux Trusty 64 Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Trusty%...)

[dsansome, 2016-07-01 03:16:24]

The CQ bit was checked by dsansome@chromium.org

[dsansome, 2016-07-01 03:16:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyberezin@chromium.org
Link to the patchset: https://codereview.chromium.org/2109393002/#ps40001
(title: "Add tests")

[commit-bot: I haz the power, 2016-07-01 03:16:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 03:30:50]

Description was changed from

==========
Make test_net_info a bit more robust.

Also add a get_all() method to metrics to return all field name/values and
metric values.

BUG=624369
==========

to

==========
Make test_net_info a bit more robust.

Also add a get_all() method to metrics to return all field name/values and
metric values.

BUG=624369

Committed:
https://chromium.googlesource.com/infra/infra/+/4ebc706fd4f94daa887aca89c5ffd...
==========

[commit-bot: I haz the power, 2016-07-01 03:30:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/4ebc706fd4f94daa887aca89c5ffd...