elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.34k stars 3.36k forks source link

TCO swallows stacktraces in some ex_unit assertions #10289

Closed michalmuskala closed 4 years ago

michalmuskala commented 4 years ago

Environment

Elixir 1.10.3 (compiled with Erlang/OTP 22)

* Operating system: macOS

### Current behavior

Given the following snippet
```elixir
import ExUnit.Assertions

defmodule Foo do
  def foo do
    assert 100 < 0
  end
end

Foo.foo()

when executed, a following error is produced

** (ExUnit.AssertionError)

Assertion with < failed
code:  assert 100 < 0
left:  100
right: 0

    lib/ex_unit/assertions.ex:393: ExUnit.Assertions.assert/2
    (elixir 1.10.3) lib/code.ex:926: Code.require_file/2

In particular the produced stacktrace does not include the Foo.foo function call.

Examining the expansion of the macro (though mix decompile), we can see it expands to:

defmodule Foo do
  def foo() do
    left = 100
    right = 0

    case(ExUnit.Assertions.__equal__?(left, right)) do
      x when :erlang.orelse(:erlang."=:="(x, false), :erlang."=:="(x, nil)) ->
        ExUnit.Assertions.assert(:erlang.<(left, right),
          left: left,
          right: right,
          expr: {:assert, [line: 5], [{:<, [line: 5], [100, 0]}]},
          message: "Assertion with < failed"
        )

      _ ->
        ExUnit.Assertions.assert(false,
          left: left,
          expr: {:assert, [line: 5], [{:<, [line: 5], [100, 0]}]},
          message: "Assertion with < failed, both sides are exactly equal"
        )
    end
  end
end

with calls to ExUnit.Assertions.assert/2 in tail positions that cause TCO to kick in and the stacktrace entry to be omitted.

Expected behavior

While for most functions, I would not complain about this, for things that are purpusefully built to produce good error messages, I think it's worth it to improve. I'd expect there to be some barrier that would prevent the compiler from applying TCO and dropping the stacktrace entry when the assertion fails.

josevalim commented 4 years ago

I remember discussing this a long time ago. We generally solved this with the test macro which always returns :ok its last expression. We decided to not to do that as part of assert because some assertions are functions (assert_raise, assert_receive, assert/4) and some are macros, and improving only the macros would be inconsistent.

michalmuskala commented 4 years ago

Ah, that definitely makes sense to solve it at the higher level. I'm going to close this.