HTML Output for White Bread #81
Conversation
We configure the current console outputer as a default under application/0 in the Mix file, fetch the module name, then call/cast into it.
Introduces a module for building HTML. Print to the console for the time being.
|
Code looks 👍 so far. |
|
Certainly! Will get this in the README soon. |
Hard coded to write to ~/report.html for the time being.
| def handle_cast({:scenario_result, {result, _}, %Scenario{name: name}}, state) when :ok == result or :failed == result do | ||
| {:noreply, %{ state | data: [ {result, name} | state.data ]}} | ||
| end | ||
|
|
There was a problem hiding this comment.
There shouldn't be a whitespace between two clauses of the same functions.
There was a problem hiding this comment.
Convention seems different in Elixir: https://github.com/elixir-lang/elixir/blob/master/lib/ex_unit/lib/ex_unit/server.ex#L59-L67
There was a problem hiding this comment.
This is from https://github.com/rrrene/elixir-style-guide:
Group function definitions. Keep the same function with different signatures together without separating blank lines. In all other cases, use blank lines to separate different functions/parts of your module (to maximize readability through "vertical white-space").
We also follow this practice in Erlang.
| end | ||
|
|
||
| def terminate(_, state) do | ||
| import WhiteBread.Outputers.HTML.Formatter |
There was a problem hiding this comment.
It's not clear which imported functions are used
| "Nothing to report." | ||
| end | ||
|
|
||
| def list(elements) when is_list(elements) and length(elements) > 0 do |
There was a problem hiding this comment.
This head could be more concise: def list([_|_] = elements), do: list(elements,"")
|
|
||
| test "file path fetched on initialization" do | ||
| old = Application.fetch_env! :white_bread, :path | ||
| :ok = Application.put_env :white_bread, :path, "/fu/bar.baz" |
There was a problem hiding this comment.
Keeping track of/restoring an app env variable looks like something to be put into setup
There was a problem hiding this comment.
Ordinarily I would but it doesn't seem worth it because we only set it in this one test.
| HTML.handle_cast {:scenario_result, {:ok, "ignore"}, %Scenario{name: "X"}}, %HTML{data: []} | ||
| end | ||
|
|
||
| test "write file on termaination" do |
There was a problem hiding this comment.
would actually be nice to test with a real process and sending in an exit signal - not necessary though
There was a problem hiding this comment.
There are system tests we can modify instead. This way we can just keep unit tests here.
| s = File.stat! p | ||
| assert File.exists? p | ||
| assert s.size > 0 | ||
| File.rm! p |
There was a problem hiding this comment.
The code could be simplified a little with:
assert {:ok, stat} = File.stat(p)
assert stat.size > 0There was a problem hiding this comment.
Yep: don't know why I did it that way.
Thank you to Szymon Mentel for suggested improvements. This introduces the PathError exception too.
Credo wouldn't work as it used an internal Elixir API call that was changed.
Nearing completion so this PR will help to review.