-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fixing watermark preconditions #1509
Conversation
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.
@alyssawilk per off-line convo, do you mind opening an issue on what kinds of tests we would need to add so that we could have caught this with our own integration tests?
// When the server stream is created it should check the status of the | ||
// underlying connection. Pretend it is overrun. | ||
EXPECT_CALL(server_connection_, aboveHighWatermark()).WillOnce(Return(true)); | ||
EXPECT_CALL(server_stream_callbacks2, onAboveWriteBufferHighWatermark()).Times(1); |
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.
Nit: Times(1)
is redundant.
#1518
Basically a brain dump of what I miss from GFE tests (excluding things
already in issue 1162)
…On Tue, Aug 22, 2017 at 12:05 PM, Matt Klein ***@***.***> wrote:
***@***.**** approved this pull request.
@alyssawilk <https://github.com/alyssawilk> per off-line convo, do you
mind opening an issue on what kinds of tests we would need to add so that
we could have caught this with our own integration tests?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1509 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ARYFvcCivReKIZz1rwm2a5uBiTbnpgsCks5savxggaJpZM4O-zav>
.
|
* envoy: update and modify new include paths Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
* envoy: update and modify new include paths Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
This fixes a bug in my implementation of #150 where we were passing connection-level flow control blockage to new client side streams but not new server side streams.
@htuch @PiotrSikora this should unblock our internal merge.