-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Tracking Issue for Testing utilities being Experimental. #1791
Comments
not stabilized yet, keep unscheduled. |
I realized that all members of I also see a similar problem on the client side. We need What would be the recommended way to run this sort of interop tests? |
@trustin I think you can define/fork your own function similar to The source code is at |
If certain methods are useful, have a reasonable API, and are maintainable, we can keep them. @trustin, details about your test:
I'm fine to work out utility functions for using test certificates. I understand that is a major PITA and using grpc's pre-generated test certs makes things much easier. But I expect they could be simplified. For example, Netty now has Functionality that looks good to keep (but maybe renamed/moved/tweaked):
|
- Brave 4.5.2 -> 4.6.0 - Checkstyle 8.1 -> 8.2 - gRPC 1.5.0 -> 1.6.1 - Remove some usage of deprecated methods, as adviced at: grpc/grpc-java#1791
@ejona86 Thanks a lot for the detailed suggestions and ideas. Using |
- Brave 4.5.2 -> 4.6.0 - Checkstyle 8.1 -> 8.2 - gRPC 1.5.0 -> 1.6.1 - Remove some usage of deprecated methods, as adviced at: grpc/grpc-java#1791
- Brave 4.5.2 -> 4.6.0 - Checkstyle 8.1 -> 8.2 - gRPC 1.5.0 -> 1.6.1 - Remove some usage of deprecated methods, as adviced at: grpc/grpc-java#1791
Deprecated |
Deprecated |
I see that
Wait, how? How do we use a blocking stub in bidirectional streaming calls? That's exactly what we've been using
We absolutely will copy the class. I don't understand the "you must know the class's implementation" part, though. The class is nicely JavaDoc'ed and we've been using it for quite some time without any knowledge about its implementation. What are the details that we need to know and how can we misuse the class? It seems to be dead simple. |
@JanecekPetr wrote:
You're right, blocking stubs don't solve client-streaming and bidi-streaming. However, those sorts of methods are rare, so that's why the "most" is there before "most users." And of the remaining, many would use mocks, since mocks are in vogue. The class is also not helpful in many cases of bidi calls since it doesn't let you wait for a response to arrive. It's actually useful to know you are using it, so pinging this was a "Good Thing." We looked through usages (internally) and only found one user, and that's because they used
Most recent thing that came up (#2385) was whether The class isn't really up to our normal standards; it was primarily exposed because it needed to be used in our tests (before blocking stubs, but also with streaming), and grpc-testing was a convenient place for that (in fact, that's what grpc-testing initially was; only utilities we needed for ourselves). So we're trying to clean things up and make it more clear. The class could be improved to be more maintainable: make it a There's been talk of providing real streaming APIs for blocking, with an API sketch I've seen that looks feasible. Although I realize it may be a pain to migrate your tests, if you were able to do full bidi streaming in a no-need-for-other-threads blocking fashion, would that be a welcome approach for your tests? |
@dapengzhang0, since @JanecekPetr noticed the planned removal in less than a workday of the release, it seems unlikely he's the only user. So it'd probably be fair to revert #3790 until we have a good streaming blocking API (or similar answer for the use-case gaps). But we should make a new issue for that and not lump it in here. |
Yes, thank you, that would be fine. Migrating tests is of course a pain, but one that can be done reasonably easily and fast. Thanks! |
I guess once #10318 is merged we can delete |
Specific usages;
StreamRecorder
TestUtils
The text was updated successfully, but these errors were encountered: