Check the server for Xrandr support version #417
Conversation
We currently only check the client libXrandr to see if it supports the XRRGetScreenResourcesCurrent function. To make sure the function will work, we also need to check the server's Xrandr version.
|
Thanks @jholveck! About the failing CI, only Python 3.14 on Windows is known to never pass yet: others are passing normally. |
|
Overall seems good to me, would you mind fixing tests + adding line in the changelog? After that, I will merge :) |
|
The CI bot seems to indicate that the Linux tests are failing too. Is that not what you're seeing? I'll take another look at this change and update the PR. |
|
Yup, I see failling Linux tests. I assumed you'll see them too and fix them :) |
|
Ah, I misunderstood your earlier comment. Yes, of course I'll fix these. How do you usually run tests from the command line before your commit? With |
|
This is specific to the CI where a X server is not necessarily available. On your machine, |
|
Ok! After looking at this, I think that the test itself is faulty. As written, it will use I'll come up with a better way to structure this test. |
|
I've verified that the test itself is at issue, but the specific path through which it was previously passing is amusing. I've got a patch with the details, and a much more stringent set of tests, but it's late in California and I want to review it with fresh eyes before I commit. |
|
Next round tomorrow :) |
Previously, the test would try to monkey-patch the libXrandr module by deleting the XRRGetScreenResourcesCurrent function. However, this actually doesn't work as expected. It deletes all the configuration you've assigned to that C function (argument and return types). But ctypes.CDLL resolves symbols lazily in __getattr__ via dlsym. Deleting the attribute only discards the previously created _FuncPtr object; the next getattr/hasattr on the CDLL will just call dlsym again and recreate it. The existing test's code path seemed to be working, because the implementation was just testing for AttributeError. The assumption had been that if XRRGetScreenResourcesCurrent was deleted, then trying to access it would raise an AttributeError. However, what was actually being raised was when the .contents attribute of the return value was being accessed: since the restype had been erased, then the return value was now an int, not a pointer. XRRGetScreenResourcesCurrent was still getting called, but trying to read its .contents raised the AttributeError. The new test now uses a proxy object to genuinely hide XRRGetScreenResourcesCurrent. Additionally, the test was split into three. The tests now will ensure that the right functions are being called based on whether XRRGetScreenResourcesCurrent is supported, missing on the client library, or missing on the server.
|
I've updated the test. You might find the description in the commit log interesting. |
|
Great tests, and thank you for the explanation. It's a lesson about properly testing things 😅 |
2 participants
We currently only check the client libXrandr to see if it supports the XRRGetScreenResourcesCurrent function. To make sure the function will work, we also need to check the server's Xrandr version.
That said, Xrandr 1.3 was released in 2009, and the code that checks for its existence (849e1fe) was added in 2019. Today, the version check might no longer be necessary.