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

Mocha/minitest issue in test_helper.rb #1149

Closed
Marinlemaignan opened this issue Apr 23, 2018 · 2 comments · Fixed by #1169
Closed

Mocha/minitest issue in test_helper.rb #1149

Marinlemaignan opened this issue Apr 23, 2018 · 2 comments · Fixed by #1169

Comments

@Marinlemaignan
Copy link
Contributor

Depending on which versions bundler would have installed it can happen to run into mocha#324
we would need to investigate that as to make this library as simple as possible

a quick fix would be

begin
  require 'mocha/minitest
rescue
  require 'mocha/mini_test'
end
@Evan-M
Copy link
Collaborator

Evan-M commented Apr 23, 2018

It is probably better to add min and max versions of mocha to the .gemspec file.
It looks like 1.3.0 only has mocha/mini_test.rb whereas 1.4.0 and 1.5.0 switch to the updated mocha/minitest.rb with a deprecation warning added to the older mocha/mini_test.rb.

If you really want to wrap it in a begin/rescue block, you should probably rescue the LoadError exception explicitly, and check that the exception was in fact from attempting to load the missing version of mocha/minitest:

begin
  require 'mocha/minitest'
rescue LoadError => e
  raise unless e.message =~ /minitest/
  require 'mocha/mini_test'
end

@Evan-M
Copy link
Collaborator

Evan-M commented Apr 30, 2018

I wanted to add here that the mocha dependency was added way back in #323 (albeit incorrectly). I reported the incorrect usage in #1137, and resolved it with #1139.

That said, as far as I can tell, the only place where mocha is actually used within the test suite is in devise_token_auth/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb (and hence why it was included in that test file only, as opposed to the whole test suite).

Instead of wrapping the require in a begin/rescue block (and attempting to cleanly support both the older and newer versions of mocha), I think a better approach here is to refactor the test in question and remove the dependency on mocha altogether.

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

Successfully merging a pull request may close this issue.

2 participants