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

rails console should not require minitest/unit #6907

Closed
mlartz opened this issue Jun 29, 2012 · 16 comments · Fixed by #7349
Closed

rails console should not require minitest/unit #6907

mlartz opened this issue Jun 29, 2012 · 16 comments · Fixed by #7349
Labels

Comments

@mlartz
Copy link

mlartz commented Jun 29, 2012

rails/railties/lib/rails/console/app.rb, lines 2 and 6 require and use Test::Unit, respectively. This prevents me from running rails console in production (fails requiring 'minitest/unit'), which is useful for debugging our live deployment databases. Is there a problem with either removing this dependency or checking for RAILS_ENV before requiring/using Test::Unit?

require 'active_support/all'
require 'active_support/test_case'
require 'action_controller'

# work around the at_exit hook in test/unit, which kills IRB
Test::Unit.run = true if Test::Unit.respond_to?(:run=)
@rafaelfranca
Copy link
Member

I didn't get. Is Rails console requiring minitest/unit in 3.2.x? Or are you using master?

@steveklabnik
Copy link
Member

Yeah, minitest is included with Ruby 1.9, so require 'minitest/unit' shouldn't ever fail. If you're on master, then 1.8 isn't supported, so this isn't a bug. If you're on 3.2, then we support 1.8, so this would be a bug.

@mlartz
Copy link
Author

mlartz commented Jul 1, 2012

I'm on Rails 3.2.2 using Ruby 1.9.3, but I'm trying to use the new Fedora 17 Ruby RPM spec files (thanks @voxik, @bkabrda), which extract the stdlib "gems" (i.e. the "gems" included with Ruby such as minitest, json, and a bunch of others) from the base install, treating them similar to real gems. Unfortunately, because they are bundled with Ruby, there are some assumptions about their availability as part of the stdlib, even though they are treated as some odd hybird of stdlib and gems. See the following for more explanation:

http://bugs.ruby-lang.org/issues/6123
http://bugs.ruby-lang.org/issues/6124
http://bugs.ruby-lang.org/issues/5481

So it is partially a Ruby problem that looks like its on track to be partially fixed in 1.9.3 and correctly fixed in 2.

But my Rails question remains ... why is 'rails console' dependent on a test framework? Can it be removed?

@steveklabnik
Copy link
Member

which extract the stdlib "gems" from the base install, treating them similar to real gems.

And people wonder why nobody uses the system package managers when installing Ruby...

Can it be removed?

Still a totally valid question. I bet the answer is "because it's always available so why not".

The test_case line was added by @jeremy here.

@voxik
Copy link
Contributor

voxik commented Jul 1, 2012

@steveklabnik Sorry, but that is not very constructive answer :/ StdLib contains plenty of other libraries which are not required. I hope you are not going to require all the libraries to solve this issue, since they are also always available.

BTW, there is available never minitest version at rubygems.org, have you tried to used them in your project? Are you sure that the correct (newest) version of minitest is always used for your testing?

@steveklabnik
Copy link
Member

@voxlk what I mean is that, as a Rubyist, I expect the full standard library to be available for any given copy of Ruby. A Ruby 1.9 without minitest is just broken.

correct (newest)

The newest isn't correct unless I explicitly require it.

@voxik
Copy link
Contributor

voxik commented Jul 2, 2012

@steveklabnik the question was "why is 'rails console' dependent on a test framework? Can it be removed?" and your answer was "because it's always available so why not". The question and answer are not aligned IMO. The question was definitely not if Ruby without minitest is broken or not. There are different threads to discuss this topic an I can assure you that I will persuade the correct solution.

@steveklabnik
Copy link
Member

Oh. I'm not saying that it's right, I'm just speculating. I didn't introduce this behavior. Sorry, I thought you were talking about the assumption that it'd be available.

What I was trying to say was "I have no idea, but it looks like @jeremy may have been the one that did it, let's see what he says."

@voxik
Copy link
Contributor

voxik commented Aug 14, 2012

ping ... any news about this matter?

@rafaelfranca
Copy link
Member

I'm not sure if this was the reason to add the test framework in the rails console but for me it make sense. With that line you can you assertion and some test helpers in your console.

@robin850
Copy link
Member

Maybe I can reopen my pull request and put in the Gemfile template (temporary ; till a solution will be found) a condition with a Regexp on the RUBY_PLATFORM constant ; if it contains "linux", then add an entry to Gemfile containing the minitest gem ?

@steveklabnik
Copy link
Member

Putting a gem in the gemfile is far, far more drastic than removing those lines or including them only if Minitest exists.

@steveklabnik
Copy link
Member

a condition with a Regexp on the RUBY_PLATFORM constant ; if it contains "linux", then add an entry to Gemfile containing the minitest gem ?

Not at all, only Fedora is stupid....errr, 'unique' enough to do this. (I shouldn't speak so quickly, Debian probably does too. rimshot)

Do something like this:

if defined?(MiniTest)
...

@robin850
Copy link
Member

@steveklabnik : Does I reopen my pull request adding this ?

@steveklabnik
Copy link
Member

@robin850 I just did it.

@rafaelfranca
Copy link
Member

We decided to not defensive programming in this case. The require is needed because Test::Unit kills IRB.

If you are using a broken Ruby, you'll need to add the missing gems in your Gemfile,

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

Successfully merging a pull request may close this issue.

5 participants