gh-128881: Do not ignore address and flags parameters in socket.{send,recv}_fds
#128882
Conversation
|
I have no idea what's happening on macOS. Should we just skip it on macOS? |
|
MSG_PEEK may got different action on macOS/FreeBSD, let's figure it out. BTW, we can skip the macOS test first |
There was a problem hiding this comment.
Can we also have a test that combine both address and flags as well for send_fds please? and see if we can have test coverage for Windows (just using a 0 flag for instance would be sufficient).
According to https://docs.python.org/3/library/socket.html#socket.send_fds, send_fds and recv_fds are is available on Windows and Unix, so we should be able to have tests for them as well.
Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst
Outdated
Show resolved
Hide resolved
| data = os.read(rfd, 100) | ||
| self.assertEqual(data, str(index).encode()) | ||
| @requireAttrs(socket, "MSG_PEEK") | ||
| @unittest.skipUnless(sys.platform in ("linux", "android"), "works on Linux") |
There was a problem hiding this comment.
Is there a way to have a flag test on macOS or not at all?
There was a problem hiding this comment.
I thought about it, but I'm not sure how to implement it. Is it good practice to write a _test_recv_fds_peek(self, skip_fd_check=False) method and 2 wrappers with different arguments?
There was a problem hiding this comment.
A good practice is to check if there's a flag that is supported both on Unix and Windows. The reason is that the availability of recv_fds is marked as Unix and Windows but not WASI. So we shouldn't have this kind of skip in the first place.
Now, if there is not a common flag that is supported on all platforms, just writing three different tests for three different flags is also fine (each of which protected by a skipUnless)
There was a problem hiding this comment.
The reason is that the availability of recv_fds is marked as Unix and Windows but not WASI. So we shouldn't have this kind of skip in the first place.
recv_fds requires recvmsg, which is only available on Unix but not WASI, so I think it may be a documentation error.
There was a problem hiding this comment.
Ok actually we have #122153 which wants to implement it but we never implemented it. Good to know that it's a doc issue. We can open a PR (using 122153 as the GH issue number) to amend the docs. For now, can you add some comment saying that it's not available on Windows yet but can be (and add some "see gh-122153)? thanks
There was a problem hiding this comment.
For now, can you add some comment saying that it's not available on Windows yet but can be (and add some "see gh-122153)? thanks
Of course!
However sendmsg/recvmsg aren't the only requirements, CPython doesn't support AF_UNIX on Windows, and Windows doesn't support SCM_RIGHTS at all, so I think it's not possible to implement send_fds/recv_fds on Windows.
ref: #77589 (comment)
There was a problem hiding this comment.
This doc issue was introduced in e3b6ff1
| self._test_pipe(fds[0], wfd, MSG) | ||
|
|
||
| @requireAttrs(socket, "MSG_DONTWAIT") | ||
| @unittest.skipUnless(sys.platform in ("linux", "android"), "Linux specific test") |
There was a problem hiding this comment.
CI shows that sendmsg raises OSError: [Errno 55] No buffer space available on macOS. I'm not familiar with macOS, so I don't know if it's the expected behavior.
picnixz
changed the title
address and flags parameters in socket.{send,recv}_fds
Co-authored-by: Bénédikt Tran <[email protected]>
fix 8d120f7
socket.send_fdsandsocket.recv_fds#128881