-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
The counter indicator increases by 2 each time (on happy path) #1548
The counter indicator increases by 2 each time (on happy path) #1548
Conversation
import static org.mockito.Mockito.*; | ||
|
||
@RunWith(MockitoJUnitRunner.class) | ||
public class MeteredClientTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the existing tests are a bit painfull to read, but, could we move this to https://github.com/OpenFeign/feign/blob/master/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java ?
That would also test any problems on dropwizard metric implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sure. I will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved test to old abstract class. It works for mictometer: old client breaks the test and new one does not.
Dropwizard clients have another meter implementation and I cannot find how to repeat this case (two calls of meter/counter) there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the bug was limited to micrometer implementation, that is great
* The counter indicator increases by 2 each time (on happy path). * Add test coverage * Make test beaty again * Fix license header * Move new tests to the common abstract class
* The counter indicator increases by 2 each time (on happy path). * Add test coverage * Make test beaty again * Fix license header * Move new tests to the common abstract class
Fixes #1480
We call
sample.stop(timer)
in thefinally
block and therefore the call in thetry-catch
block is unnecessary.