Rev 4760: More robusts tests for osutils.terminal_width(). in file:///home/vila/src/bzr/bugs/353370-notty-no-term-width/

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Dec 9 10:31:04 GMT 2009


At file:///home/vila/src/bzr/bugs/353370-notty-no-term-width/

------------------------------------------------------------
revno: 4760
revision-id: v.ladeuil+lp at free.fr-20091209103104-3mu4zqvl6myzq4cd
parent: v.ladeuil+lp at free.fr-20091209082155-2ai0k52k996ayibh
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 353370-notty-no-term-width
timestamp: Wed 2009-12-09 11:31:04 +0100
message:
  More robusts tests for osutils.terminal_width().
  
  * bzrlib/tests/test_osutils.py:
  (TestTerminalWidth): Make the tests more robust.
  
  * bzrlib/osutils.py:
  (terminal_width): Document the implemented behavior. Delegate the
  OS specific part to _terminal_size.
  (_ioctl_terminal_size): The unix implementation for
  _terminal_size.
  (_win32_terminal_size): The windows implementation for
  _terminal_size.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-12-09 08:21:55 +0000
+++ b/NEWS	2009-12-09 10:31:04 +0000
@@ -64,11 +64,13 @@
 * Lots of bugfixes for the test suite on Windows. We should once again
   have a test suite with no failures on Windows. (John Arbash Meinel)
 
-* ``osutils.terminal_width()`` obeys the BZR_COLUMNS envrionment
+* ``osutils.terminal_width()`` obeys the BZR_COLUMNS environment
   variable but returns None if the terminal is not a tty (when output is
   redirected for example). Also fixes its usage under OSes that doesn't
-  provide termios.TIOCGWINSZ.  
+  provide termios.TIOCGWINSZ. Make sure the corresponding tests runs on
+  windows too.
   (Joke de Buhr, Vincent Ladeuil, #353370, #62539)
+  (John Arbash Meinel, Vincent Ladeuil, #492561)
 
 * Terminate ssh subprocesses when no references to them remain, fixing
   subprocess and file descriptor leaks.  (Andrew Bennetts, #426662)

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2009-12-04 11:27:57 +0000
+++ b/bzrlib/osutils.py	2009-12-09 10:31:04 +0000
@@ -1342,6 +1342,23 @@
     """Return terminal width.
 
     None is returned if the width can't established precisely.
+
+    The rules are:
+    - if BZR_COLUMNS is set, returns its value
+    - if there is no controlling terminal, returns None
+    - if COLUMNS is set, returns its value,
+
+    From there, we need to query the OS to get the size of the controlling
+    terminal.
+
+    Unices:
+    - get termios.TIOCGWINSZ
+    - if an error occurs or a negative value is obtained, returns None
+
+    Windows:
+    
+    - win32utils.get_console_size() decides,
+    - returns None on error (provided default value)
     """
 
     # If BZR_COLUMNS is set, take it, user is always right
@@ -1355,30 +1372,50 @@
         # Don't guess, setting BZR_COLUMNS is the recommended way to override.
         return None
 
-    if sys.platform == 'win32':
-        return win32utils.get_console_size(defaultx=None)[0]
-
-    # If COLUMNS is set, take it, the terminal knows better (at least under
-    # emacs, COLUMNS gives an accurate answer while the fcntl.ioctl call below
-    # doesn't) -- vila 20091204
+    # If COLUMNS is set, take it, the terminal knows better (even inside a
+    # given terminal, the application can decide to set COLUMNS to a lower
+    # value (splitted screen) or a bigger value (scroll bars))
     try:
         return int(os.environ['COLUMNS'])
     except (KeyError, ValueError):
         pass
 
+    width, height = _terminal_size(None, None)
+    if width <= 0:
+        # Consider invalid values as meaning no width
+        return None
+
+    return width
+
+
+def _win32_terminal_size(width, height):
+    width, height = win32utils.get_console_size(defaultx=width, defaulty=height)
+    return width, height
+
+
+def _ioctl_terminal_size(width, height):
     try:
         import struct, fcntl, termios
         s = struct.pack('HHHH', 0, 0, 0, 0)
         x = fcntl.ioctl(1, termios.TIOCGWINSZ, s)
-        width = struct.unpack('HHHH', x)[1]
+        width, height = struct.unpack('HHHH', x)[0:2]
     except (IOError, AttributeError):
-        return None
-
-    if width <= 0:
-        # Consider invalid values as meaning no width
-        return None
-
-    return width
+        pass
+    return width, height
+
+_terminal_size = None
+"""Returns the terminal size as (width, height).
+
+:param width: Default value for width.
+:param height: Default value for height.
+
+This is defined specifically for each OS and query the size of the controlling
+terminal. If any error occurs, the provided default values should be returned.
+"""
+if sys.platform == 'win32':
+    _terminal_size = _win32_terminal_size
+else:
+    _terminal_size = _ioctl_terminal_size
 
 
 def supports_executable():

=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py	2009-12-09 08:21:55 +0000
+++ b/bzrlib/tests/test_osutils.py	2009-12-09 10:31:04 +0000
@@ -1928,44 +1928,58 @@
 
 class TestTerminalWidth(tests.TestCase):
 
+    def replace_stdout(self, new):
+        orig_stdout = sys.stdout
+        def restore():
+            sys.stdout = orig_stdout
+        self.addCleanup(restore)
+        sys.stdout = new
+
+    def replace__terminal_size(self, new):
+        orig__terminal_size = osutils._terminal_size
+        def restore():
+            osutils._terminal_size = orig__terminal_size
+        self.addCleanup(restore)
+        osutils._terminal_size = new
+
     def test_default_values(self):
-        self.assertEquals(80, osutils.default_terminal_width)
+        self.assertEqual(80, osutils.default_terminal_width)
 
     def test_defaults_to_BZR_COLUMNS(self):
         # BZR_COLUMNS is set by the test framework
-        self.assertEquals('80', os.environ['BZR_COLUMNS'])
+        self.assertNotEqual('12', os.environ['BZR_COLUMNS'])
         os.environ['BZR_COLUMNS'] = '12'
-        self.assertEquals(12, osutils.terminal_width())
+        self.assertEqual(12, osutils.terminal_width())
 
     def test_falls_back_to_COLUMNS(self):
         del os.environ['BZR_COLUMNS']
+        self.assertNotEqual('42', os.environ['COLUMNS'])
         os.environ['COLUMNS'] = '42'
-        self.assertEquals(42, osutils.terminal_width())
+        self.assertEqual(42, osutils.terminal_width())
 
     def test_tty_default_without_columns(self):
         del os.environ['BZR_COLUMNS']
         del os.environ['COLUMNS']
-        orig_stdout = sys.stdout
-        def restore():
-            sys.stdout = orig_stdout
-        self.addCleanup(restore)
 
         class I_am_a_tty(object):
             def isatty(self):
                 return True
 
-        sys.stdout = I_am_a_tty()
-        self.assertEquals(None, osutils.terminal_width())
+        def terminal_size(w, h):
+            return 42, 42
+
+        self.replace_stdout(I_am_a_tty())
+        # We need to override the osutils definition as it depends on the
+        # running environment that we can't control (PQM running without a
+        # controlling terminal is one example).
+        self.replace__terminal_size(terminal_size)
+        self.assertEqual(42, osutils.terminal_width())
 
     def test_non_tty_default_without_columns(self):
         del os.environ['BZR_COLUMNS']
         del os.environ['COLUMNS']
-        orig_stdout = sys.stdout
-        def restore():
-            sys.stdout = orig_stdout
-        self.addCleanup(restore)
-        sys.stdout = None
-        self.assertEquals(None, osutils.terminal_width())
+        self.replace_stdout(None)
+        self.assertEqual(None, osutils.terminal_width())
 
     def test_no_TIOCGWINSZ(self):
         self.requireFeature(TermIOSFeature)
@@ -1983,4 +1997,5 @@
             del termios.TIOCGWINSZ
         del os.environ['BZR_COLUMNS']
         del os.environ['COLUMNS']
-        self.assertIs(None, osutils.terminal_width())
+        # Whatever the result is, if we don't raise an exception, it's ok.
+        osutils.terminal_width()



More information about the bazaar-commits mailing list