Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix 3-2-stable to work with Mocha v0.13.0 #8200

Merged
merged 4 commits into from

6 participants

@floehopper

Two commits (716bdfc & c3e186e) are fixes for problems that existed prior to the release of Mocha v0.13.0, but have remained unnoticed.

The other two commits (5573c1d & 3791cac) fix Rails v3.2 to work with Mocha v0.13.0.

floehopper added some commits
@floehopper floehopper Copy Mocha bug fix.
A bug was fixed [1] in Mocha's integration with Test::Unit, but this
monkey-patching code was copied before the fix. We need to copy the
fixed version.

The bug meant that an unexpected invocation against a mock within the
teardown method caused a test *error* and not a test *failure*.

[1]
freerange/mocha@f1ff647#diff-5
716bdfc
@floehopper floehopper Use MiniTest in Ruby 1.8 if it is available.
ActiveSupport::TestCase was always inheriting from Test::Unit::TestCase.
This works fine in Ruby 1.9 where Test::Unit::TestCase is a thin wrapper
around MiniTest::Unit::TestCase, but does not work in Ruby 1.8 if the
MiniTest gem is used. What happens is that ActiveSupport inherits from
the Test::Unit::TestCase provided by the standard library, but then
since Minitest is defined, it then seems to proceed on the assumption
that ActiveSupport::TestCase has MiniTest::Unit::TestCase in its
ancestor chain. However, in this case it does not.

The fix is simply to choose which test library TestCase to inherit from
using the same logic used elsewhere to detect MiniTest.

I noticed this bug causing issues when using MiniTest and Mocha
in Ruby 1.8, but there may well be other issues.
c3e186e
@floehopper floehopper Fix for Test::Unit Mocha compatibility.
Mocha is now using a single AssertionCounter which needs a reference to
the testcase as opposed to the result.

This change is an unfortunate consequence of the copying of a chunk of
Mocha's internal code in order to monkey-patch Test::Unit.
5573c1d
@floehopper floehopper Avoid a Mocha deprecation warning. 3791cac
@carlosantoniodasilva carlosantoniodasilva merged commit 0ed585a into rails:3-2-stable
@carlosantoniodasilva

Thanks!

@floehopper

Great! No problem.

@nikitug nikitug referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@floehopper

Any idea when this might be released? Thanks.

@floehopper floehopper referenced this pull request in freerange/mocha
Closed

Impossible to boot mocha properly with rspec-rails? #115

@steveklabnik
Collaborator

Whenever it does. There's no direct plans for a 3.2.x release that I'm aware of right now.

@pboling

Please release a non security release of rails 3.2.X so we can get these non-security fixes! :+1:

@mattscilipoti

YARR (Yet another release request). Please release a version that will remove this deprecation noise.

@sbleon

+1

@floehopper

It's worth noting that the equivalent changes have already been (possibly inadvertently) released in the 3.0.20 & 3.1.11 security releases. However, these changes are not in the 3.2.12 release. It seems odd that there is such a discrepancy.

I do understand that this is because the recent 3-2-stable releases have been security fixes, however there hasn't been a non-security release of 3-2-stable for some months now.

I'd be really grateful if there could be a non-security release of 3-2-stable - the Mocha deprecation warnings are causing a lot of confusion.

@carlosantoniodasilva

@floehopper yeah those changes were released because they were merged into those branches just to work with latest mocha, but since the merge policy for those branches is "security fixes only", they were supposedly ready to be released as is - so they went out with the changes. 3-2, on the other way, branched out to release the sec fixes on top of the last stable version (not the branch), and thus not including anything in 3-2-stable branch.

This was probably unfortunate but it is how it happened. In any case, we're not merging anything else in 3.0/3.1 branches anymore. And about a new 3-2 release, I agree it's been a while but it had to wait due to the latest sec releases. Lets see if we can get something out soon, thanks!

@floehopper

Thanks. I appreciate all your efforts.

@bobbus bobbus referenced this pull request from a commit in bobbus/minitest-rails
@floehopper floehopper Avoid deprecation warning in Mocha 0.13.0.
I'm not entirely sure why this code is duplicated from ActiveSupport,
but this deprecation warning has been fixed in 3-2-stable [1] and in
master [2].

