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

Relax the handshake count check in handshakeloss test #415

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nhorman
Copy link

@nhorman nhorman commented Jan 3, 2025

In doing interop testing, its been observed that, occasionally, if the test drops a sufficient number of handshake frames, a client may transiently fail for any number of reasons (running up against the max idle timeout for instance). In this event it may be handled by the application by simply retrying the request. This however causes the interop test to fail, as the test checks for the presence of exactly 50 handshakes, and a retry triggers handshakes above that count. It would seem prudent to allow the test to only check for a minimum number of handshakes, but allow for the possibility of more for clients that behave in this manner.

Alter the handshakeloss test to only fail if there are less than 50 handshakes present.

In doing interop testing, its been observed that, occasionally, if the
test drops a sufficient number of handshake frames, a client may
transiently fail for any number of reasons (running up against the max
idle timeout for instance).  In this event it may be handled by the
application by simply retrying the request.  This however causes the
interop test to fail, as the test checks for the presence of exactly 50
handshakes, and a retry triggers handshakes above that count.  It would
seem prudent to allow the test to only check for a minimum number of
handshakes, but allow for the possibility of more for clients that
behave in this manner.

Alter the handshakeloss test to only fail if there are less than 50
handshakes present.
@nhorman
Copy link
Author

nhorman commented Jan 3, 2025

@andrewkdinh for reference

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.

1 participant