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
Make MockLogAppender itself Releasable #108526
Conversation
Existing uses of MockLogAppender first construct an appender, then call capturing on the instance in a try-with-resources block. This commit adds a new method, capture, which creates an appender and sets up the capture the the same time. The intent is that this will replace the existing capturing calls, but there are too many to change in one PR.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
63d8832
to
23a6776
Compare
/** | ||
* Adds the list of class loggers to this {@link MockLogAppender}. | ||
* | ||
* Stops and runs some checks on the {@link MockLogAppender} once the returned object is released. | ||
*/ | ||
public Releasable capturing(Class<?>... classes) { |
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'm confused: you stated in the description that you wanted to replace capturing
gradually (hence the non-provate MockLogAppender
ctor), but here you are removing them. What am I missing?
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.
They still exist, just above this. The diff shows weird because the javadoc is moved to the new method.
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.
Ah! Got it. Yes, the diff was confusing.
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.
Looks good, and ++ to making the changes and removal of capturing
in follow-ups
Existing uses of MockLogAppender first construct an appender, then call capturing on the instance in a try-with-resources block. This commit adds a new method, capture, which creates an appender and sets up the capture the the same time. The intent is that this will replace the existing capturing calls, but there are too many to change in one PR.