Skip to content
This repository

Simplify code by taking advantage of latest mocha (v0.13.0). #8180

Merged
merged 1 commit into from over 1 year ago

2 participants

James Mead Carlos Antonio da Silva
James Mead

As well as simplifying the code considerably and reducing coupling to mocha, these changes also fix a few subtle mocha-related bugs present in the current implementation :-

  • Mocha was raising a MiniTest::Assertion instead of a
    Mocha::ExpectationError as intended. The latter is not recognized by
    MiniTest as an assertion failure and so it is recorded as a test
    error, not a test failure as it ought to. This leads to
    potentially confusing output in the test results.

  • Mocha verification should happen as part of the test. The verification
    of expectations is equivalent to a set of assertions. These assertions
    should happen as part of the test so that they have a chance to
    cause the test to fail, and not just as part of the teardown. Also if
    an assertion fails during the test, then there is no need to verify
    expectations, because only the first assertion failure is normally
    reported and all subsequent bets are off.

  • Expectation verification should be counted as an assertion. Mocha
    cannot record each expectation verification as an assertion, because
    we weren't passing in an assertion counter to #mocha_verify.

These changes only work with mocha v0.13.0 or later. In case you would prefer to support older versions of mocha, I will submit an alternative pull request, however the code in that pull request will be necessarily more complicated/messy.

I'd be more than happy to provide back-ported fixes to 3-x-stable branches if these would be useful.

James Mead Simplify code by taking advantage of latest mocha (v0.13.0).
This only works with mocha v0.13.0 or later.

Note that this also fixes a few subtle bugs present in the current
implementation :-

* Mocha was raising a `MiniTest::Assertion` instead of a
  `Mocha::ExpectationError` as intended. The latter is not recognized by
  MiniTest as an assertion failure and so it is recorded as a test
  *error*, not a test *failure* as it ought to. This leads to
  potentially confusing output in the test results.

* Mocha verification should happen as part of the test. The verification
  of expectations is equivalent to a set of assertions. These assertions
  should happen as *part of* the test so that they have a chance to
  cause the test to fail, and not just as part of the teardown. Also if
  an assertion fails during the test, then there is no need to verify
  expectations, because only the first assertion failure is normally
  reported and all subsequent bets are off.

* Expectation verification should be counted as an assertion. Mocha
  cannot record each expectation verification as an assertion, because
  we weren't passing in an assertion counter to `#mocha_verify`.
57b333e
James Mead

The alternative pull request I mentioned above is this one.

James Mead

I have also submitted a pull request for 3-2-stable so Rails v3.2 works with Mocha v0.13.0.

Carlos Antonio da Silva

I think we're going to stick with support for Mocha >= 0.13.0 for now, thanks!

Carlos Antonio da Silva carlosantoniodasilva merged commit 395d6b4 into from November 13, 2012
Carlos Antonio da Silva carlosantoniodasilva closed this November 13, 2012
James Mead

Great. That'd be my preferred option anyway!

I think that since require 'mocha/setup' will fail for earlier versions of Mocha, then no Mocha-related functionality will be available and it'll be pretty obvious that something is wrong.

James Mead floehopper referenced this pull request in blowmage/minitest-rails December 01, 2012
Merged

Avoid deprecation warning in Mocha 0.13.0. #85

Yves Senn senny referenced this pull request March 28, 2013
Closed

Mocha Constant Warning #9478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Nov 12, 2012
James Mead Simplify code by taking advantage of latest mocha (v0.13.0).
This only works with mocha v0.13.0 or later.

Note that this also fixes a few subtle bugs present in the current
implementation :-

* Mocha was raising a `MiniTest::Assertion` instead of a
  `Mocha::ExpectationError` as intended. The latter is not recognized by
  MiniTest as an assertion failure and so it is recorded as a test
  *error*, not a test *failure* as it ought to. This leads to
  potentially confusing output in the test results.

* Mocha verification should happen as part of the test. The verification
  of expectations is equivalent to a set of assertions. These assertions
  should happen as *part of* the test so that they have a chance to
  cause the test to fail, and not just as part of the teardown. Also if
  an assertion fails during the test, then there is no need to verify
  expectations, because only the first assertion failure is normally
  reported and all subsequent bets are off.

* Expectation verification should be counted as an assertion. Mocha
  cannot record each expectation verification as an assertion, because
  we weren't passing in an assertion counter to `#mocha_verify`.
57b333e
This page is out of date. Refresh to see the latest.
8  activesupport/lib/active_support/test_case.rb
@@ -5,16 +5,18 @@
5 5
 require 'active_support/testing/assertions'
6 6
 require 'active_support/testing/deprecation'
7 7
 require 'active_support/testing/isolation'
8  
-require 'active_support/testing/mocha_module'
9 8
 require 'active_support/testing/constant_lookup'
10 9
 require 'active_support/core_ext/kernel/reporting'
11 10
 require 'active_support/deprecation'
12 11
 
  12
+begin
  13
+  silence_warnings { require 'mocha/setup' }
  14
+rescue LoadError
  15
+end
  16
+
13 17
 module ActiveSupport
14 18
   class TestCase < ::MiniTest::Spec
15 19
 
16  
-    include ActiveSupport::Testing::MochaModule
17  
-
18 20
     # Use AS::TestCase for the base class when describing a model
19 21
     register_spec_type(self) do |desc|
20 22
       Class === desc && desc < ActiveRecord::Base
22  activesupport/lib/active_support/testing/mocha_module.rb
... ...
@@ -1,22 +0,0 @@
1  
-module ActiveSupport
2  
-  module Testing
3  
-    module MochaModule
4  
-      begin
5  
-        require 'mocha/api'
6  
-        include Mocha::API
7  
-
8  
-        def before_setup
9  
-          mocha_setup
10  
-          super
11  
-        end
12  
-
13  
-        def after_teardown
14  
-          super
15  
-          mocha_verify
16  
-          mocha_teardown
17  
-        end
18  
-      rescue LoadError
19  
-      end
20  
-    end
21  
-  end
22  
-end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.