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

Add blocking socket based MockTcpTransport #19332

Merged
merged 5 commits into from Jul 11, 2016

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jul 8, 2016

Today we have a bunch of tests that use netty transport for several reasons
these tests use it because they need to run some tcp based transport. Yet, this
couples our tests tightly to the netty implementation which should be tested on it's own.
This change adds a plain socket based blocking TcpTransport implementation that is used by
default in tests if local transport is suppressed or if network is selected.
It also adds another tcp network implementation as a showcase how the interface works.

@s1monw s1monw added >test Issues or PRs that are addressing/adding tests review :Distributed/Network Http and internode communication implementations labels Jul 8, 2016
if (e.getMessage().contains("Connection reset by peer")) {
return true;
}
if (e.getMessage().contains("Connection reset")) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you just remove the "by peer" from the first, since matching this new case is a superset of matching the first case?

@rjernst
Copy link
Member

rjernst commented Jul 8, 2016

This looks great! I left a few small comments, LGTM as is though except for the question about commented out code.

@s1monw
Copy link
Contributor Author

s1monw commented Jul 8, 2016

@rjernst I pushed a new commit

@rjernst
Copy link
Member

rjernst commented Jul 8, 2016

LGTM

@s1monw s1monw force-pushed the add_mock_network_transport branch from 08e35b5 to 50d1a4b Compare July 11, 2016 08:51
Today we have a bunch of tests that use netty transport for several reasons
these tests use it because they need to run some tcp based trasnport. Yet, this
couples our tests tightly to the netty implementation which should be tested on it's own.
This change adds a plain socket based blocking TcpTransport implementation that is used by
default in tests if local transport is suppressed or if network is selected.
It also adds another tcp network implementation as a showcase how the interface works.
@s1monw s1monw force-pushed the add_mock_network_transport branch from 50d1a4b to 8f7c41e Compare July 11, 2016 09:38
@s1monw s1monw merged commit 3f3c93e into elastic:master Jul 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Network Http and internode communication implementations >test Issues or PRs that are addressing/adding tests v5.0.0-alpha5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants