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

Breaking tests / travis builds - Mocha gem was updated. #1137

Closed
Evan-M opened this issue Apr 6, 2018 · 2 comments
Closed

Breaking tests / travis builds - Mocha gem was updated. #1137

Evan-M opened this issue Apr 6, 2018 · 2 comments

Comments

@Evan-M
Copy link
Collaborator

Evan-M commented Apr 6, 2018

This issue seems to have appeared right around the same time I merged #1134 into master. It looks like it's happening on other builds as well though.

A recent update to the Mocha gem (release 1.5.0) prevents the use of Mocha outside of test/examples.

This seems to break all of the Travis CI builds, since they try to bundle the latest release.

The simplest solution is probably to specify ~> 1.4.0 in the Gemfile.

That said, it seems like the only place Mocha is used in this project is in /test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb. Perhaps the better long term approach is to refactor that test to either: (A) not stub method calls in the before blocks, (B) stub the methods with Minitest instead (thereby removing the dependency on Mocha), or (C) replace the stubbed responses with real data and not stub any methods at all (thereby removing the dependency on Mocha).

I should probably add, the test in question inherits from ActionDispatch::IntegrationTest, so it is probably a bad idea to be stubbing method calls here.

@Evan-M Evan-M mentioned this issue Apr 6, 2018
@floehopper
Copy link

I think the problem is actually that you are not initializing Mocha correctly. For quite a long time now, Rails has been using Minitest not Test::Unit, and so you should be requiring "mocha/minitest" on this line, not "mocha/test_unit" as per the instructions in the Mocha README. These instruction have been unchanged since Mocha v1.0.0.

However, making that change then starts verifying your Mocha expectations and you end up with a different test failure:

$ BUNDLE_GEMFILE=$PWD/gemfiles/rails_5_1.gemfile bundle exec ruby -I test test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb 
Started with run options --seed 23387

 FAIL["test_sign_in_was_called", "success callback", 2.9544389999937266]
 test_sign_in_was_called#success callback (2.95s)
        not all expectations were satisfied
        unsatisfied expectations:
        - expected exactly once, not yet invoked: #<AnyInstance:User>.sign_in(any_parameters)
        test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb:73:in `block (2 levels) in <class:OmniauthTest>'

  28/28: [=====================================================================================================================================] 100% Time: 00:00:03, Time: 00:00:03

Finished in 3.44703s
28 tests, 53 assertions, 1 failures, 0 errors, 0 skips

This hasn't been failing until now, because Mocha wasn't being initialized correctly and thus no expectations were being verified. I'll leave you to work out why this expectation is not being satisfied.

I hope this helps.

@Evan-M
Copy link
Collaborator Author

Evan-M commented Apr 6, 2018

Thanks for looking into this @floehopper! I'll try updating the setup for Mocha.

UPDATE:
It seems like swapping out require 'mocha/test_unit' with require 'mocha/minitest' is indeed the solution here. Of course, as mentioned, this leads to a different test failure.

However, it appears that this new failure should in fact happen! The test in question sets an expectation that any instance of the User model should receive the #sign_in method. The actual implementation instead calls the Devise::Controllers::SignInOut#sign_in method, passing the user instance as an argument.

Evan-M added a commit to tmjfitch/devise_token_auth that referenced this issue Apr 9, 2018
 - Update file to require correct Mocha lib for MiniTest (instead of
   Test::Unit). Using the wrong lib previously, meant that Mocha was
   never verifying the stubbed method, and the test passed
   (erroneously).

 - Update test to expect the correct method call for sign_in.

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

No branches or pull requests

2 participants