sconover / wrong

Wrong provides a general assert method that takes a predicate block. Assertion failure messages are rich in detail.
MIT License
433 stars 30 forks source link

Misleading output when variables inside assert block have gone out of scope #32

Closed wwcline closed 11 years ago

wwcline commented 11 years ago

This is closely related to https://github.com/sconover/wrong/issues/21.

In his comment on that issue, @alexch pointed out that variables inside an assert block might be out of scope when the gem tries to introspect them. This can make for misleading output. An example using Test::Unit:

  test "wrong block example" do
    assert do
      (1..5).all? do |num|
        num.in? 1..4
      end
    end
  end

This test fails with the following output:

===============================================================================
Failure:
  Expected (1..5).all? { |num| num.in?((1..4)) }, but
      num.in?((1..4)) raises NameError: undefined local variable or method `num' for test_wrong_block_example(WorkShelfTest):WorkShelfTest
      num raises NameError: undefined local variable or method `num' for test_wrong_block_example(WorkShelfTest):WorkShelfTest
      (1..4) is 1..4
test_wrong_block_example(WorkShelfTest)
test/unit/work_shelf_test.rb:7:in `block in <class:WorkShelfTest>'
===============================================================================

A test failure like this had me scratching my head for several minutes today, because I couldn't see how num could possibly be undefined.

Perhaps the solution is not to report such NameErrors in test output? If a real NameError exception were being generated by the code inside the assert block, we could rely on the test framework to report that exception, right?

alexch commented 11 years ago

Interesting proposal. I haven't quite thought it through all the way but it seems plausible. I think even if you're right it's not a good idea -- if we open the door to this exceptional exception, are there any others? And will that lead to confusion from someone else who does understand the (admittedly baroque) semantics of nested expression evaluation?

The recommended idiom (and solution) is to wrap the assertion around the smallest block of code possible, viz.

  (1..5).each do |num|
    assert { num.in? 1..4 }
  end

which would both cure the NameError and give you a more precise failure (namely that 5 is the num that fails).

(all? is not going to work because the assert will return nil and abort the loop)

Is in? part of Test::Unit? Or a new method you're testing? Seems useful! Maybe I'll add it to Wrong :-)

wwcline commented 11 years ago

Is in? part of Test::Unit? Or a new method you're testing? Seems useful! Maybe I'll add it to Wrong :-)

It's an extension of Object supplied by Ruby on Rails:

http://api.rubyonrails.org/classes/Object.html#method-i-in-3F

I think even if you're right it's not a good idea -- if we open the door to this exceptional exception, are there any others? And will that lead to confusion from someone else who does understand the (admittedly baroque) semantics of nested expression evaluation?

I think I agree. Moreover, while reviewing the README to see if I could propose a revision that would warn about this gotcha, I saw that just such a warning is already there:

https://github.com/sconover/wrong#details

Thanks for reading, and sorry for the bother.