Use test-unit's cleanup hook instead of monkey patching #89

Closed
wants to merge 3 commits into
from

Projects

None yet

2 participants

@kou
kou commented Jul 8, 2012

test-unit supports cleanup hook since v2.3.0. [1][2]
Mocha can use it for verifying. It is more maintainable way rather than
overriding run method because cleanup hook is public API.

test-unit-rr, RR adapter for test-unit, also uses the cleanup hook. [3]

This change supports test-unit v2.5.1. It is the latest version.

[1] http://test-unit.rubyforge.org/test-unit/en/file.news.html#2_3_0___2011-04-17
[2] "Overview" section in http://test-unit.rubyforge.org/test-unit/en/Test/Unit/TestCase.html
[3] https://github.com/test-unit/test-unit-rr/blob/master/lib/test/unit/rr.rb

@kou kou Use test-unit's cleanup hook instead of monkey patching
test-unit supports cleanup hook since v2.3.0. [1][2]
Mocha can use it for verifying. It is more maintainable way rather than
overriding `run` method because cleanup hook is public API.

test-unit-rr, RR adapter for test-unit, also uses the cleanup hook. [3]

This change supports test-unit v2.5.1. It is the latest version.

[1] http://test-unit.rubyforge.org/test-unit/en/file.news.html#2_3_0___2011-04-17
[2] "Overview" section in http://test-unit.rubyforge.org/test-unit/en/Test/Unit/TestCase.html
[3] https://github.com/test-unit/test-unit-rr/blob/master/lib/test/unit/rr.rb
2570267
@floehopper
Member

Thanks for submitting this. I'm definitely sympathetic to the intention. I'll take a proper look at your patch as soon as I can. Will this cope with an Mocha::ExpectationError being raised in setup e.g. due to an unexpected invocation on a mock object? The current code translates this exception into a Test::Unit assertion failure.

As per my comment in #87, I'm considering an alternative strategy whereby Mocha has an exception factory to raise the exception appropriate to the test framework, or dynamically defining Mocha::ExpectationError and inheriting from the appropriate test framework exception.

@kou
kou commented Jul 9, 2012

Will this cope with an Mocha::ExpectationError being raised in setup e.g. due to an unexpected invocation on a mock object?

The patch can't cope the case!
I've pushed new patch as 2c61b1d. It uses undocumented (sorry :<) custom exception handler API.

ac15aae is code cleanup. Sorry for including it in the first patch...

As per my comment in #87, I'm considering an alternative strategy whereby Mocha has an exception factory to raise the exception appropriate to the test framework, or dynamically defining Mocha::ExpectationError and inheriting from the appropriate test framework exception.

In my opinion, it looks that testing Mocha itself is difficult a bit...

@kou kou referenced this pull request in test-unit/test-unit Jul 10, 2012
Closed

private method `run' called #36

@kou kou added a commit to test-unit/test-unit-activesupport that referenced this pull request Jul 20, 2012
@kou kou Add workaroud for Mocha
This is a workaround until freerange/mocha#89 is merged.

GitHub: #2

Reported by Michael Grosser. Thanks!!!
6c3be55
@kou kou referenced this pull request in test-unit/test-unit-activesupport Jul 21, 2012
Closed

Fails with activesupport 2.3 #2

@floehopper
Member

I still haven't managed to find time to look at this, but I'm hoping it will be addressed by my changes in the minitest-and-testunit-integration-without-monkey-patching branch. More specifically in Mocha::Integration::TestUnit.

The workaround that @kou has had to add to Test::Unit is obviously undesirable, so it would be good to resolve this as soon as possible.

@kou
kou commented Aug 10, 2012

Thanks for working on it!

I hope that the branch is merged soon. 😄

@kou kou referenced this pull request in test-unit/test-unit Aug 15, 2012
Closed

"private method `run_suite' called" in 2.5.1 #38

@floehopper floehopper added a commit that referenced this pull request Aug 24, 2012
@floehopper floehopper Tweaks to new Test::Unit adapter.
* It's reassuring to see that the changes I independently arrived at
  in e9d05cc are almost identical to
  those @kou made in #89.
* This commit brings the code more into line with what @kou recommended,
  although I haven't extracted all the private methods that he did -
  they seem like overkill to me.
* I'm still curious to know whether #problem_occurred and #add_failure
  are part of the Test::Unit public API...?
7869f38
@floehopper
Member

@kou what's the earliest version of Test::Unit that will support these changes?

@kou
kou commented Aug 25, 2012

It is 2.3.0.

  • cleanup hook: >= 2.3.0
  • custom exception handler: >= 2.0.0
    ** custom exception handler by block: >= 2.5.2 (2.5.2 will be released next week.)
@floehopper
Member

@kou can you explain what the :after => :append option does? And should I add it to the setup declaration as well as cleanup & teardown? Ideally I want the mocha setup to run before any other setup. Thanks.

@kou
kou commented Aug 26, 2012

OK.

Short answer: Add :before => :prepend to setup.

