fix tests

7 files changed, 152 insertions, 27 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index 30fd1eec..c76e4a9a 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -13,6 +13,8 @@ Bug tracker at https://github.com/giampaolo/psutil/issues
 - #755: Process.memory_percent() "memtype" parameter.
 - #758: tests now live in psutil namespace.
 - #760: expose OS constants (LINUX, OSX, etc.)
+- #756: [Linux] disk_io_counters() return 2 new fields: read_merged_count and
+  write_merged_count.
 - #762: new sripts/procsmem.py script.
 
 **Bug fixes**
@@ -31,6 +33,8 @@ Bug tracker at https://github.com/giampaolo/psutil/issues
 - #759: [Linux] Process.memory_maps() may return paths ending with " (deleted)"
 - #761: [Windows] psutil.boot_time() wraps to 0 after 49 days.
 - #764: [NetBSD] fix compilation on NetBSD-6.x.
+- #767: [Linux] disk_io_counters() may raise ValueError on 2.6 kernels and it's
+  broken on 2.4 kernels.
 
 
 3.4.2 - 2016-01-20
diff --git a/docs/index.rst b/docs/index.rst
index eb8b283b..94cc623d 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -275,6 +275,11 @@ Disks
   - **read_time**: time spent reading from disk (in milliseconds)
   - **write_time**: time spent writing to disk (in milliseconds)
 
+  On Linux we get the following extra fields:
+
+  - **read_merged_count** (Linux): number of merged reads (see `iostat doc <https://www.kernel.org/doc/Documentation/iostats.txt>`__).
+  - **write_merged_count** (Linux): number of merged writes (see `iostat doc <https://www.kernel.org/doc/Documentation/iostats.txt>`__).
+
   If *perdisk* is ``True`` return the same information for every physical disk
   installed on the system as a dictionary with partition names as the keys and
   the namedtuple described above as the values.
@@ -290,6 +295,9 @@ Disks
      'sda2': sdiskio(read_count=18707, write_count=8830, read_bytes=6060, write_bytes=3443, read_time=24585, write_time=1572),
      'sdb1': sdiskio(read_count=161, write_count=0, read_bytes=786432, write_bytes=0, read_time=44, write_time=0)}
 
+  .. versionchanged:: 4.0.0 *read_merged_count* and *write_merged_count* were
+     addded on Linux.
+
 Network
 -------
 
diff --git a/psutil/__init__.py b/psutil/__init__.py
index b94c70dd..297ef5cd 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -1799,7 +1799,7 @@ def disk_io_counters(perdisk=False):
     rawdict = _psplatform.disk_io_counters()
     if not rawdict:
         raise RuntimeError("couldn't find any physical disk")
-    nt = getattr(_psplatform, 'sdiskio', _common.sdiskio)
+    nt = getattr(_psplatform, "sdiskio", _common.sdiskio)
     if perdisk:
         for disk, fields in rawdict.items():
             rawdict[disk] = nt(*fields)
diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index b060b1a0..d80dfba2 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -217,6 +217,10 @@ except Exception:
 svmem = namedtuple(
     'svmem', ['total', 'available', 'percent', 'used', 'free',
               'active', 'inactive', 'buffers', 'cached'])
+sdiskio = namedtuple('sdiskio', ['read_count', 'write_count',
+                                 'read_bytes', 'write_bytes',
+                                 'read_time', 'write_time',
+                                 'read_merged_count', 'write_merged_count'])
 
 pmem = namedtuple('pmem', 'rss vms shared text lib data dirty')
 paddrspmem = namedtuple('paddrspmem', ['uss', 'pss', 'swap'])
@@ -575,14 +579,14 @@ class Connections:
             return
         with open_text(file, buffering=BIGGER_FILE_BUFFERING) as f:
             f.readline()  # skip the first line
-            for line in f:
+            for lineno, line in enumerate(f, 1):
                 try:
                     _, laddr, raddr, status, _, _, _, _, _, inode = \
                         line.split()[:10]
                 except ValueError:
                     raise RuntimeError(
-                        "error while parsing %s; malformed line %r" % (
-                            file, line))
+                        "error while parsing %s; malformed line %s %r" % (
+                            file, lineno, line))
                 if inode in inodes:
                     # # We assume inet sockets are unique, so we error
                     # # out if there are multiple references to the
@@ -757,23 +761,44 @@ def disk_io_counters():
     with open_text("%s/diskstats" % get_procfs_path()) as f:
         lines = f.readlines()
     for line in lines:
-        # http://www.mjmwired.net/kernel/Documentation/iostats.txt
+        # OK, this is a bit confusing. The format of /proc/diskstats can
+        # have 3 variations.
+        # On Linux 2.4 each line has always 15 fields, e.g.:
+        # "3     0   8 hda 8 8 8 8 8 8 8 8 8 8 8"
+        # On Linux 2.6+ each line *usually* has 14 fields, and the disk
+        # name is in another position, like this:
+        # "3    0   hda 8 8 8 8 8 8 8 8 8 8 8"
+        # ...unless (Linux 2.6) the line refers to a partition instead
+        # of a disk, in which case the line has less fields (7):
+        # "3    1   hda1 8 8 8 8"
+        # See:
+        # https://www.kernel.org/doc/Documentation/iostats.txt
         fields = line.split()
-        if len(fields) > 7:
-            _, _, name, reads, _, rbytes, rtime, writes, _, wbytes, wtime = \
-                fields[:11]
+        fields_len = len(fields)
+        if fields_len == 15:
+            # Linux 2.4
+            name = fields[3]
+            reads = int(fields[2])
+            (reads_merged, rbytes, rtime, writes, writes_merged,
+                wbytes, wtime) = map(int, fields[4:11])
+        elif fields_len == 14:
+            # Linux 2.6+, line referring to a disk
+            name = fields[2]
+            (reads, reads_merged, rbytes, rtime, writes, writes_merged,
+                wbytes, wtime) = map(int, fields[3:11])
+        elif fields_len == 7:
+            # Linux 2.6+, line referring to a partition
+            name = fields[2]
+            reads, rbytes, writes, wbytes = map(int, fields[3:])
+            rtime = wtime = reads_merged = writes_merged = 0
         else:
-            # from kernel 2.6.0 to 2.6.25
-            _, _, name, reads, rbytes, writes, wbytes = fields
-            rtime, wtime = 0, 0
+            raise ValueError("not sure how to interpret line %r" % line)
+
         if name in partitions:
-            rbytes = int(rbytes) * SECTOR_SIZE
-            wbytes = int(wbytes) * SECTOR_SIZE
-            reads = int(reads)
-            writes = int(writes)
-            rtime = int(rtime)
-            wtime = int(wtime)
-            retdict[name] = (reads, writes, rbytes, wbytes, rtime, wtime)
+            rbytes = rbytes * SECTOR_SIZE
+            wbytes = wbytes * SECTOR_SIZE
+            retdict[name] = (reads, writes, rbytes, wbytes, rtime, wtime,
+                             reads_merged, writes_merged)
     return retdict
 
 
diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py
index 13c2e464..1d19eff6 100644
--- a/psutil/tests/test_linux.py
+++ b/psutil/tests/test_linux.py
@@ -399,18 +399,87 @@ class TestSystemDisks(unittest.TestCase):
                     assert ret
                     self.assertEqual(ret[0].fstype, 'zfs')
 
-    # not sure why (doesn't fail locally)
-    # https://travis-ci.org/giampaolo/psutil/jobs/108629915
-    @unittest.skipIf(TRAVIS, "fails on travis")
-    def test_disk_io_counters_mocked(self):
-        # From kernel 2.6.0 to 2.6.25 /proc/diskstats has less fields;
-        # we test psutil handles this case by setting read_time and
-        # write_time to 0.
+    def test_disk_io_counters_kernel_2_4_mocked(self):
+        # Tests /proc/diskstats parsing format for 2.4 kernels, see:
+        # https://github.com/giampaolo/psutil/issues/767
+        def open_mock(name, *args, **kwargs):
+            if name == '/proc/partitions':
+                return io.StringIO(textwrap.dedent(u"""\
+                    major minor  #blocks  name
+
+                       8        0  488386584 hda
+                    """))
+            elif name == '/proc/diskstats':
+                return io.StringIO(
+                    u("   3     0   1 hda 2 3 4 5 6 7 8 9 10 11 12"))
+            else:
+                return orig_open(name, *args, **kwargs)
+            return orig_open(name, *args)
+
+        orig_open = open
+        patch_point = 'builtins.open' if PY3 else '__builtin__.open'
+        with mock.patch(patch_point, side_effect=open_mock) as m:
+            ret = psutil.disk_io_counters()
+            assert m.called
+            self.assertEqual(ret.read_count, 1)
+            self.assertEqual(ret.read_merged_count, 2)
+            self.assertEqual(ret.read_bytes, 3 * 512)
+            self.assertEqual(ret.read_time, 4)
+            self.assertEqual(ret.write_count, 5)
+            self.assertEqual(ret.write_merged_count, 6)
+            self.assertEqual(ret.write_bytes, 7 * 512)
+            self.assertEqual(ret.write_time, 8)
+
+    def test_disk_io_counters_kernel_2_6_full_mocked(self):
+        # Tests /proc/diskstats parsing format for 2.6 kernels,
+        # lines reporting all metrics:
+        # https://github.com/giampaolo/psutil/issues/767
         def open_mock(name, *args, **kwargs):
