This project is archived and is in readonly mode.

#2858 ✓resolved
James Mead

Mocha release 0.9.6 heads-up

Reported by James Mead | June 30th, 2009 @ 06:12 PM | in 2.x

This is just a heads up that ActiveSupport::TestCase might need some love after the latest Mocha release. Mocha no longer loads any test framework by default. Instead it applies the required monkey-patch to any supported [1] test framework that has been loaded before Mocha is loaded.

I've just had a bug report [2] from someone who was using the Test::Unit gem. I suggested they fixed it by explicitly loading the test-unit gem before requiring "test_help" in "test/test_helper.rb"

[1] Test::Unit (standard library or gem), MiniTest (standard library or gem). [2] http://floehopper.lighthouseapp.com/projects/22289-mocha/tickets/50

Comments and changes to this ticket

  • Repository

    Repository July 1st, 2009 @ 08:12 PM

    • State changed from “new” to “resolved”

    (from [a8bd3c8a109fe88a4ce567e4272fd796e42b42da]) Move mocha down below initial T::U require and bump version to 0.9.7 [#2858 state:resolved] http://github.com/rails/rails/commit/a8bd3c8a109fe88a4ce567e4272fd7...

  • Repository
  • Michael Koziarski

    Michael Koziarski July 2nd, 2009 @ 10:25 AM

    Was this meant to be 0.9.6 not 0.9.7?

  • James Mead

    James Mead July 2nd, 2009 @ 10:26 AM

    I think using 0.9.7 is fine - the difference between 0.9.6 and 0.9.7 was just an emergency fix for RSpec.

  • Michael Koziarski

    Michael Koziarski July 2nd, 2009 @ 10:30 AM

    Cool, I guess this makes us 2.3.3 safe then

  • James Mead

    James Mead July 2nd, 2009 @ 10:33 AM

    BTW - Thanks for the quick response. I haven't yet had any time to look at the ActiveSupport::TestCase code in detail, but won't your change mean that the monkey patch made by ActiveSupport::Testing::SetupAndTeardown::ForClassicTestUnit#run will be overwritten by Mocha's monkey-patch. The obvious implication for this is that the testing callbacks will not work (but I haven't had a chance to try them).

    As a more general observation, I hadn't realised how hacky the ActiveSupport::TestCase code has had to be to support Mocha. I know that this is mainly due to the lack of decent extensibility in Test::Unit, but I wonder if we can come up with a better solution. Perhaps we can get together with other extenders of Test::Unit and come up with a standardized modification to Test::Unit so that we can all safely modify Test::Unit without breaking each others stuff...?

    As another general observation, am I right in thinking that the reason Mocha is loaded in ActiveSupport::TestCase is in order to support Rails tests and not so that people can use Mocha in their application tests? If this assumption is correct, I wonder if there is value in separating the TestCase class used for Rails own tests and the TestCase supplied to the punters for their application tests. The latter would not need to require Mocha.

    Anyway, just a few thoughts.

    Cheers, James.

  • James Mead

    James Mead July 2nd, 2009 @ 10:37 AM

    I guess what I meant to say in my first point was that I understand moving the loading of Mocha to a point after the loading of Test::Unit, but I'm not sure I understand moving it to a point after the loading of ActiveSupport::SetupAndTeardown, etc. But I may well be missing something.

  • Michael Koziarski

    Michael Koziarski July 2nd, 2009 @ 10:40 AM

    My brain's a bit fried by this head-cold so I'm not sure on yehuda's
    changes either. Will check them out properly in the morning, but odds
    are this lighthouse email spam will make him try it out in the morning
    ;)

    The double-monkey-patching is incredibly unfortunate but we both need
    those callbacks and I'm not sure there's a nice way around this if we
    want to support users using AS::TestCase and mocha in their own apps?

    We could introduce our own subclasses for rails' own tests but users
    will have the same issues won't they?

  • David Chelimsky

    David Chelimsky July 2nd, 2009 @ 02:18 PM

    I had posted #2240 a while back, requesting that mocha either not be auto-included in AS::TC, or that it least be made configurable (so you could turn it off). The reason is that RSpec derives it's ExampleGroup classes from AS::TC when working with rails, and RSpec offers users the option of using mocha, rr, flexmock or RSpec's own mocking framework. When they choose anything other than mocha, but they have mocha installed on their system, they end up getting more than one mock framework attaching itself to every example (test method). For the most part this does not cause any real problems, but I do recall seeing a couple of messages on the rspec-users mailing list where people were confused by unexpected references to mocha in backtraces.

    In #2240 I seemed to have buy-in from Pratik and Jeremy, and the next step was for me to supply a patch, but I had trouble getting the Rails test suite to run successfully in the first place and gave up.

    Having said all that, I'd be curious about your thoughts on this.

    Thanks,
    David

  • James Mead

    James Mead July 2nd, 2009 @ 03:28 PM

    I've just realised that I was mistaken in my concern about the load order. Although 'active_support/testing/setup_and_teardown' is now loaded before Mocha, ActiveSupport::Testing::SetupAndTeardown is not included into ActiveSupport::TestCase until after Mocha is loaded, so ActiveSupport::Testing::SetupAndTeardown#run will replace the Mocha one. This gives the desired end result with the testing callbacks working correctly.

    So I now think this commit is fine. Sorry for the confusion.

  • James Mead

    James Mead July 2nd, 2009 @ 05:53 PM

    Michael :-

    I agree the double monkey-patch is unfortunate. I was wondering whether it might be possible to agree on the use of a shared extension to Test::Unit which allows both Mocha and Rails (and any other relevant libs) to hook in their respective bits of code. This shared extension would basically be a single monkey-patch of the Test::Unit::TestCase#run method, providing the relevant hooks. Both Mocha and Rails would attempt this exact monkey-patch (ideally only applying it if it's not already applied), before then actually hooking in the relevant bits of code.

    For me the killer blow of non-extensibility in Test::Unit is the call to "send(@method_name)" in the middle of TestCase#run. Interestingly in the latest Test::Unit gem, this line is extracted into a separate TestCase#run_test method, albeit a private method. But this makes it more amenable to extension. In the Test::Unit gem there is also a TestCase#handle_exception method which allows for registering multiple exception handlers. Again this could be a useful extension point.

    The other difficulty with this whole idea is that the order in which the hooks get executed would probably be important and that might require the shared Test::Unit extension to have some kind of "global" knowledge about which hooks should be called before which other hooks.

    Anyway, I'd be interested to hear what you think. I might try having a go at implementing it if I can find some spare time.

  • Michael Koziarski

    Michael Koziarski July 5th, 2009 @ 04:56 AM

    Jeremy's the guy who did all the work the last time we had to deal
    with test/unit, so he's probably the best one to comment on any
    unified approach.

    But yeah, fundamentally we both just want before and after hooks, so
    if we could agree on a tiny change to add those hooks, both of us
    could simply do something like conditionally add those hooks depending
    on

    Test::Unit::TestCase.respond_to?(:add_hook_or_something_whatever)

  • bshand

    bshand November 24th, 2009 @ 11:26 AM

    From the mocha rubyforge page: Note that versions 0.9.6 & 0.9.7 of the Rails plugin were broken. As of version 0.9.8, you need to explicitly load Mocha after the test framework e.g. by adding "require 'mocha'" at the bottom of test/test_helper.rb.

    They don't mention that if you use config.gem in config/environments/test.rb, you need to ensure that mocha is not loaded at that point, i.e.

    config.gem "mocha", :version => ">=0.9.8", :lib => false
    
  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:52 PM

    Automatic cleanup of spam.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages