Backward compatibility broken in 2.4.9 #28

Closed
VladRassokhin opened this Issue Jun 5, 2012 · 5 comments

Comments

Projects
None yet
2 participants

Backward compatibility broken in API with that change.
I mean renaming & changing signature of method "run_suite".

Same changes must not be included in maintenance releases. Only in minor/major releases.
Just because this behavior is unexpected and produce bugs in other software.

So, could you please release 2.4.10 with fix for that?

Owner

kou commented Jun 6, 2012

Sorry...

Could you show a problem case? I'll add a backward compatibility API and release a new version.

We are patching Test::Unit::UI::TestRunnerMediator as part of uor TeamCity reporter for test-unit framework, so our code looks like:

class Test::Unit::UI::TestRunnerMediator
  TC_TESTCOUNT = name + "::TC_TESTCOUNT"
  TC_REPORTER_ATTACHED = name + "::TC_REPORTER_ATTACHED"

  alias :old_run_suite :run_suite
  def run_suite
    count = calc_patched_size(@suite)

    # Notify test reporter attached
    notify_listeners(TC_REPORTER_ATTACHED)

    # Notify patched size
    notify_listeners(TC_TESTCOUNT, count)    
    # delegate call to super
    old_run_suite ## Line number 41, error occurred here
  end
#some other code
end

So we have our integration tests failed with stacktrace:

c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/ui/testrunnermediator.rb:82:in `run_suite': wrong number of arguments (0 for 1) (ArgumentError)
      from ##BASE_DIR##/svnrepo/rake-runner/lib/rb/patch/testunit/test/unit/ui/testrunnermediator.rb:41:in `run_suite'
      from ##BASE_DIR##/svnrepo/rake-runner/lib/rb/patch/testunit/test/unit/ui/teamcity/testrunner.rb:131:in `start_mediator'
      from ##BASE_DIR##/svnrepo/rake-runner/lib/rb/patch/testunit/test/unit/ui/teamcity/testrunner.rb:119:in `start'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/ui/testrunnerutilities.rb:24:in `run'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/autorunner.rb:378:in `block in run'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/autorunner.rb:434:in `change_work_directory'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/autorunner.rb:377:in `run'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit/autorunner.rb:58:in `run'
      from c:/ruby/ruby-1.9.2/lib/ruby/gems/1.9.1/gems/test-unit-2.4.9/lib/test/unit.rb:330:in `block in <top (required)>'

kou added a commit that referenced this issue Jun 6, 2012

Use "run_suite" name as an alias of "run" for backward compatibility
GitHub: #28

Reported by Vladislav Rassokhin. Thanks!!!

@kou kou closed this Jun 6, 2012

Owner

kou commented Jun 6, 2012

I've fixed it and released a new version 2.5.0!
Please try it!

Why version is 2.5.0?
What versioning strategy do you use? Why not Semantic Versioning?

Because if users have

gem "test-unit", "~>2.4.0"

in Gemfile they will newer have that fix. Because Bundler and rubygems uses Semantic Versioning

Owner

kou commented Jun 7, 2012

test-unit doesn't use 2 or more digits for version component like CRuby does.

You should know about monkey patch introduces a trade-off between easy-to-change and difficult-to-maintain.
You should use specific version (e.g. "=2.4.0") with monkey patch.

I think that you don't try it yet because I noticed that your code doesn't work with 2.5.0. (Yes, it's my mistake.)
But I'll never work to work your code. It'll block test-unit improvements.

You should use "=2.4.8" or substitute "run_suite" with "run" in your code.

kou added a commit that referenced this issue Jun 10, 2012

Revert "Use "run_suite" name as an alias of "run" for backward compat…
…ibility"

This reverts commit 7dec384.

This change doesn't solve the problem reported as GitHub #28.
And the problem will never be solved because it will block improvements.
So I reverted this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment