Skip to content
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

test(pdo_firebird): Fix the dummy server running on localhost to IPv4 #16115

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

KentarouTakeda
Copy link
Contributor

@KentarouTakeda KentarouTakeda commented Sep 29, 2024

This is related to #16113.

In the pdo_firebird test, a dummy server was created on localhost using stream_socket_server() to test for error responses that do not normally occur.

stream_socket_get_name() was used to generate the name of the dummy server, but this function seems to return the IPv6 address ::1 even in host environments where IPv6 is not available. As a result, in certain environments it was not possible to connect to the dummy server, and the test failed.

This problem is solved by creating a server on 127.0.0.1 and fixing it to IPv4, rather than creating it on localhost and leaving the choice of IPv6 or IPv4 to stream_socket_get_name().

@KentarouTakeda
Copy link
Contributor Author

Note: The issue fixed by this pull request potentially existed before #16113.

When I created #12677 in the past, I encountered the exact same test failure in the docker environment I personally use. I left it as it was because it didn't reproduce in CI, but after seeing #16113, I realized the cause.

I'm submitting screenshots of before and after the fix for your reference.

  • Before fix - fail
    before-fix

  • After fix - pass
    after-fix

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! This looks good to me but I'll wait for Saki to confirm.
I filed an issue for stream_socket_get_name(): #16117

Copy link
Member

@SakiTakamachi SakiTakamachi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@nielsdos nielsdos merged commit fec2055 into php:master Sep 29, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants