Skip to content

Add Python Client to Test Negative HTTP2 Conditions#9097

Merged
ncteisen merged 1 commit intogrpc:masterfrom
ncteisen:negative_http2_interop_tests
Dec 19, 2016
Merged

Add Python Client to Test Negative HTTP2 Conditions#9097
ncteisen merged 1 commit intogrpc:masterfrom
ncteisen:negative_http2_interop_tests

Conversation

@ncteisen
Copy link
Copy Markdown
Contributor

These tests are for the new Http/2 test server (#8900). They are designed to test client behavior when the server sends unexpected responses during rpcs.

cc @ericgribkoff @makdharma

@nathanielmanistaatgoogle
Copy link
Copy Markdown
Contributor

Cool! Please prepend a leading underscore on the file name. Does this test need to be recorded in tests.json?

@ncteisen ncteisen force-pushed the negative_http2_interop_tests branch from f172649 to d1ee2a3 Compare December 14, 2016 03:08
@ncteisen
Copy link
Copy Markdown
Contributor Author

Renamed the file. Although this file is more similar to interop/client.py than is is to the unit tests, I was wondering if maybe it should follow that convention instead?

And at this point, no, it does not belong in tests.json since it cannot run as a standalone test. Eventually, we will do the same thing to is as was done to the interop tests, by making them into unit tests.

@nathanielmanistaatgoogle
Copy link
Copy Markdown
Contributor

You're absolutely right - the file should not have a leading underscore and should not be added to tests.json; I apologize for the erroneous comment.

Please link in your commit log message to the document specifying these tests? I expect it would be a sibling of the interop test specifications?

@ncteisen
Copy link
Copy Markdown
Contributor Author

The documentation is in progress. Will link to it and update this thread when it is finished.


# common requests
_SMALL_SIZE = 1024
_LARGE_SIZE = 314159
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not used anywhere.

_SMALL_SIZE = 1024
_LARGE_SIZE = 314159

_SMALL_SIMPLE_REQUEST = messages_pb2.SimpleRequest(
Copy link
Copy Markdown
Contributor

@adelez adelez Dec 17, 2016

Choose a reason for hiding this comment

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


_SMALL_SIMPLE_REQUEST = messages_pb2.SimpleRequest(
response_type=messages_pb2.COMPRESSABLE,
response_size=_SMALL_SIZE,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to the same doc the response_size should be _LARGE_SIZE. Can you please review the sizes to make sure they are in sync with the doc. Thanks!

@ncteisen ncteisen force-pushed the negative_http2_interop_tests branch 2 times, most recently from fe77d28 to 02a7d42 Compare December 19, 2016 16:50
@ncteisen
Copy link
Copy Markdown
Contributor Author

Addressed all comments. Documentation is here.

@ncteisen ncteisen merged commit 4e00a99 into grpc:master Dec 19, 2016
@ncteisen ncteisen deleted the negative_http2_interop_tests branch December 19, 2016 19:33
@ncteisen
Copy link
Copy Markdown
Contributor Author

Almost all test are green. cloud_to_cloud:ruby:node_server:timeout_on_sleeping_server fails, but that is a well known flake (see #8989).

Going to merge

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants