Fix test failures caused by mockk not cleaning up #5207
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samgst-amazon
approved these changes
Dec 14, 2024
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at [email protected]
|
Contributor
Author
|
resolved ~15, but still failing TelemetryHelperTest and some others |
Contributor
Author
|
MockTelemetryServiceExtension does not work as expected, needs follow-up |
karanA-aws
pushed a commit
to karanA-aws/aws-toolkit-jetbrains
that referenced
this pull request
Jan 17, 2025
Will-ShaoHua
added a commit
that referenced
this pull request
Jan 21, 2025
…owing NotAMockException (#5258) ## Description <!--- Describe your changes in detail --> <!--- If appropriate, providing screenshots will help us review your contribution --> <!--- If there is a related issue, please provide a link here --> Now there are 2 ways to use NoOpTelemetryService 1. if devs are not interested in validating the metric contents, don't need do anything as default test implementation is NoOpTelemetryService 2. if devs are interested in the metric contents, can use `MockTelemetryServiceExtension` which will inject a spy of batcher and replace the original test implementation in junit before hook and revert it in after hook so that you can use ArgumentCaptor to capture what's going through the batcher Relevant #5207 ### root cause: Originally by doing `class NoOpTelemetryService : TelemetryService(publisher, spy(DefaultTelemetryBatcher(publisher))) {`, it's not 100% guaranteed that the TelemetryBatcher passed in is a mockito spy, and it result in `NotAMockException` thrown from these cases. ### solution Manually inject spy batcher separately in a different step instead of IDE doing the work when initializing app/project service components--------- Co-authored-by: Richard Li <[email protected]>
karanA-aws
pushed a commit
to karanA-aws/aws-toolkit-jetbrains
that referenced
this pull request
Jan 22, 2025
karanA-aws
pushed a commit
to karanA-aws/aws-toolkit-jetbrains
that referenced
this pull request
Jan 22, 2025
…owing NotAMockException (aws#5258) ## Description <!--- Describe your changes in detail --> <!--- If appropriate, providing screenshots will help us review your contribution --> <!--- If there is a related issue, please provide a link here --> Now there are 2 ways to use NoOpTelemetryService 1. if devs are not interested in validating the metric contents, don't need do anything as default test implementation is NoOpTelemetryService 2. if devs are interested in the metric contents, can use `MockTelemetryServiceExtension` which will inject a spy of batcher and replace the original test implementation in junit before hook and revert it in after hook so that you can use ArgumentCaptor to capture what's going through the batcher Relevant aws#5207 ### root cause: Originally by doing `class NoOpTelemetryService : TelemetryService(publisher, spy(DefaultTelemetryBatcher(publisher))) {`, it's not 100% guaranteed that the TelemetryBatcher passed in is a mockito spy, and it result in `NotAMockException` thrown from these cases. ### solution Manually inject spy batcher separately in a different step instead of IDE doing the work when initializing app/project service components--------- Co-authored-by: Richard Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
~30 tests fail on initial run but succeed on automatic retry due to improper mockk cleanup
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.