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 Java Unit Test docs example in Unix Domain Sockets #2119

Merged
merged 3 commits into from Feb 3, 2020

Conversation

seglo
Copy link
Member

@seglo seglo commented Jan 31, 2020

The lone Java test in unix-domain-socket used for docs has always failed, but we never noticed it because we didn't block on stream completion before the test completed. The blocking was added #2040 (https://github.com/akka/alpakka/pull/2040/files#r368017011), but the test failure got confused with a transient test failure and the PR was merged.

Changes

  • Use a temp directory instead of a file. jnr-unixsocket will create the socket files on our behalf, if the file already exists then we'll observe an Address in use exception (https://travis-ci.com/akka/alpakka/jobs/277967218#L358)
  • Actually send data through socket!
  • Remove framing boilerplate and just parse as a ByteString like the Scala docs example does.

@seglo seglo added this to Incoming Issues and PRs in Akka streaming via automation Jan 31, 2020
@seglo seglo moved this from Incoming Issues and PRs to Ready for review in Akka streaming Jan 31, 2020
@raboof
Copy link
Member

raboof commented Feb 3, 2020

build failure should be fixed by #2123

Akka streaming automation moved this from Ready for review to Reviewer approved Feb 3, 2020
Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

LGTM

.map(s -> s + "!!!\n")
.map(ByteString::fromString);
// server logic ...
// #outgoingConnection
Copy link
Member

Choose a reason for hiding this comment

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

Good to keep the actual implementation out of the docs

// #outgoingConnection
ByteString sendBytes = ByteString.fromString("Hello");
Copy link
Member

Choose a reason for hiding this comment

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

(this line could be moved down to below the part that makes it into the docs)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@seglo seglo force-pushed the seglo/java-unix-domain-socket-test branch from 1f92892 to e1547ff Compare February 3, 2020 17:16
@seglo seglo merged commit 8b6d63c into akka:master Feb 3, 2020
Akka streaming automation moved this from Reviewer approved to Done Feb 3, 2020
@seglo seglo deleted the seglo/java-unix-domain-socket-test branch February 3, 2020 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants