Make test_net_info a bit more robust.

infra/services/sysmon/test/system_metrics_test.py

 # Copyright (c) 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import collections
 import os
 import sys
 import unittest
 
 import mock
@@ skipping 93 matching lines @@
   def test_mem_info(self):
     system_metrics.get_mem_info()
 
     free = system_metrics.mem_free.get()
     total = system_metrics.mem_total.get()
 
     self.assertIsNotNone(free)
     self.assertIsNotNone(total)
     self.assertLessEqual(free, total)
 
-  # See http://crbug.com/624369
-  def DISABLED_test_net_info(self):  # pragma: no cover
+  def test_net_info(self):  # pragma: no cover

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

Remove the pragma - here and in system_metrics.py.

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

Oops thanks

     system_metrics.get_net_info()

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

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.

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

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

 
+    interface_names = [fields[0][1]
+                       for fields, _
+                       in system_metrics.net_up.get_all()]
+    self.assertGreater(len(interface_names), 0)
+
     # A network interface that should always be present.
     if sys.platform == 'win32':  # pragma: no cover
-      interface = 'Local Area Connection'
+      self.assertTrue(
+          any(x.startswith('Local Area Connection') for x in interface_names),

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

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]

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.

+          interface_names)
     elif sys.platform == 'darwin':  # pragma: no cover
-      interface = 'en0'
+      self.assertTrue(
+          any(x.startswith('en') for x in interface_names),
+          interface_names)
     else:  # pragma: no cover
-      interface = 'lo'
+      self.assertIn('lo', interface_names)
 
-    labels = {'interface': interface}
+    labels = {'interface': interface_names[0]}
 
     self.assertIsNotNone(system_metrics.net_up.get(labels))
     self.assertIsNotNone(system_metrics.net_down.get(labels))
     self.assertIsNotNone(system_metrics.net_err_up.get(labels))
     self.assertIsNotNone(system_metrics.net_err_down.get(labels))
     self.assertIsNotNone(system_metrics.net_drop_up.get(labels))
     self.assertIsNotNone(system_metrics.net_drop_down.get(labels))
 
   def test_proc_info(self):
     system_metrics.get_proc_info()
 
     self.assertGreater(system_metrics.proc_count.get(), 10)
 
     if os.name == 'posix':  # pragma: no cover
       self.assertGreater(
           system_metrics.load_average.get(fields={'minutes': 1}), 0)
       self.assertGreater(
           system_metrics.load_average.get(fields={'minutes': 5}), 0)
       self.assertGreater(
           system_metrics.load_average.get(fields={'minutes': 15}), 0)
 
   def test_get_unix_time(self):
     system_metrics.get_unix_time()
     self.assertTrue(
       1464000000000 < system_metrics.unix_time.get() < 9464000000000)