Feature #2643

test/unit redefinition check of test_* method

Added by mame (Yusuke Endoh) over 2 years ago. Updated 2 months ago.

[ruby-core:27790]
Status:Assigned Start date:01/25/2010
Priority:Normal Due date:
Assignee:sorah (Shota Fukumori) % Done:

0%
Category:lib
Target version:2.0.0

Description

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>

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

History

#1
Updated by Eregon (Benoit Daloze) over 2 years ago

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

#2
Updated by mame (Yusuke Endoh) over 2 years ago

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>

#3
Updated by shyouhei (Shyouhei Urabe) over 1 year ago

  • Status changed from Open to Assigned

[ruby-core:38091]#4
Updated by zenspider (Ryan Davis) 10 months ago

  • Status changed from Assigned to Open
  • Assignee deleted (zenspider (Ryan Davis))
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.

[ruby-core:43393]#5
Updated by nahi (Hiroshi Nakamura) 2 months ago

  • 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

#6
Updated by shyouhei (Shyouhei Urabe) 2 months ago

  • Status changed from Open to Assigned

Also available in: Atom PDF