Long answer:
setup, teardown and cleanup have its own callback list. And each callback list has two sub callback lists. They are before callback list and after callback list. Before callback list is used before the test's callback. The test's callback means user defined callback by def setup; ...; end. After callback list is used after the test's callback.

There are four patterns to select a sub callback list:

  • :before => :prepend: Prepends a callback to before callback list.
  • :before => :append: Appends a callback to before callback list.
  • :after => :prepend: Prepends a callback to after callback list.
  • :after => :append: Appends a callback to after callback list.

setup uses :after => :append by default.
teardown uses :before => :prepend by default.
cleanup uses :before => :prepend by default.

To run the mocha setup before any other setup, use :before => :prepend for setup.

Here is a sample test to explain the behavior:

require "test-unit"

class TestMyClass < Test::Unit::TestCase
  setup :before => :prepend do
    p [:setup, :before, :prepend, 1]
  end

  setup :before => :prepend do
    p [:setup, :before, :prepend, 2]
  end

  setup :before => :append do
    p [:setup, :before, :append, 1]
  end

  setup :before => :append do
    p [:setup, :before, :append, 2]
  end

  setup :after => :prepend do
    p [:setup, :after, :prepend, 1]
  end

  setup :after => :prepend do
    p [:setup, :after, :prepend, 2]
  end

  setup :after => :append do
    p [:setup, :after, :append, 1]
  end

  setup :after => :append do
    p [:setup, :after, :append, 2]
  end

  setup do
    p [:setup, :no_option, 1]
  end

  setup do
    p [:setup, :no_option, 2]
  end

  def setup
    p :setup
  end

  cleanup :before => :prepend do
    p [:cleanup, :before, :prepend, 1]
  end

  cleanup :before => :prepend do
    p [:cleanup, :before, :prepend, 2]
  end

  cleanup :before => :append do
    p [:cleanup, :before, :append, 1]
  end

  cleanup :before => :append do
    p [:cleanup, :before, :append, 2]
  end

  cleanup :after => :prepend do
    p [:cleanup, :after, :prepend, 1]
  end

  cleanup :after => :prepend do
    p [:cleanup, :after, :prepend, 2]
  end

  cleanup :after => :append do
    p [:cleanup, :after, :append, 1]
  end

  cleanup :after => :append do
    p [:cleanup, :after, :append, 2]
  end

  cleanup do
    p [:cleanup, :no_option, 1]
  end

  cleanup do
    p [:cleanup, :no_option, 2]
  end

  def cleanup
    p :cleanup
  end

  teardown :before => :prepend do
    p [:teardown, :before, :prepend, 1]
  end

  teardown :before => :prepend do
    p [:teardown, :before, :prepend, 2]
  end

  teardown :before => :append do
    p [:teardown, :before, :append, 1]
  end

  teardown :before => :append do
    p [:teardown, :before, :append, 2]
  end

  teardown :after => :prepend do
    p [:teardown, :after, :prepend, 1]
  end

  teardown :after => :prepend do
    p [:teardown, :after, :prepend, 2]
  end

  teardown :after => :append do
    p [:teardown, :after, :append, 1]
  end

  teardown :after => :append do
    p [:teardown, :after, :append, 2]
  end

  teardown do
    p [:teardown, :no_option, 1]
  end

  teardown do
    p [:teardown, :no_option, 2]
  end

  def teardown
    p :teardown
  end

  def test_my_method
    p :test
  end
end

It outputs the following:

[:setup, :before, :prepend, 2]
[:setup, :before, :prepend, 1]
[:setup, :before, :append, 1]
[:setup, :before, :append, 2]
:setup
[:setup, :after, :prepend, 2]
[:setup, :after, :prepend, 1]
[:setup, :after, :append, 1]
[:setup, :after, :append, 2]
[:setup, :no_option, 1]
[:setup, :no_option, 2]
:test
[:cleanup, :no_option, 2]
[:cleanup, :no_option, 1]
[:cleanup, :before, :prepend, 2]
[:cleanup, :before, :prepend, 1]
[:cleanup, :before, :append, 1]
[:cleanup, :before, :append, 2]
:cleanup
[:cleanup, :after, :prepend, 2]
[:cleanup, :after, :prepend, 1]
[:cleanup, :after, :append, 1]
[:cleanup, :after, :append, 2]
[:teardown, :no_option, 2]
[:teardown, :no_option, 1]
[:teardown, :before, :prepend, 2]
[:teardown, :before, :prepend, 1]
[:teardown, :before, :append, 1]
[:teardown, :before, :append, 2]
:teardown
[:teardown, :after, :prepend, 2]
[:teardown, :after, :prepend, 1]
[:teardown, :after, :append, 1]
[:teardown, :after, :append, 2]
@kou
kou commented Aug 29, 2012

FYI: test-unit 2.5.2 has been released!
Custom exception handler by block feature is included in the release.

@floehopper
Member

@kou I believe the latest release of mocha (v0.13.0) incorporates the changes we discussed. Please close this issue if you are happy it is now resolved. Thanks.

@kou
kou commented Nov 14, 2012

It looks good! Thanks!

@kou kou closed this Nov 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment