Use Erlang logger handlers in ExUnit.CaptureLog #12408
Conversation
|
I'm experiencing a strange behavior related to this, logs from erlang libraries in my elixir project are not captured by ExUnit and show up in the console while running tests. Is that known and is this the problem this PR is solving? (@whatyouhide sorry for the noise but I just found this problem today and this PR "è arrivata a fagiuolo" 😄 ) |
| status = | ||
| with {:ok, config} <- :logger.get_handler_config(:default), | ||
| :ok <- :logger.remove_handler(:default) do | ||
| :ok <- :logger.remove_handler(:default), | ||
| :ok <- :logger.add_handler(@name, __MODULE__, handler_cfg) do |
There was a problem hiding this comment.
We should attach this handler even if there is no default. So I recommend moving it out of the with and keeping these two code paths separated.
There was a problem hiding this comment.
We should also remember to remove this handler on maybe_add_console.
There was a problem hiding this comment.
maybe_add_console should probably be renamed to revert_handlers or something :D
| @spec with_log(keyword, (-> any)) :: {any, String.t()} | ||
| def with_log(opts \\ [], fun) do | ||
| @spec with_log(keyword, (-> result)) :: {result, String.t()} when result: any | ||
| def with_log(opts \\ [], fun) when is_list(opts) do | ||
| opts = Keyword.put_new(opts, :level, nil) | ||
| {:ok, string_io} = StringIO.open("") | ||
|
|
||
| try do | ||
| :ok = add_capture(string_io, opts) |
There was a problem hiding this comment.
We should remove this capture. We are likely capturing everything twice right now. :) plus, we need to pass the level to the ets table and only write if it matches the ETS table level.
There was a problem hiding this comment.
Note we can default the level to :all.
|
@josevalim we had the same ideas, yep. I just implemented all of that, ready for another review! |
| @@ -59,11 +66,17 @@ defmodule ExUnit.CaptureServer do | |||
| {:reply, :ok, release_device(ref, config)} | |||
| end | |||
|
|
|||
| def handle_call({:log_capture_on, pid}, _from, config) do | |||
| def handle_call({:log_capture_on, pid, string_io, opts}, _from, config) do | |||
| ref = Process.monitor(pid) | |||
| refs = Map.put(config.log_captures, ref, true) | |||
There was a problem hiding this comment.
I think we'll be able to get rid of this and only use the ETS table to store these, but I'd rather do one thing at a time and do that in another PR. 🙃
Co-authored-by: José Valim <[email protected]>
| {level, opts} = Keyword.pop(opts, :level) | ||
| true = :ets.insert(@ets, {ref, string_io, level || :all}) |
There was a problem hiding this comment.
| {level, opts} = Keyword.pop(opts, :level) | |
| true = :ets.insert(@ets, {ref, string_io, level || :all}) | |
| {level, opts} = Keyword.pop(opts, :level, :all) | |
| true = :ets.insert(@ets, {ref, string_io, level}) |
Co-authored-by: José Valim <[email protected]>
Part of #9465.