jaseg / python-mpv

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

testsuite failure on Debian for version 1.0.1 #209

Closed baldurmen closed 2 years ago

baldurmen commented 2 years ago

Hi!

Running python-mpv 1.01 on Debian, the following tests fail:

The error I get is X connection to :1267492075 broken (explicit kill or server shutdown).. I'm not actually sure I understand what is happening, so I'm reaching out :)

baldurmen commented 2 years ago

Oh, and it seems test_event_callback is flaky? I've ran the testsuite a couple of times (10-12?) the same exact way and it failed once out of the blue with this error:

======================================================================                                               
FAIL: test_event_callback (tests.test_mpv.TestLifecycle)                                                             
----------------------------------------------------------------------                                               
Traceback (most recent call last):                                                                                   
  File "/<<PKGBUILDDIR>>/tests/test_mpv.py", line 583, in test_event_callback                                        
    handler.assert_not_called()                                                                                      
  File "/usr/lib/python3.9/unittest/mock.py", line 868, in assert_not_called                                         
    raise AssertionError(msg)                                                                                        
AssertionError: Expected 'mock' to not have been called. Called 1 times.                                             
Calls: [call({'event': 'idle'})].  
jaseg commented 2 years ago

Thanks for the report! I haven't seen those failures on my arch setup, but I'll get myself a debian VM to reproduce these.

jaseg commented 2 years ago

test_wait_for_prooperty_event_overflow can be disabled if it is flaky. This is a test of a safeguard against a queue overrun that is extremely hard to trigger in the first place, which is why I assume the test fails on some platforms. With the github actions CI system recently contributed by @trin94 we have this test run on windows regularly, which should be enough to make sure we don't get an outright regression.

jaseg commented 2 years ago

I was able to reproduce some of these. They stem from race conditions around the test teardown process. As far as I can tell, there is no clean way to explicitly synchronize this process (without resorting to some extreme measures such as using OS-level threading functions via ctypes).

I have added some 1s delays around the teardown of these test cases, and on my system I did not get any more failures during 85 runs of the test suite. @baldurmen please update this issue if you see any more flakiness.

baldurmen commented 2 years ago

I ran the testsuite with the latest commit on the main branch (85aaed0) 12 times (6 with py3.9, 6 with py3.10) in slightly different environments, all based on Debian, and I had no failures :)

I'd be glad if a release could be made soonish so I can incorporate the improvements.

Cheers!