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

Bazel rules for gRPC Python interop tests #16813

Merged

Conversation

ghostwriternr
Copy link
Contributor

@ghostwriternr ghostwriternr commented Oct 9, 2018

Add interop tests for gRPC Python. py_proto_library rules are added to src/proto/grpc/testing/BUILD since grpc_proto_library is not compatible with py_* rules.

'requests' python module is added to requirements.bazel.txt as it is a dependency for google-auth. Previously, this was installed through tools/run_tests/helper_scripts/build_python.sh before running tests.

@grpc-testing
Copy link

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,329      Total (=)      1,965,329

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,888,226      Total (<)     10,888,229

 No significant differences in binary sizes


@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

@ghostwriternr ghostwriternr force-pushed the bazel-python-interop-tests branch 2 times, most recently from acfa7c5 to a7c4b57 Compare October 9, 2018 09:10
@grpc-testing
Copy link

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,329      Total (=)      1,965,329

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,888,232      Total (=)     10,888,232

 No significant differences in binary sizes


Copy link
Member

@nathanielmanistaatgoogle nathanielmanistaatgoogle left a comment

Choose a reason for hiding this comment

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

Looks great to me - @mehrdada or @srini100, do you want to have a look before this gets integrated?

@grpc-testing
Copy link

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,918      Total (=)      1,965,918

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,891,934      Total (<)     10,891,938

 No significant differences in binary sizes


@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,918      Total (=)      1,965,918

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,891,940      Total (<)     10,891,943

 No significant differences in binary sizes


@srini100
Copy link
Contributor

Restarted the tests as interop test was broken yesterday due to some other reason.

@grpc-testing
Copy link

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,918      Total (=)      1,965,918

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,891,941      Total (>)     10,891,937

 No significant differences in binary sizes


@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

]
)


Copy link
Member

Choose a reason for hiding this comment

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

Are these newlines produced by a formatting tool? If not, let's get rid of extra newlines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no. Just my editor! I believe it is a standard convention to have one newline at the end of a Python file (which Skylark essentially is a stripped down version of). I've updated my commit to reflect that now.

Choose a reason for hiding this comment

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

Something to be done at a later time in a later pull request: Bazel has a tool called "buildifier", and whatever buildifier says is the way a BUILD file should be formatted is the way it should be formatted. 🙂

@nicolasnoble: is now or the near future about the right time to add buildifier to the sanity tests?

Add interop tests for gRPC Python. py_proto_library rules are added to
src/proto/grpc/testing/BUILD since grpc_proto_library is not compatible
with py_* rules.

'requests' python module is added to requirements.bazel.txt as it is a
dependency for google-auth. Previously, this was installed through
tools/run_tests/helper_scripts/build_python.sh before running tests.
@grpc-testing
Copy link

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link

[trickle] No significant performance differences

@grpc-testing
Copy link

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,918      Total (=)      1,965,918

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,891,796      Total (<)     10,891,800

 No significant differences in binary sizes


@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 8ae3285 into grpc:master Oct 12, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
infra/Bazel lang/Python priority/P2 release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants