make code work with current xvfbwrapper and make it bwd compatible. add tests for this

Author: oesteban

nipype/utils/config.py

@@ -266,7 +266,7 @@ def get_display(self):
         #                    shell=True, stdout=sp.DEVNULL))
 
         if self._display is not None:
-            return ':%d' % self._display.vdisplay_num
+            return ':%d' % self._display.new_display
 
         sysdisplay = None
         if self._config.has_option('execution', 'display_variable'):
@@ -281,7 +281,7 @@ def _mock():
 
             # Store a fake Xvfb object
             ndisp = int(sysdisplay.split(':')[-1])
-            Xvfb = namedtuple('Xvfb', ['vdisplay_num', 'stop'])
+            Xvfb = namedtuple('Xvfb', ['new_display', 'stop'])
             self._display = Xvfb(ndisp, _mock)
             return sysdisplay
         else:
@@ -306,12 +306,12 @@ def _mock():
             self._display = Xvfb(nolisten='tcp')
             self._display.start()
 
-            # Older versions of Xvfb used vdisplay_num
-            if hasattr(self._display, 'vdisplay_num'):
-                return ':%d' % self._display.vdisplay_num
+            # Older versions of xvfbwrapper used vdisplay_num
+            if not hasattr(self._display, 'new_display'):
+                setattr(self._display, 'new_display',
+                        self._display.vdisplay_num)
 
-            if hasattr(self._display, 'new_display'):
-                return ':%d' % self._display.new_display
+            return ':%d' % self._display.new_display
 
     def stop_display(self):
         """Closes the display if started"""

nipype/utils/tests/test_config.py

@@ -15,8 +15,16 @@
 except ImportError:
     has_Xvfb = False
 
-xvfbpatch = MagicMock()
-xvfbpatch.Xvfb.return_value = MagicMock(vdisplay_num=2010)
+# Define mocks for xvfbwrapper. Do not forget the spec to ensure that
+# hasattr() checks return False with missing attributes.
+xvfbpatch = MagicMock(spec=['Xvfb'])
+xvfbpatch.Xvfb.return_value = MagicMock(spec=['new_display', 'start', 'stop'],
+                                        new_display=2010)
+
+# Mock the legacy xvfbwrapper.Xvfb class (changed display attribute name)
+xvfbpatch_old = MagicMock(spec=['Xvfb'])
+xvfbpatch_old.Xvfb.return_value = MagicMock(spec=['vdisplay_num', 'start', 'stop'],
+                                            vdisplay_num=2010)
 
 
 @pytest.mark.parametrize('dispnum', range(5))
@@ -71,6 +79,32 @@ def test_display_empty_patched(monkeypatch):
     assert config.get_display() == ':2010'
 
 
+def test_display_noconfig_nosystem_patched_oldxvfbwrapper(monkeypatch):
+    """
+    Check that when no $DISPLAY nor option are specified,
+    a virtual Xvfb is used (with a legacy version of xvfbwrapper).
+    """
+    config._display = None
+    if config.has_option('execution', 'display_variable'):
+        config._config.remove_option('execution', 'display_variable')
+    monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
+    monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
+    assert config.get_display() == ":2010"
+
+
+def test_display_empty_patched_oldxvfbwrapper(monkeypatch):
+    """
+    Check that when $DISPLAY is empty string and no option is specified,
+    a virtual Xvfb is used (with a legacy version of xvfbwrapper).
+    """
+    config._display = None
+    if config.has_option('execution', 'display_variable'):
+        config._config.remove_option('execution', 'display_variable')
+    monkeypatch.setitem(os.environ, 'DISPLAY', '')
+    monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
+    assert config.get_display() == ':2010'
+
+
 def test_display_noconfig_nosystem_notinstalled(monkeypatch):
     """
     Check that an exception is raised if xvfbwrapper is not installed