Skip to content

[FIX] Errors parsing $DISPLAY #2363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jan 19, 2018
Merged

[FIX] Errors parsing $DISPLAY #2363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1a35f44
[FIX] Errors parsing ``$DISPLAY``
oesteban Jan 9, 2018
e1ef2e5
add test
oesteban Jan 9, 2018
6a9bc46
Update CHANGES
oesteban Jan 9, 2018
2383177
fix test name
oesteban Jan 10, 2018
fbac4ee
fix implementation of get_display()
oesteban Jan 10, 2018
4dc5896
use mokeypatch.setenv
oesteban Jan 10, 2018
5de504a
fix test for Travis and Circle
oesteban Jan 10, 2018
1286d1c
Merge remote-tracking branch 'upstream/master' into fix/displaynumber
oesteban Jan 18, 2018
22c1a27
update travis
oesteban Jan 18, 2018
ba25260
Merge upstream/master
effigies Jan 18, 2018
28e2cd6
Merge remote-tracking branch 'upstream/master' into fix/displaynumber
oesteban Jan 19, 2018
9cc40da
revise tests
oesteban Jan 19, 2018
7d80634
roll back travis setting
oesteban Jan 19, 2018
603ae77
Merge upstream/master [skip ci]
effigies Jan 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Upcoming release (0.14.1)
=========================

* FIX: Errors parsing ``$DISPLAY`` (https://github.com/nipy/nipype/pull/2363)
* FIX: MultiProc starting workers at dubious wd (https://github.com/nipy/nipype/pull/2368)
* REF+FIX: Move BIDSDataGrabber to `interfaces.io` + fix correct default behavior (https://github.com/nipy/nipype/pull/2336)
* ENH: Add AFNI interface for 3dConvertDset (https://github.com/nipy/nipype/pull/2337)
Expand Down
11 changes: 5 additions & 6 deletions nipype/utils/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,11 @@ def get_display(self):
def _mock():
pass

# Store a fake Xvfb object
ndisp = int(sysdisplay.split(':')[-1])
# Store a fake Xvfb object. Format - <host>:<display>[.<screen>]
ndisp = sysdisplay.split(':')[-1].split('.')[0]
Xvfb = namedtuple('Xvfb', ['new_display', 'stop'])
self._display = Xvfb(ndisp, _mock)
return sysdisplay
self._display = Xvfb(int(ndisp), _mock)
return self.get_display()
else:
if 'darwin' in sys.platform:
raise RuntimeError(
Expand All @@ -343,8 +343,7 @@ def _mock():
if not hasattr(self._display, 'new_display'):
setattr(self._display, 'new_display',
self._display.vdisplay_num)

return ':%d' % self._display.new_display
return self.get_display()

def stop_display(self):
"""Closes the display if started"""
Expand Down
56 changes: 47 additions & 9 deletions nipype/utils/tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@
spec=['vdisplay_num', 'start', 'stop'], vdisplay_num=2010)


@pytest.mark.parametrize('dispvar', [':12', 'localhost:12', 'localhost:12.1'])
def test_display_parse(monkeypatch, dispvar):
"""Check that when $DISPLAY is defined, the display is correctly parsed"""
config._display = None
config._config.remove_option('execution', 'display_variable')
monkeypatch.setenv('DISPLAY', dispvar)
assert config.get_display() == ':12'
# Test that it was correctly cached
assert config.get_display() == ':12'


@pytest.mark.parametrize('dispnum', range(5))
def test_display_config(monkeypatch, dispnum):
"""Check that the display_variable option is used ($DISPLAY not set)"""
Expand All @@ -46,7 +57,7 @@ def test_display_system(monkeypatch, dispnum):
config._display = None
config._config.remove_option('execution', 'display_variable')
dispstr = ':%d' % dispnum
monkeypatch.setitem(os.environ, 'DISPLAY', dispstr)
monkeypatch.setenv('DISPLAY', dispstr)
assert config.get_display() == dispstr
# Test that it was correctly cached
assert config.get_display() == dispstr
Expand All @@ -58,7 +69,7 @@ def test_display_config_and_system(monkeypatch):
config._display = None
dispstr = ':10'
config.set('execution', 'display_variable', dispstr)
monkeypatch.setitem(os.environ, 'DISPLAY', ':0')
monkeypatch.setenv('DISPLAY', ':0')
assert config.get_display() == dispstr
# Test that it was correctly cached
assert config.get_display() == dispstr
Expand All @@ -72,10 +83,17 @@ def test_display_noconfig_nosystem_patched(monkeypatch):
config._config.remove_option('execution', 'display_variable')
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch)
monkeypatch.setattr(sys, 'platform', value='linux')
assert config.get_display() == ":2010"
# Test that it was correctly cached
assert config.get_display() == ':2010'

# Check that raises in Mac
config._display = None
monkeypatch.setattr(sys, 'platform', value='darwin')
with pytest.raises(RuntimeError):
config.get_display()


def test_display_empty_patched(monkeypatch):
"""
Expand All @@ -85,12 +103,18 @@ def test_display_empty_patched(monkeypatch):
config._display = None
if config.has_option('execution', 'display_variable'):
config._config.remove_option('execution', 'display_variable')
monkeypatch.setitem(os.environ, 'DISPLAY', '')
monkeypatch.setenv('DISPLAY', '')
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch)
monkeypatch.setattr(sys, 'platform', value='linux')
assert config.get_display() == ':2010'
# Test that it was correctly cached
assert config.get_display() == ':2010'

# Check that raises in Mac
config._display = None
monkeypatch.setattr(sys, 'platform', value='darwin')
with pytest.raises(RuntimeError):
config.get_display()

def test_display_noconfig_nosystem_patched_oldxvfbwrapper(monkeypatch):
"""
Expand All @@ -102,10 +126,16 @@ def test_display_noconfig_nosystem_patched_oldxvfbwrapper(monkeypatch):
config._config.remove_option('execution', 'display_variable')
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
monkeypatch.setattr(sys, 'platform', value='linux')
assert config.get_display() == ":2010"
# Test that it was correctly cached
assert config.get_display() == ':2010'

# Check that raises in Mac
config._display = None
monkeypatch.setattr(sys, 'platform', value='darwin')
with pytest.raises(RuntimeError):
config.get_display()

def test_display_empty_patched_oldxvfbwrapper(monkeypatch):
"""
Expand All @@ -115,12 +145,18 @@ def test_display_empty_patched_oldxvfbwrapper(monkeypatch):
config._display = None
if config.has_option('execution', 'display_variable'):
config._config.remove_option('execution', 'display_variable')
monkeypatch.setitem(os.environ, 'DISPLAY', '')
monkeypatch.setenv('DISPLAY', '')
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
monkeypatch.setattr(sys, 'platform', value='linux')
assert config.get_display() == ':2010'
# Test that it was correctly cached
assert config.get_display() == ':2010'

# Check that raises in Mac
config._display = None
monkeypatch.setattr(sys, 'platform', value='darwin')
with pytest.raises(RuntimeError):
config.get_display()

def test_display_noconfig_nosystem_notinstalled(monkeypatch):
"""
Expand All @@ -130,7 +166,7 @@ def test_display_noconfig_nosystem_notinstalled(monkeypatch):
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.delenv('DISPLAY', raising=False)
monkeypatch.setitem(sys.modules, 'xvfbwrapper', None)
with pytest.raises(RuntimeError):
config.get_display()
Expand All @@ -144,13 +180,14 @@ def test_display_empty_notinstalled(monkeypatch):
config._display = None
if config.has_option('execution', 'display_variable'):
config._config.remove_option('execution', 'display_variable')
monkeypatch.setitem(os.environ, 'DISPLAY', '')
monkeypatch.setenv('DISPLAY', '')
monkeypatch.setitem(sys.modules, 'xvfbwrapper', None)
with pytest.raises(RuntimeError):
config.get_display()


@pytest.mark.skipif(not has_Xvfb, reason='xvfbwrapper not installed')
@pytest.mark.skipif('darwin' in sys.platform, reason='macosx requires root for Xvfb')
def test_display_noconfig_nosystem_installed(monkeypatch):
"""
Check that actually uses xvfbwrapper when installed (not mocked)
Expand All @@ -159,14 +196,15 @@ def test_display_noconfig_nosystem_installed(monkeypatch):
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.delenv('DISPLAY', raising=False)
newdisp = config.get_display()
assert int(newdisp.split(':')[-1]) > 1000
# Test that it was correctly cached
assert config.get_display() == newdisp


@pytest.mark.skipif(not has_Xvfb, reason='xvfbwrapper not installed')
@pytest.mark.skipif('darwin' in sys.platform, reason='macosx requires root for Xvfb')
def test_display_empty_installed(monkeypatch):
"""
Check that actually uses xvfbwrapper when installed (not mocked)
Expand All @@ -175,7 +213,7 @@ def test_display_empty_installed(monkeypatch):
config._display = None
if config.has_option('execution', 'display_variable'):
config._config.remove_option('execution', 'display_variable')
monkeypatch.setitem(os.environ, 'DISPLAY', '')
monkeypatch.setenv('DISPLAY', '')
newdisp = config.get_display()
assert int(newdisp.split(':')[-1]) > 1000
# Test that it was correctly cached
Expand All @@ -191,7 +229,7 @@ def test_display_empty_macosx(monkeypatch):
config._display = None
if config.has_option('execution', 'display_variable'):
config._config.remove_option('execution', 'display_variable')
monkeypatch.delitem(os.environ, 'DISPLAY', '')
monkeypatch.delenv('DISPLAY', '')

monkeypatch.setattr(sys, 'platform', 'darwin')
with pytest.raises(RuntimeError):
Expand Down