-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add :formatter to ExUnit.CaptureLog options
#14914
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
Add :formatter to ExUnit.CaptureLog options
#14914
Conversation
|
This is a minimal adaptation of the changes I made when publishing the proof-of-concept / backport package that I created for this, CaptureLogger. In CaptureLogger I'm testing with LoggerJSON, but I've created a much more simplistic "ReverseLogger" that stringifies and reverses the message being logged. In CaptureLogger, I am using a default formatter possibly pulled from I also think that I could duplicate one of the tests so that it shows the use of leftover options, as I'm showing two variations of |
4544fc3 to
bce6200
Compare
Mostly modified `:log_capture_on` handler for ExUnit.CaptureServer to respect a `:formatter` option instead `Logger.default_formatter/1`. Updates were made to documentation and tests to reflect this change.
bce6200 to
9ba53f8
Compare
Also simplify test
whatyouhide
left a comment
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.
Lovely
|
💚 💙 💜 💛 ❤️ |
| {formatter_mod, formatter_config} = Logger.default_formatter(opts) | ||
|
|
||
| {formatter_mod, formatter_config} = | ||
| Keyword.get_lazy(opts, :formatter, fn -> Logger.default_formatter(opts) end) |
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.
According to the spec, formatter can be {module(), term()} | nil, but wouldn't this fail in the nil case?
Should it be Keyword.get(opts, :formatter) || Logger.default_formatter(opts), or should we fix the spec?
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.
That's a good point. I can make either change.
Mostly modified
:log_capture_onhandler for ExUnit.CaptureServer to respect a:formatteroption insteadLogger.default_formatter/1.Updates were made to documentation and tests to reflect this change.