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

gRPC treating clients without configuration as self-calling #17338

Merged

Conversation

michalszynkiewicz
Copy link
Member

@michalszynkiewicz michalszynkiewicz commented May 19, 2021

At the moment, gRPC testing experience is worse than REST. A user has to know the port of the gRPC server in tests, and has to configure a client stub for tests in application.properties.

This PR proposes allowing injection of client stubs without config when running in test mode. Such clients are automatically configured to call the exposed gRPC server.

The drawback of this change is that misconfiguration problems may be a bit more difficult to spot in tests.

@cescoffier @mkouba WDYT?

@michalszynkiewicz michalszynkiewicz requested review from mkouba and cescoffier and removed request for mkouba May 19, 2021 06:08
@quarkus-bot quarkus-bot bot added the area/grpc gRPC label May 19, 2021
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

This feature is intended for internal tests (aka QuarkusUnitTest) anyway, or?

@@ -30,7 +30,7 @@
*/
String value() default ELEMENT_NAME;

public final class Literal extends AnnotationLiteral<GrpcClient> implements GrpcClient {
final class Literal extends AnnotationLiteral<GrpcClient> implements GrpcClient {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

everything is public in an interface anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't know it works like this for annotations as well.

@michalszynkiewicz
Copy link
Member Author

It should work with @QuarkusTest too, no?

Native may not work though, I'm not sure if we enable test profile when running a native executable.

@michalszynkiewicz michalszynkiewicz merged commit e982294 into quarkusio:main May 19, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants