Feature #2643: test/unit redefinition check of test_* method

• Status: Closed
• Priority: Normal
• Assignee: sorah Shota Fukumori

[Yusuke Endoh]

=begin
Hi, Ryan Davis --

When writing test cases with test/unit, we often by mistake define a
test function whose name is already used:

class TestFoo < Test::Unit::TestCase
def test_some_test # not executed
...
end

 ...

 def test_some_test  # redefined
   ...
 end

end

This leads to a loss of a valuable chance of test. It is good for
test/unit to detect and warn such a redefinition.

A patch is attached. Could you merge the patch into test/unit?
I'm less than picky about the implementation detail and error
message.

By the attached patch, I actually found eight redefinitions in
tests of ruby core :-)

• TestUTF16#test_casecmp

  • defined by akr, redefined by akr

• TestHash#test_key?

  • defined by ko1, redefined by ko1

• TestHash#test_value?

  • defined by ko1, redefined by ko1

• TestProc#test_proc_args_opt_and_block

  • defined by yugui, redefined by yugui

• TestEncodingConverter#test_invalid_replace

  • defined by akr, redefined by akr

• TestZlibGzipReader#test_gets

  • defined first by mame, redefined by mame

• TestBigDecimal#test_coerce

  • defined first by mame, redefined by nobu

• TestStringScanner#test_inspect

  • defined first by aamine, redefined by mame

--
Yusuke ENDOH [e-mail]
=end

[Benoit Daloze]

=begin

This leads to a loss of a valuable chance of test.  It is good for
test/unit to detect and warn such a redefinition.

I think you can already see them with warning enabled:
(in Ruby 1.9.2)
test.rb:163: warning: method redefined; discarding old test_one
test.rb:160: warning: previous definition of test_one was here
(in 1.8.7)
test.rb:163: warning: method redefined; discarding old test_one

B.D.

=end

[Yusuke Endoh]

=begin
2010/1/25 Benoit Daloze [e-mail]:

This leads to a loss of a valuable chance of test. It is good for
test/unit to detect and warn such a redefinition.

I think you can already see them with warning enabled:
(in Ruby 1.9.2)
test.rb:163: warning: method redefined; discarding old test_one
test.rb:160: warning: previous definition of test_one was here
(in 1.8.7)
test.rb:163: warning: method redefined; discarding old test_one

The warning is very forgetful. It is waste of time and intelligence
for all test/unit users to enable the warning. In addition, method
redefinition is not always wrong.

But, it is reasonable to enable all warning during test. If so, it is
also good for test/unit to enable warning by default, I think.

--
Yusuke ENDOH [e-mail]

=end

[Shyouhei Urabe]

• Status changed from Open to Assigned

=begin

=end

[Ryan Davis]

• Assignee deleted (Ryan Davis)
• Status changed from Assigned to Open

I agree with Benoit. -w is there for a reason. If you're willing to ignore warnings coming from ruby, why would adding another line that says essentially the same thing be any better?

Regardless, I don't maintain test/unit, but I vote that this ticket be rejected.

[Hiroshi Nakamura]

• Assignee set to sorah Shota Fukumori

Shota, please handle this. We also added you as a maintainer of test/unit. http://bugs.ruby-lang.org/projects/ruby/wiki/Maintainers

[Shyouhei Urabe]

• Status changed from Open to Assigned

[sorah Shota Fukumori]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r36484.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• lib/test/unit.rb: warn when test_* method is redefined.
  Patch by mame (Yusuke Endoh). [Feature #2643]

• test/testunit/test_redefinition.rb: Test for above.

• test/testunit/test4test_redefinition.rb: Ditto.