ponty / PyVirtualDisplay

Python wrapper for Xvfb, Xephyr and Xvnc
BSD 2-Clause "Simplified" License
709 stars 78 forks source link

0.2.3 broken on Windows #40

Closed surfaceowl closed 5 years ago

surfaceowl commented 5 years ago

PyVirtualDisplay is no longer working on Windows after PR #35. We do selenium testing in both Linux and Windows. Everything works great with PyVirtualDisplay v0.21, but fails with timeouts when we use v0.2.3.

Per the PyVirtualDisplay listing on pypi, it is an OS Independent library... so we need to change this useful check on the X Server so PyVirtualDisplay still words cross-platform.

This PR added a check to make sure the X Server used is solely used by PyVirtualDisplay to avoid a race condition. This is a helpful change. Unfortunately, the OS library used was fcntl, which is only available on Unix/Linux , and that breaks cross-platform ability of PyVirtualDisplay >0.2.1.

Short term, in abstractdisplay.py we could change line 46 with functionality to only do this check if NOT on a windows machine.
current code:

if self.check_startup:
....do stuff


Sample code:

import platform
self.check_startup and platform.system() != "Windows":
....do stuff
else:
....pass

Long Term - Could fcntl be replaced with a cross-platform library calledportalocker? This adds a python dependency but is fully cross-platform Links on pypi and github.

I'm not familiar with the details of PyVirtualDisplay, but would be glad to help draft a fix and do regression testing on both Windows and Linux.

ponty commented 5 years ago

I didn't know this lib can be used on Windows. What is the error message? ImportError? In this case my solution would be: https://github.com/benediktschmitt/py-filelock/blob/master/filelock.py#L47

try:
    import fcntl
except ImportError:
    fcntl = None
surfaceowl commented 5 years ago

@ponty --- yeah, we've been using this lib for about two years now.

The error message is ModuleNotFoundError- sorry didn't include earlier:

...\venv\lib\site-packages\pyvirtualdisplay\abstractdisplay.py", line 9, in <module> import fcntl
ModuleNotFoundError: No module named 'fcntl'

Agree the fix needs to be @import level, not around call to fcntl.

ponty commented 5 years ago

Can you please test 43aa3386beb6c667ab085d8a5d14b68b99b51c44 ?

The-Compiler commented 5 years ago

@ponty Maybe the check should be if not fcntl and check_startup: so the warning isn't logged when no check was requested anyways?

ponty commented 5 years ago

Fixed!

The-Compiler commented 5 years ago

Thanks @ponty, 0.2.4 seems to work fine for me! I guess this can be closed then?

ponty commented 5 years ago

OK, thanks