-            if name == ('/proc/partitions'):
+            if name == '/proc/partitions':
+                return io.StringIO(textwrap.dedent(u"""\
+                    major minor  #blocks  name
+
+                       8        0  488386584 hda
+                    """))
+            elif name == '/proc/diskstats':
+                return io.StringIO(
+                    u("   3    0   hda 1 2 3 4 5 6 7 8 9 10 11"))
+            else:
                 return orig_open(name, *args, **kwargs)
+            return orig_open(name, *args)
+
+        orig_open = open
+        patch_point = 'builtins.open' if PY3 else '__builtin__.open'
+        with mock.patch(patch_point, side_effect=open_mock) as m:
+            ret = psutil.disk_io_counters()
+            assert m.called
+            self.assertEqual(ret.read_count, 1)
+            self.assertEqual(ret.read_merged_count, 2)
+            self.assertEqual(ret.read_bytes, 3 * 512)
+            self.assertEqual(ret.read_time, 4)
+            self.assertEqual(ret.write_count, 5)
+            self.assertEqual(ret.write_merged_count, 6)
+            self.assertEqual(ret.write_bytes, 7 * 512)
+            self.assertEqual(ret.write_time, 8)
+
+    def test_disk_io_counters_kernel_2_6_limited_mocked(self):
+        # Tests /proc/diskstats parsing format for 2.6 kernels,
+        # where one line of /proc/partitions return a limited
+        # amount of metrics when it bumps into a partition
+        # (instead of a disk). See:
+        # https://github.com/giampaolo/psutil/issues/767
+        def open_mock(name, *args, **kwargs):
+            if name == '/proc/partitions':
+                return io.StringIO(textwrap.dedent(u"""\
+                    major minor  #blocks  name
+
+                       8        0  488386584 hda
+                    """))
+            elif name == '/proc/diskstats':
+                return io.StringIO(
+                    u("   3    1   hda 1 2 3 4"))
             else:
-                return io.StringIO(u("8       1 sda1 2 2 2 2\n"))
+                return orig_open(name, *args, **kwargs)
             return orig_open(name, *args)
 
         orig_open = open
@@ -418,7 +487,14 @@ class TestSystemDisks(unittest.TestCase):
         with mock.patch(patch_point, side_effect=open_mock) as m:
             ret = psutil.disk_io_counters()
             assert m.called
+            self.assertEqual(ret.read_count, 1)
+            self.assertEqual(ret.read_bytes, 2 * 512)
+            self.assertEqual(ret.write_count, 3)
+            self.assertEqual(ret.write_bytes, 4 * 512)
+
+            self.assertEqual(ret.read_merged_count, 0)
             self.assertEqual(ret.read_time, 0)
+            self.assertEqual(ret.write_merged_count, 0)
             self.assertEqual(ret.write_time, 0)
 
 
diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py
index e96cf233..8de14ab3 100644
--- a/psutil/tests/test_process.py
+++ b/psutil/tests/test_process.py
@@ -1883,6 +1883,8 @@ class TestUnicode(unittest.TestCase):
         self.assertIsInstance(path, str)
         self.assertEqual(os.path.normcase(path), os.path.normcase(self.uexe))
 
+    @unittest.skipUnless(hasattr(psutil.Process, "environ"),
+                         "environ not available")
     def test_proc_environ(self):
         env = os.environ.copy()
         env['FUNNY_ARG'] = self.uexe
@@ -1998,6 +2000,8 @@ class TestNonUnicode(unittest.TestCase):
         self.assertIsInstance(path, str)
         self.assertIn(funny_file, encode_path(path))
 
+    @unittest.skipUnless(hasattr(psutil.Process, "environ"),
+                         "environ not available")
     def test_proc_environ(self):
         env = os.environ.copy()
         funny_path = self.temp_directory
diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py
index b1e67d3f..3fd6876a 100644
--- a/psutil/tests/test_system.py
+++ b/psutil/tests/test_system.py
@@ -612,6 +612,14 @@ class TestSystemAPIs(unittest.TestCase):
             self.assertEqual(nt[3], nt.write_bytes)
             self.assertEqual(nt[4], nt.read_time)
             self.assertEqual(nt[5], nt.write_time)
+            if LINUX:
+                self.assertEqual(nt[6], nt.read_merged_count)
+                self.assertEqual(nt[7], nt.write_merged_count)
+                assert nt.read_merged_count >= 0, nt
+                assert nt.write_merged_count >= 0, nt
+            elif BSD:
+                self.assertEqual(nt[6], nt.busy_time)
+                assert nt.busy_time >= 0, nt
             assert nt.read_count >= 0, nt
             assert nt.write_count >= 0, nt
             assert nt.read_bytes >= 0, nt