Feature #2643

test/unit redefinition check of test_* method

Added by Yusuke Endoh about 5 years ago. Updated over 2 years ago.

[ruby-core:27790]
Status:Closed
Priority:Normal
Assignee:Shota Fukumori

Description

=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 mame@tsg.ne.jp
=end

test-unit-redefinition-check.patch Magnifier (565 Bytes) Yusuke Endoh, 01/25/2010 07:41 PM

Associated revisions

Revision 36484
Added by Shota Fukumori over 2 years ago

  • 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.

Revision 36484
Added by Shota Fukumori over 2 years ago

  • 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.

History

#1 Updated by Benoit Daloze about 5 years ago

=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

#2 Updated by Yusuke Endoh about 5 years ago

=begin
2010/1/25 Benoit Daloze eregontp@gmail.com:

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 mame@tsg.ne.jp

=end

#3 Updated by Shyouhei Urabe over 4 years ago

  • Status changed from Open to Assigned

=begin

=end

#4 Updated by Ryan Davis over 3 years ago

  • 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.

#5 Updated by Hiroshi Nakamura almost 3 years ago

  • Assignee set to 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

#6 Updated by Shyouhei Urabe almost 3 years ago

  • Status changed from Open to Assigned

#7 Updated by Shota Fukumori over 2 years ago

  • 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.

Also available in: Atom PDF