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

Parameter injection of Resources does not clean up #12

Closed
saadlu opened this issue Sep 5, 2022 · 5 comments · Fixed by #13
Closed

Parameter injection of Resources does not clean up #12

saadlu opened this issue Sep 5, 2022 · 5 comments · Fixed by #13
Assignees
Labels
bug Something isn't working
Milestone

Comments

@saadlu
Copy link

saadlu commented Sep 5, 2022

Hi Again,

I am trying out parameter injection of Resources.

See https://github.com/saadlu/helloworld-grpc-test/blob/main/src/test/java/net/saad/learning/grpc/hello/HelloWorldServerJ5ParameterInjectionTest.java

I put a breakpoint at Resources::clean (https://github.com/asarkar/grpc-test/blob/master/src/main/kotlin/com/asarkar/grpc/test/Resources.kt#L65), and it does not hit when I run the above test in debug mode.

For instance field of Resources the breakpoint does hit. So test https://github.com/saadlu/helloworld-grpc-test/blob/main/src/test/java/net/saad/learning/grpc/hello/HelloWorldServerJ5Test.java does clean up.

I have also trying putting breakpoints in io.grpc.internal.ServerImpl#shutdown and it does not hit when parameter injection is used but does hit when instance field of Resources is used.

I am not sure if I am doing something wrong.

But I did ran your test

https://github.com/asarkar/grpc-test/blob/master/src/test/kotlin/com/asarkar/grpc/test/GrpcCleanupExtensionIntegrationTests.kt#L22

and it fails for me, too.

Wanted but not invoked:
server.shutdown();
-> at com.asarkar.grpc.test.GrpcCleanupExtensionIntegrationTests.testSuccessful(GrpcCleanupExtensionIntegrationTests.kt:45)
Actually, there were zero interactions with this mock.

Wanted but not invoked:
server.shutdown();
-> at com.asarkar.grpc.test.GrpcCleanupExtensionIntegrationTests.testSuccessful(GrpcCleanupExtensionIntegrationTests.kt:45)
Actually, there were zero interactions with this mock.

any advice?

@asarkar
Copy link
Owner

asarkar commented Sep 5, 2022

Parameter injection works as expected. I’m unable to provide debugging service for individual users for free.

@asarkar asarkar closed this as completed Sep 5, 2022
@asarkar asarkar added invalid This doesn't seem right question Further information is requested labels Sep 5, 2022
@saadlu
Copy link
Author

saadlu commented Sep 6, 2022

thanks

if it works and your tests are passing, then it's on me.

thanks for contributing this.

I sincerely apologies. I did not mean to ask for debugging service.

@asarkar
Copy link
Owner

asarkar commented Sep 7, 2022

Thank you for sponsoring me. As a perk for paid sponsorship, you will receive personalized support. I’m reopening this ticket and will look into it shortly.

@asarkar asarkar reopened this Sep 7, 2022
@asarkar asarkar removed the invalid This doesn't seem right label Sep 7, 2022
@asarkar asarkar changed the title Question: parameter injection of Resources does not seem work Parameter injection of Resources does not clean up Sep 12, 2022
@asarkar asarkar added bug Something isn't working and removed question Further information is requested labels Sep 12, 2022
@asarkar asarkar self-assigned this Sep 12, 2022
@asarkar asarkar added this to the v1.2.2 milestone Sep 12, 2022
@asarkar
Copy link
Owner

asarkar commented Sep 12, 2022

@saadlu I've determined that this is a bug, and created a PR with a fix that'll be released shortly. Unfortunately, I couldn't find a way to write unit or integration tests to verify the fix, but I've tested it manually with your project, and verified that Resources.cleanUp method is invoked when your test is ran.
Thanks for the report.

@asarkar
Copy link
Owner

asarkar commented Sep 12, 2022

https://github.com/asarkar/grpc-test/releases/tag/v1.2.2. It might take a few hours to show up in Maven Central.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants