Skip to content

Conversation

@ericentin
Copy link
Contributor

@ericentin ericentin commented May 28, 2016

I would like to pad the message better, at the moment it ends up looking like this:

iex(2)> assert :a == :b
** (ExUnit.AssertionError) Assertion with == failed
code: :a == :b
lhs:  :a
rhs:  :b

    lib/ex_unit/assertions.ex:287: ExUnit.Assertions.assert/2
iex(2)> spawn_link fn -> assert :a == :b end
** (EXIT from #PID<0.57.0>) an exception was raised:
    ** (ExUnit.AssertionError) Assertion with == failed
code: :a == :b
lhs:  :a
rhs:  :b

        lib/ex_unit/assertions.ex:287: ExUnit.Assertions.assert/2

Since we don't have any information about how nested the messages are in Exception.message, adding the appropriate padding seems very difficult, if not impossible. Happy to accept any feedback on how we should format it, given that limitation. I was considering adding an additional newline between the note and code like this:

** (ExUnit.AssertionError) 
Assertion with == failed
code: :a == :b
lhs:  :a
rhs:  :b

    lib/ex_unit/assertions.ex:287: ExUnit.Assertions.assert/2
iex(2)> spawn_link fn -> assert :a == :b end
** (EXIT from #PID<0.57.0>) an exception was raised:
    ** (ExUnit.AssertionError) 
Assertion with == failed
code: :a == :b
lhs:  :a
rhs:  :b

        lib/ex_unit/assertions.ex:287: ExUnit.Assertions.assert/2

but I'm not sure if that makes it look better or worse. :)

\cc @josevalim

@josevalim
Copy link
Member

A new line works nicely for me. :)

@josevalim
Copy link
Member

If you add a new line though, won't it change how all errors are displayed by ExUnit CLI formatter?

]

fields =
if formatter.(:colors_enabled?, nil) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change :colors_enabled? to :diff_enabled? and make the default be false. This way we won't need to check for it here: https://github.com/elixir-lang/elixir/pull/4709/files#diff-699b0020c6a196231a7d473322f1744dR24

We probably use colors_enabled? in a couple other places, so let's not forget to change those. :)

@ericentin
Copy link
Contributor Author

@josevalim not if I only add it in ExUnit.AssertionError.message/1! :bowtie:

@ericentin
Copy link
Contributor Author

Ok, looks like this now, which is pretty nice:

iex(2)> assert :a == :b
** (ExUnit.AssertionError)

Assertion with == failed
code: :a == :b
lhs:  :a
rhs:  :b

    lib/ex_unit/assertions.ex:288: ExUnit.Assertions.assert/2
iex(2)> spawn_link fn -> assert :a = :b end
** (EXIT from #PID<0.57.0>) an exception was raised:
    ** (ExUnit.AssertionError)

match (=) failed
code: :a = :b
rhs:  :b

        (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6
        (stdlib) erl_eval.erl:438: :erl_eval.expr/5
        (stdlib) erl_eval.erl:122: :erl_eval.exprs/5

@josevalim josevalim merged commit 5439a2b into elixir-lang:master May 28, 2016
@ericentin ericentin deleted the enhance-ex-unit-assertion-error-message branch May 28, 2016 16:39
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants