Skip to content

Conversation

zuercher
Copy link
Member

@zuercher zuercher commented Sep 5, 2018

OS X's localhost interface often takes longer to fail to connect
than Linux. Modifies RawConnectionDriver to detect when its
underlying connection is connected or failed and pauses the
echo_integration_test until that state is reached before
checking the connection state.

Risk Level: low, test-only changes
Testing: existing tests
Doc Changes: n/a
Release Notes: n/a

Signed-off-by: Stephan Zuercher stephan@turbinelabs.io

OS X's localhost interface often takes longer to fail to connect
then Linux. Modifies RawConnectionDriver to detect when its
underlying connection is connected or failed and pauses the
echo_integration_test until that state is reached before
checking the connection state.

*Risk Level*: low, test-only changes
*Testing*: existing tests
*Doc Changes*: n/a
*Release Notes*: n/a

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
@zuercher zuercher requested a review from alyssawilk September 5, 2018 19:27
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

On some level I'd rather we expose this in the state() of the connection, but that'd be a super high risk change and probably be worth it for deflaking :-P

one nit and you're good to go

Api::ApiPtr api_;
Event::DispatcherPtr dispatcher_;
std::shared_ptr<ConnectionCallbacks> callbacks_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be a shared pointer, or can we get away with unique?

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
@dnoe dnoe merged commit 1537e68 into envoyproxy:master Sep 6, 2018
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.

3 participants