search

jaseg / python-mpv

Python interface to the awesome mpv media player
https://git.jaseg.de/python-mpv.git
Other
531 stars 67 forks source link

Please replace unmaintained xvfbwrapper (e.g. with PyVirtualDisplay) #249

Closed mgorny closed 1 year ago

mgorny commented 1 year ago

xvfbwrapper is no longer maintained — it has no commit activity since 2019, it has annoying race conditions and nobody to merge the fixes. Would you consider replacing it with another package?

My personal recommendation would be PyVirtualDisplay — it is actively maintained and uses a more robust approach to starting Xvfb (-displayfd). If you agree, I can prepare a pull request doing the replacement.

We'd like to remove xvfbwrapper entirely from Gentoo.

jaseg commented 1 year ago

Sounds good. If you'd open a PR that would be awesome, else I'll do it when I have capacity.

mgorny commented 1 year ago

Done. It seems that it was a drop-in replacement here ;-).

mgorny commented 1 year ago

That said, I do wonder why you're calling Xvfb from within tests, if you run the whole test suite inside a Xvfb in the CI pipeline anyway.

jaseg commented 1 year ago

The reason for the per-test Xvfb-wrapper is for when you run the tests locally. Otherwise when you run the testsuite it looks like you just navigated to a pirate website without an adblocker.

mgorny commented 1 year ago

The reason for the per-test Xvfb-wrapper is for when you run the tests locally. Otherwise when you run the testsuite it looks like you just navigated to a pirate website without an adblocker.

Hmm but a "global" instance of Xvfb for the whole of the test suite (started via conftest.py) would work for that as well, wouldn't it?

jaseg commented 1 year ago

You're right, that would probably work too and might save a few seconds per run. I'll try it out the next time I mess around with the unit tests.