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.

Sign up for GitHub

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

Jump to bottom

Trying to fix mocha upgrade failures #5907

Closed
wants to merge 1 commit into from
Closed

Trying to fix mocha upgrade failures #5907

wants to merge 1 commit into from

Conversation

arunagw
Copy link
Member

@arunagw arunagw commented Apr 20, 2012

So Mocha got an update to new version and our actionpack started failing after that

 1) Error:
test_parameters(RequestTest):
ArgumentError: wrong number of arguments (1 for 0)
    /Users/arunagw/checkouts/rails/actionpack/lib/action_dispatch/http/request.rb:79:in `method'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/class_method.rb:39:in `hide_original_method'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/class_method.rb:15:in `stub'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/central.rb:13:in `stub'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/object.rb:121:in `block in stubs'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/argument_iterator.rb:15:in `call'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/argument_iterator.rb:15:in `each'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/object.rb:117:in `stubs'
    /Users/arunagw/checkouts/rails/actionpack/test/dispatch/request_test.rb:438:in `block in <class:RequestTest>'
    /Users/arunagw/checkouts/rails/bundle/ruby/1.9.1/gems/mocha-0.11.0/lib/mocha/integration/mini_test/version_230_to_262.rb:28:in `run'

3507 tests, 15952 assertions, 0 failures, 1 errors, 0 skips

So after research i found that mocha internally doing some stuff with 

 method

We have one function 

method
in our request model which is not having any argument.

This is why we are getting above error that mocha is giving 1 argument for stubbee

here you can found that call https://github.com/floehopper/mocha/compare/24314c2e77...master#L68R39

I tried to fix this in test itself as we don't want to change much things.

Please let me know your suggestions. I will make changes according to that.

@josevalim
Copy link
Contributor

/cc @tenderlove

@rafaelfranca rafaelfranca mentioned this pull request Apr 23, 2012
Closed
@jeremy
Copy link
Member

jeremy commented Apr 23, 2012

This is a reasonable workaround for now, but this should probably be resolved with Mocha.

It needs to alias Object#method to something like #mocha_method and use that exclusively, leaving #method itself available for override.

@sikachu
Copy link
Member

sikachu commented Apr 23, 2012

Interesting idea. I'm going to see if that's possible.

@tenderlove
Copy link
Member

I'll take a look.

On Monday, April 23, 2012, José Valim wrote:

/cc @tenderlove

Reply to this email directly or view it on GitHub:
#5907 (comment)

Aaron Patterson
http://tenderlovemaking.com/

@sikachu
Copy link
Member

sikachu commented Apr 23, 2012

I already send a pull request in mocha. https://github.com/floehopper/mocha/pull/77

@tenderlove
Copy link
Member

@jeremy you're right. Can we just not upgrade mocha until this is fixed upstream?

@jeremy
Copy link
Member

jeremy commented Apr 24, 2012

By all means :) This is in our own Gemfile after all. Really loose requirement.

@sikachu
Copy link
Member

sikachu commented Apr 24, 2012

Yep, letz lock it! :D

Sent from my iPhone

On Apr 23, 2012, at 7:45 PM, Jeremy Kemperreply@reply.github.com wrote:

By all means :) This is in our own Gemfile after all. Really loose requirement.

Reply to this email directly or view it on GitHub:
#5907 (comment)

@arunagw
Copy link
Member Author

arunagw commented Apr 24, 2012

Yes I think lets lock it for now. and later can be upgraded once mocha upgrade.

I will do a PR.

arunagw added a commit to arunagw/rails that referenced this pull request Apr 24, 2012
@arunagw
Locking mocha to older version as waiting to fix
0770dbf 
from mocha.

See rails#5907 and 
https://github.com/floehopper/mocha/pull/77
@arunagw arunagw mentioned this pull request Apr 24, 2012
Closed
@arunagw arunagw mentioned this pull request Apr 24, 2012
Closed
@arunagw
Copy link
Member Author

arunagw commented Apr 24, 2012

Should i close this or keep it open until mocha gets an update?

Sent to PR for locking mocha.

#5951 #5952

Not doing for 3-1-stable right now. Will do when required.

Cheers,
Arun

arunagw added a commit to arunagw/rails that referenced this pull request Apr 24, 2012
@arunagw
Locking mocha to older version as waiting to fix
325e4c7 
from mocha.

See rails#5907 and
https://github.com/floehopper/mocha/pull/77
@floehopper
Copy link
Contributor

While I think the suggested solution (i.e. the change to Mocha) is the best pragmatic option, philosophically I think it implies that Mocha should not rely on any other core Ruby methods in classes like Class, Object or Kernel and therefore ought to alias all of these as well which seems a bit crazy.

It's not impossible that other libraries or applications may also expect Object#method to have it's original implementation and may be surprised by Rails' redefinition. Perhaps a better longer term solution is for Ruby itself to alias Object#method as Object#__method__ like it does for Object#send, given that #method is so generic it's very likely to be used in other situations.

Anyway, I'll attend to the Mocha pull request and release a new version.

@floehopper
Copy link
Contributor

Mocha v0.11.2 is now available including a slightly tweaked version of the pull request. Hopefully this will do the trick.

@arunagw
Copy link
Member Author

arunagw commented Apr 24, 2012

Closing this as 0.11.2 is out. thanks guys :-)

@arunagw arunagw closed this Apr 24, 2012
jperkin pushed a commit to TritonDataCenter/pkgsrc-legacy that referenced this pull request Dec 9, 2013
Update ruby-mocha to 0.11.3.
e29c934 
= 0.11.3
* Fix for #78 i.e. alias Object#method as Object#_method, not Object#__method__ which already exists as another Ruby method.

= 0.11.2
* Rails has a Request class which defines its own #method method. This broke the new mechanism for stubbing a method. This release includes a slightly modified version of fix #77 provided by @sikachu. See rails/rails#5907 for further info.

= 0.11.1 ()
* In Ruby 1.8.7 methods accepting a block parameter were incorrectly restored without the block parameter after being stubbed. Fix for #76.

= 0.11.0 (fa601c89a7f5314dc3d258391a99c6a9e25cefb3)
* Store original method when stubbing rather than using alias_method. This fixes #41, #47, #74 and all tests now pass on both Ruby 1.8.7 and 1.9.3.
* Attempting to stub a method on a frozen object should fail fast. See #68.
* Prevent stubbing a method on nil by default. See #68.
* Generate documentation using YARD instead of Rdoc - removes dependency on Coderay.
* Publish documentation on Github pages instead of Rubyforge - uses rake task written by @tomafro.
* Remove agiledox which has outlived it's usefulness.
* Removed trailing whitespace throughout codebase.
* Add documentation for Mock#unstub.
* Improve documentation for ObjectMethods.
* Provide a way to run multiple tests within a single acceptance test method.
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Oct 11, 2014
Update ruby-mocha to 0.11.3.
4fae80a 
= 0.11.3
* Fix for #78 i.e. alias Object#method as Object#_method, not Object#__method__ which already exists as another Ruby method.

= 0.11.2
* Rails has a Request class which defines its own #method method. This broke the new mechanism for stubbing a method. This release includes a slightly modified version of fix #77 provided by @sikachu. See rails/rails#5907 for further info.

= 0.11.1 ()
* In Ruby 1.8.7 methods accepting a block parameter were incorrectly restored without the block parameter after being stubbed. Fix for #76.

= 0.11.0 (fa601c89a7f5314dc3d258391a99c6a9e25cefb3)
* Store original method when stubbing rather than using alias_method. This fixes #41, #47, #74 and all tests now pass on both Ruby 1.8.7 and 1.9.3.
* Attempting to stub a method on a frozen object should fail fast. See #68.
* Prevent stubbing a method on nil by default. See #68.
* Generate documentation using YARD instead of Rdoc - removes dependency on Coderay.
* Publish documentation on Github pages instead of Rubyforge - uses rake task written by @tomafro.
* Remove agiledox which has outlived it's usefulness.
* Removed trailing whitespace throughout codebase.
* Add documentation for Mock#unstub.
* Improve documentation for ObjectMethods.
* Provide a way to run multiple tests within a single acceptance test method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@arunagw @josevalim @jeremy @sikachu @tenderlove @floehopper