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

Fix grpc tests when running from cmd-line/eachdist script #1027

Merged
merged 4 commits into from
Aug 21, 2020

Conversation

mariojonke
Copy link
Contributor

Description

When running the grpc tests from the command line with pytest instrumentation/opentelemetry-instrumentation-grpc/tests or python scripts/eachdist.py test the 2nd test in the TestClientProto test case was failing with:

instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py:200: in _verify_error_records
    ("status_code", grpc.StatusCode.INVALID_ARGUMENT),
E   AssertionError: Tuples differ: (('me[65 chars]code', <StatusCode.UNAVAILABLE: (14, 'unavailable')>)) != (('me[65 chars]code', <StatusCode.INVALID_ARGUMENT: (3, 'invalid argument')>))
E   
E   First differing element 1:
E   ('status_code', <StatusCode.UNAVAILABLE: (14, 'unavailable')>)
E   ('status_code', <StatusCode.INVALID_ARGUMENT: (3, 'invalid argument')>)
E   
E     (('method', '/GRPCTestServer/BidirectionalStreamingMethod'),
E   -  ('status_code', <StatusCode.UNAVAILABLE: (14, 'unavailable')>))
E   ?                              ^ -   ^ ^^    ^^   ^ -   ^ ^^
E   
E   +  ('status_code', <StatusCode.INVALID_ARGUMENT: (3, 'invalid argument')>))
E   ?                              ^   + ^^ ^^^^ ++ 

Looks like the problem is a leftover connection from the previous test due to the channel not being properly closed on test teardown. The current test then fails to connect to the test server which results in StatusCode.UNAVAILABLE.

For tox tests this was already addressed in #974 by pinning the gprc version to 1.30

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Run pytest instrumentation/opentelemetry-instrumentation-grpc/tests without and with closing the channel.

* when running the grpc tests with pytest or eachdist from the command
  line the 2nd test trying to connect to the test server failed with
  a connection refused message.
  Seems like the connection from the previous test was still alive due to
  the channel not being properly closed.
* restore grpcio requirements to the oriignal version
@mariojonke mariojonke requested a review from a team August 20, 2020 12:10
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Change looks good, just a non-blocking question

@@ -42,7 +42,7 @@ packages=find_namespace:
install_requires =
opentelemetry-api == 0.13dev0
opentelemetry-sdk == 0.13dev0
grpcio == 1.30
grpcio ~= 1.27
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checking, this isn't required for the test to pass correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it is not required. The tests work also without this change.
Since #974 / #975 fixed the tox tests this is just restoring the requirements to the previous state since it now also works with the lower version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good 👍

@mariojonke mariojonke mentioned this pull request Aug 21, 2020
@codeboten codeboten merged commit dfc7aa5 into open-telemetry:master Aug 21, 2020
@mariojonke mariojonke deleted the fix-grpc-tests branch April 13, 2021 07:56
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