-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Skip socket sendrecv test (WIN32) not for Windows #3661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip socket sendrecv test (WIN32) not for Windows #3661
Conversation
Hello, thanks for all the patches @mizunashi-mana keeping tests in working condition is great. Worth noting that this one has been previously changed to OS agnostic testing via 2f70038#diff-7e882b334d4823080a099828b9f9a16d It seems that this test can be run under Windows after all, as it also says in the comment... |
Additionally, this test fails on macOS because IPV6_RECVPKTINFO is not defined: php-src/ext/sockets/sendrecvmsg.c Line 36 in 49a4e69
|
Just thinking out loud, wouldn't it be better to filter it using it the PHP 7.2+ constant if (PHP_OS_FAMILY === 'Darwin') {
die('skip non macOS test');
} |
Worth noting that |
Sounds reasonable. If we do it, than we should also update the documentation. |
Thanks for share this link @cmb69, there's some stuff I'd like to put into there and I didn't even know about it. |
Hmm... This may not be the mistake of test, and be the bug of PHP... We should add fallback for macOS by defining See also: |
I reported on https://bugs.php.net/bug.php?id=77136 . May I close this PR? |
Closing this as #3663 has landed, which also fixes this issue, if I understood correctly. |
This seems to be for Unix test.
And, https://github.com/php/php-src/blob/8021e0f5dcc52af60d1b1b823c732e22a390d48a/ext/sockets/tests/socket_sendrecvmsg_multi_msg.phpt is for Windows, right?
So, we should skip the test for Windows on macOS or Linux.