[1] rails/rails#8200
[2] rails/rails#8180
b3aa657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 13, 2012
  1. @floehopper

    Copy Mocha bug fix.

    floehopper authored
    A bug was fixed [1] in Mocha's integration with Test::Unit, but this
    monkey-patching code was copied before the fix. We need to copy the
    fixed version.
    
    The bug meant that an unexpected invocation against a mock within the
    teardown method caused a test *error* and not a test *failure*.
    
    [1]
    freerange/mocha@f1ff647#diff-5
  2. @floehopper

    Use MiniTest in Ruby 1.8 if it is available.

    floehopper authored
    ActiveSupport::TestCase was always inheriting from Test::Unit::TestCase.
    This works fine in Ruby 1.9 where Test::Unit::TestCase is a thin wrapper
    around MiniTest::Unit::TestCase, but does not work in Ruby 1.8 if the
    MiniTest gem is used. What happens is that ActiveSupport inherits from
    the Test::Unit::TestCase provided by the standard library, but then
    since Minitest is defined, it then seems to proceed on the assumption
    that ActiveSupport::TestCase has MiniTest::Unit::TestCase in its
    ancestor chain. However, in this case it does not.
    
    The fix is simply to choose which test library TestCase to inherit from
    using the same logic used elsewhere to detect MiniTest.
    
    I noticed this bug causing issues when using MiniTest and Mocha
    in Ruby 1.8, but there may well be other issues.
  3. @floehopper

    Fix for Test::Unit Mocha compatibility.

    floehopper authored
    Mocha is now using a single AssertionCounter which needs a reference to
    the testcase as opposed to the result.
    
    This change is an unfortunate consequence of the copying of a chunk of
    Mocha's internal code in order to monkey-patch Test::Unit.
  4. @floehopper
This page is out of date. Refresh to see the latest.
View
4 activesupport/lib/active_support/test_case.rb
@@ -9,7 +9,9 @@
require 'active_support/core_ext/kernel/reporting'
module ActiveSupport
- class TestCase < ::Test::Unit::TestCase
+ test_library = defined?(MiniTest) ? ::MiniTest : ::Test
+
+ class TestCase < test_library::Unit::TestCase
if defined? MiniTest
Assertion = MiniTest::Assertion
alias_method :method_name, :name if method_defined? :name
View
2  activesupport/lib/active_support/testing/mochaing.rb
@@ -1,5 +1,5 @@
begin
- silence_warnings { require 'mocha' }
+ silence_warnings { require 'mocha/setup' }
rescue LoadError
# Fake Mocha::ExpectationError so we can rescue it in #run. Bleh.
Object.const_set :Mocha, Module.new
View
10 activesupport/lib/active_support/testing/setup_and_teardown.rb
@@ -61,7 +61,7 @@ module ForClassicTestUnit
def run(result)
return if @method_name.to_s == "default_test"
- mocha_counter = retrieve_mocha_counter(result)
+ mocha_counter = retrieve_mocha_counter(self, result)
yield(Test::Unit::TestCase::STARTED, name)
@_result = result
@@ -83,6 +83,8 @@ def run(result)
begin
teardown
run_callbacks :teardown
+ rescue Mocha::ExpectationError => e
+ add_failure(e.message, e.backtrace)
rescue Test::Unit::AssertionFailedError => e
add_failure(e.message, e.backtrace)
rescue Exception => e
@@ -100,14 +102,16 @@ def run(result)
protected
- def retrieve_mocha_counter(result) #:nodoc:
+ def retrieve_mocha_counter(test_case, result) #:nodoc:
if respond_to?(:mocha_verify) # using mocha
if defined?(Mocha::TestCaseAdapter::AssertionCounter)
Mocha::TestCaseAdapter::AssertionCounter.new(result)
elsif defined?(Mocha::Integration::TestUnit::AssertionCounter)
Mocha::Integration::TestUnit::AssertionCounter.new(result)
- else
+ elsif defined?(Mocha::MonkeyPatching::TestUnit::AssertionCounter)
Mocha::MonkeyPatching::TestUnit::AssertionCounter.new(result)
+ else
+ Mocha::Integration::AssertionCounter.new(test_case)
end
end
end
Something went wrong with that request. Please try again.