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

Fix unix domain socket tests on Windows #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michal-josef-spacek
Copy link

There is no support in FCGI for this type of usage.

There is no support in FCGI for this type of usage.
@michal-josef-spacek
Copy link
Author

@michal-josef-spacek
Copy link
Author

Tested on Linux and Windows by me

Copy link

@PhilterPaper PhilterPaper left a comment

Choose a reason for hiding this comment

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

Looks OK from here, if this is intended to be a permanent fix (FCGI using Unix sockets on Windows will likely never work). On the other hand, if FCGI might be fixed to work, as one ticket response seemed to suggest, and this is only a temporary fix, I would just comment out the socket() test (leaving the plan skip_all).

@michal-josef-spacek
Copy link
Author

@PhilterPaper My intent with FCGI is to add tests for functional things. After it, I want to fix the library (not working say, ipv6, deprecated functions). Unix sockets on Windows could be new functionality.

@PhilterPaper
Copy link

OK, I made your change to the t-test and upgraded my FCGI to 0.82. At least now it will stop bugging me to update it. I take it that so long as Windows doesn't support this kind of socket, you'll skip that OS, and if you do get a fix, you'll re-enable it. Do you also skip sockets on Windows in the FCGI code (not just the t-tests), or is that just "user beware"? Happy Holidays!

@michal-josef-spacek
Copy link
Author

@PhilterPaper Yes, something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants