I'm not sure if there is a reason for not having done this in the past but 
I couldn't find anything in the mailing list about it already.

There is essentially a behaviour that's defined  in the documentation for 
ExUnit.Formatter, but it's not actually enforced by defining a @behaviour 
for those formatters because it's just a duplicate of the GenServer 
behaviour. Because this behaviour is rather specific, I think it would be 
easier for users to implement their own formatters if there was a properly 
defined ExUnit.Formatter behaviour. If there was a way to `use 
ExUnit.Formatter` as a base implementation that would be even better, but I 
think a good first step is moving to a behaviour here.

My idea for this behaviour is as follows:

```
defmodule ExUnit.Formatter do
  @callback init(config :: list) ::
              {:ok, state}
              | {:ok, state, timeout | :hibernate | {:continue, term}}
              | :ignore
              | {:stop, reason :: any}
            when state: any

  @callback suite_started(opts :: list) :: :ok
  @callback suite_finished(run_us :: non_neg_integer, load_us :: 
non_neg_integer) :: :ok
  # similar callbacks for all the other events
end
```

By keeping `init/1` from the GenServer behaviour we make it so we can still 
easily start these formatters under the DynamicSupervisor in 
ExUnit.EventManager.add_handler/2, but by moving away from calling 
`GenServer.cast/2` directly and using the behaviour interface we can more 
clearly define that interface and also broaden the way people can implement 
formatters. For example, it's currently deprecated to use a gen_event 
handler, but if we move to this behaviour then users can use any 
abstraction they want behind that interface as long as the module has an 
`init` function so it can be started under a supervisor. They would most 
likely still be GenServers, but if folks wanted to do something else, they 
totally could. This would require a change to how formatters are stored in 
ExUnit.EventManager to keep a list of the formatter modules that we're 
calling our functions on instead of just relying on 
DynamicSupervisor.which_children/1 to get the PIDs.

I also thought that this behaviour might also work (and maybe be better) 
since it would more easily allow creating formatters that weren't started 
as globally named processes:

```
defmodule ExUnit.Formatter do
  @callback init(config :: list) ::
              {:ok, state}
              | {:ok, state, timeout | :hibernate | {:continue, term}}
              | :ignore
              | {:stop, reason :: any}
            when state: any

  @callback suite_started(formatter :: pid, opts :: list) :: :ok
  @callback suite_finished(formatter :: pid, run_us :: non_neg_integer, 
load_us :: non_neg_integer) :: :ok
  # similar callbacks for all the other events
end
```

This essentially keeps the same behaviour that we have now, but moves the 
implementation details of sending messages to the formatters into the 
formatters themselves as opposed to ExUnit.EventManager.notify/2 knowing 
about the internals of sending messages to the formatters as they do now 
(and has caused issues as evidenced by the gen_event deprecation). For this 
we'd want to change ExUnit.EventManager to keep a mapping of which pids 
belong to which module to make things easy when processing events.

The biggest downside of this that I can see is that it would be a bit of a 
hassle to keep this third way of handling formatters in addition to the 
existing two, but once Elixir 2.0 comes along and breaking changes are 
allowed to be made, we could go down to just this single way of handling 
things and it should be extensible in the future since it more cleanly 
decouples the developer's formatter implementation from ExUnit's 
implementation.

I'd be happy to send a PR on this if y'all think it might make it easier to 
discuss this with a bit of an implementation to look at as a reference.

-- 
You received this message because you are subscribed to the Google Groups 
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/elixir-lang-core/bc15caaa-ee33-4fb4-8287-40d26806bf71%40googlegroups.com.

Reply via email to