Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Superclass singleton method stub is not properly reset #99

Closed
alan opened this issue Aug 7, 2012 · 20 comments
Closed

Superclass singleton method stub is not properly reset #99

alan opened this issue Aug 7, 2012 · 20 comments
Assignees

Comments

@alan
Copy link

alan commented Aug 7, 2012

I found a problem that I think is related to Mocha when used with Rails.

The problem is that class methods seems that have had an expectation are left in a 'bad' state for subsequent tests.

I was able to reproduce the bug only with an empty rails project (v 3.2.7) and mocha (v 0.12.3). I also tried on a new empty project using only mini test + mocha as the only dependencies and could not reproduce the bug.

These is the spec file that shows the bug:

require File.expand_path(File.join(File.dirname(__FILE__), "test_helper"))
require 'mocha'

class MochaTest < ActiveSupport::TestCase
  class A
    class <<self
      def my_class_method
        method_on_subclass
      end
    end
  end

  class B < A
    class <<self
      def method_on_subclass
        :my_value
      end
    end
  end

  test "one" do
    A.expects(:my_class_method).never
  end

  test "two" do
    assert_equal :my_value, B.my_class_method
  end
end

Test 'two' needs to run after test 'one' and both should pass, but the test 'two' doesn't because an error is raised.

When 'second' test runs after 'first' test it throws a NoMethodError saying that method_on_subclass is not defined. The problem seems to be that when B.my_class_method is executed in the second test self is class A when it should be class B. Removing the expectation of 'first' test self is class B, and the method works as expected.

Let me know if you need any more details.

PS I am using rvm with a clean gemset, and with gem 'mocha', require: false in the Gemfile.

@alan
Copy link
Author

alan commented Aug 7, 2012

Ruby version is 1.9.3

@alan
Copy link
Author

alan commented Aug 7, 2012

And the test run output:

# Running tests:

.E

Finished tests in 0.172061s, 11.6238 tests/s, 5.8119 assertions/s.

  1) Error:
test_two(MochaTest):
NameError: undefined local variable or method `method_on_subclass' for MochaTest::A:Class
    test/mocha_test.rb:8:in `my_class_method'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/class_method.rb:71:in `call'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/class_method.rb:71:in `block in restore_original_method'
    test/mocha_test.rb:26:in `block in <class:MochaTest>'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/integration/mini_test/version_230_to_2101.rb:28:in `run'

2 tests, 1 assertions, 0 failures, 1 errors, 0 skips

@ghost ghost assigned floehopper Aug 7, 2012
@floehopper
Copy link
Member

Are you just using the version of MiniTest in the Ruby 1.9.3 standard library or are you using it as a gem? If the latter, what version are you using?

@alan
Copy link
Author

alan commented Aug 7, 2012

Just the one that comes with the standard library.

The rails project is barebones, with only mocha in the Gemfile set to require false.

If it helps, this issue also happens when using mocha through rspec-rails.

@floehopper
Copy link
Member

Thanks. If you still have your empty Rails project, could you try adding the latest minitest (v3.3.0, I think) to your Gemfile and run the test again?

@alan
Copy link
Author

alan commented Aug 7, 2012

It's still failing. Given that happens with both minitest and rspec-rails, could it be something that Rails does? In another barebones project with only minitest (from standard library) and mocha there is no issue.

@floehopper
Copy link
Member

Thanks. Can you supply the stack trace for the failure with the latest MiniTest?

I'll have another go at trying to reproduce it here. I couldn't reproduce it earlier, but I took a bit of a shortcut, so I'll have a proper go this time.

The problem is quite likely to be due to the way ActiveSupport integrates with the test frameworks. Unfortunately there's a lot of monkey-patching going on in both Mocha and ActiveSupport which causes headaches all round.

I've done some work recently in conjunction with @tenderlove & @zenspider to make Mocha work with a ActiveSupport and MiniTest without any monkey-patching, but unfortunately I haven't had time to finish it off and release it.

@alan
Copy link
Author

alan commented Aug 8, 2012

This is the stack trace with 'minitest' in the Gemfile:

# Running tests:

.E

Finished tests in 0.182637s, 10.9507 tests/s, 5.4753 assertions/s.

  1) Error:
test_two(MochaTest):
NameError: undefined local variable or method `method_on_subclass' for MochaTest::A:Class
    test/mocha_test.rb:8:in `my_class_method'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/class_method.rb:71:in `call'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/class_method.rb:71:in `block in restore_original_method'
    test/mocha_test.rb:26:in `block in <class:MochaTest>'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/mocha-0.12.3/lib/mocha/integration/mini_test/version_330.rb:32:in `run'
    /Users/alan/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/1.9.1/test/unit/testcase.rb:17:in `run'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/testing/setup_and_teardown.rb:36:in `block in run'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/callbacks.rb:425:in `_run__2422877623144156460__setup__3014898139906642276__callbacks'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/callbacks.rb:405:in `__run_callback'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/callbacks.rb:385:in `_run_setup_callbacks'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/alan/.rvm/gems/ruby-1.9.3-p125@railsmocha/gems/activesupport-3.2.7/lib/active_support/testing/setup_and_teardown.rb:35:in `run'

The stack trace changes, however I can't minitest in the stack trace.

@floehopper
Copy link
Member

Thanks. I've now managed to reproduce the failure locally. I'll continue to investigate.

@floehopper
Copy link
Member

I think I've figured it out. It isn't anything to do with Rails or ActiveSupport. Mocha::ClassMethod#restore_original_method was changed in 379c9cd to use a closure to implement the restored method, but it doesn't have the correct binding and so it doesn't know about your B.method_on_subclass method. Ironically I think this problem is fixed in Ruby 1.9 by a recent (but unreleased) commit - 8d37bbd.

I'll have a ponder about the best solution. Thanks for reporting it so thoroughly.

@floehopper
Copy link
Member

Could you verify that it is fixed by pointing your Gemfile at the HEAD of master i.e. gem "mocha", git: "git://github.com/freerange/mocha.git" and re-running the test?

@alan
Copy link
Author

alan commented Aug 8, 2012

I'll give it a go now and let you know. Strange that is nothing to do with Rails, I couldn't reproduce without Rails when trying to narrow down the problem.

@alan
Copy link
Author

alan commented Aug 8, 2012

Yes, the problem is fixed now!

@alan
Copy link
Author

alan commented Aug 8, 2012

Specs are also passing on the rails app I originally found the problem.

@floehopper
Copy link
Member

Thanks for letting us know. It looks like the problem may still be occurring in Ruby 1.8 (see #95), so we may hold off releasing this change until we have a more comprehensive solution.

@boutil
Copy link

boutil commented Oct 15, 2012

Hi!

A problem with the tests of ruby-net-sftp has been reported in Debian:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=674324

Reading this issue made me think it could be related to that problem, although what is used is test/unit, and not activesupport.

Debian testing has at the moment version 0.11.3 of mocha, but I have been able to reproduce the issue consistently with several versions/snapshots starting from commit 995bf43 (ClassMethod & InstanceMethod no longer use alias_method).

A minimal example presenting the problem is the following:

require "test/unit"
require "mocha"


class A
  class <<self
    def foo
      new
    end
  end
end

class B < A
end

class C
  def from_A
    A.foo
  end
end


class ATest < Test::Unit::TestCase
  def setup
    @base = C.new
  end

  def test_using_expects
    A.expects(:foo).returns(:output)
    assert_equal @base.from_A, :output
  end
end

class BTest < Test::Unit::TestCase
  def test_using_foo
    assert_instance_of B, B.foo
  end
end

After the test in class ATest, the behavior of the foo method is altered for the B class. Instead of returning a B object, it returns an object of class A, causing the test in BTest to fail.
However, if class ATest is commented out, the test in BTest passes.

Applying 8d37bbd makes the problem disappear with Ruby 1.9 but the problem still occurs with Ruby1.8.

Do you confirm it is the same issue?

@floehopper
Copy link
Member

I'm sorry that I don't have time to investigate this in detail now, but given that your example relates to a singleton method on a subclass and given that 8d37bbd fixes it in Ruby 1.9, I'd be pretty confident that it is the same issue.

In order to fix this, it looks like we'll need to re-introduce the old mechanism of aliasing methods (as mentioned in #95 (comment)) in order to hide them and then restore them - at least for Ruby versions < 1.9.

Realistically, I doubt I'm going to be able to do this any time soon. I would, however, welcome a pull request to fix the problem assuming it came along with acceptance tests which demonstrated the problem.

@floehopper
Copy link
Member

I hope you don't mind, but I'm going to edit the title of this issue to more accurately reflect the problem described within it.

@floehopper
Copy link
Member

#162 was closed as a duplicate of this issue.

@floehopper
Copy link
Member

Given that this problem was fixed for Ruby > v1.9, I'm not planning on fixing it for earlier versions unless someone supplies a patch. Closing.

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

No branches or pull requests

3 participants