-
Notifications
You must be signed in to change notification settings - Fork 92
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 file socket transport test on Windows #1755
Fix file socket transport test on Windows #1755
Conversation
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
UnixDomainSocketAddress addr = UnixDomainSocketAddress.of(socketName); | ||
|
||
try { | ||
SocketChannel channel = SocketChannel.open(StandardProtocolFamily.UNIX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's possible to wrap this in a try-with-resources to ensure proper closing of the channel after a successful test?
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks good. I still found two nits... 😄
try (SocketChannel channel = SocketChannel.open(StandardProtocolFamily.UNIX)) { | ||
// Just see if we can create an unix domain socket on Windows. | ||
} catch (UnsupportedOperationException e) { | ||
// Windows version is too old to support unix domain sockets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a little System.out.println
trace would be nice here that one can read in the output what happened.
@@ -93,6 +93,15 @@ public static void main(String[] args) throws Throwable { | |||
System.exit(1); | |||
} | |||
|
|||
if (Platform.isWindows()) { | |||
try (SocketChannel channel = SocketChannel.open(StandardProtocolFamily.UNIX)) { | |||
// Just see if we can create an unix domain socket on Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the correct article here is "a". " ... create a unix domain socket ..."
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
Which Windows version do you thin support it? Is there some minimum version where we expect it to work? |
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
This will not run the FileSocketTransportTest on Windows versions which don't support Unix Domain Sockets.
fixes #1690