Use require instead of load with test/unit/version #140

Merged
merged 1 commit into from Mar 4, 2013

Conversation

Projects
None yet
2 participants
@tmiller
Contributor

tmiller commented Feb 24, 2013

Other gems also require test/unit/version. When this gem loads the
same file it over writes the constant and causes a warning. If there is any way possible I would also like this back ported to the 0.12.X version.

Tom Miller
Use require instead of load with test/unit/version
  Other gems also require test/unit/version. When this gem loads the
same file it over writes the constant and causes a warning.

@ghost ghost assigned floehopper Feb 24, 2013

@floehopper

This comment has been minimized.

Show comment
Hide comment
@floehopper

floehopper Feb 24, 2013

Member

Thanks for taking the time to submit a pull request. The load statement was itself introduced to fix another problem. So I need to figure out whether that problem still exists after your changes. I'll try to take a look as soon as I can.

Member

floehopper commented Feb 24, 2013

Thanks for taking the time to submit a pull request. The load statement was itself introduced to fix another problem. So I need to figure out whether that problem still exists after your changes. I'll try to take a look as soon as I can.

@tmiller

This comment has been minimized.

Show comment
Hide comment
@tmiller

tmiller Feb 24, 2013

Contributor

I took a brief look at the problem. If it is caused by a standard library being loaded first you should require the gem like below before any gem that depends on it. It sounds like they might have still had issues with this solution though. Thanks for taking the time looking into it!

require 'rubygems'  # make sure we have gem method
gem 'test-unit'     # make sure to load the gem version
require 'test/unit' # safe to require test/unit or let the dependent gem load it
require 'mocha'     # dependent gem
Contributor

tmiller commented Feb 24, 2013

I took a brief look at the problem. If it is caused by a standard library being loaded first you should require the gem like below before any gem that depends on it. It sounds like they might have still had issues with this solution though. Thanks for taking the time looking into it!

require 'rubygems'  # make sure we have gem method
gem 'test-unit'     # make sure to load the gem version
require 'test/unit' # safe to require test/unit or let the dependent gem load it
require 'mocha'     # dependent gem
@floehopper

This comment has been minimized.

Show comment
Hide comment
@floehopper

floehopper Mar 4, 2013

Member

Your suggestion makes sense to me. I tried to reproduce the original problem with more recent versions of test-unit and mocha gems, but I ran into a different exception:

~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit/adapter.rb:27:in `included': undefined method `setup' for Test::Unit::TestCase:Class (NoMethodError)
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `include'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `send'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `to_proc'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration.rb:8:in `map'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration.rb:8:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/setup.rb:6:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/setup.rb:10
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:60:in `gem_original_require'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:60:in `require'
  from foo.rb:8

However, this was fixed by using the gem "test-unit" statement that you suggested.

So I'm going to go ahead and merge your pull request. Thanks.

Member

floehopper commented Mar 4, 2013

Your suggestion makes sense to me. I tried to reproduce the original problem with more recent versions of test-unit and mocha gems, but I ran into a different exception:

~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit/adapter.rb:27:in `included': undefined method `setup' for Test::Unit::TestCase:Class (NoMethodError)
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `include'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `send'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration/test_unit.rb:43:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `to_proc'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration.rb:8:in `map'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/integration.rb:8:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/setup.rb:6:in `activate'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/gems/1.8/gems/mocha-0.13.2/lib/mocha/setup.rb:10
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:60:in `gem_original_require'
  from ~/.rbenv/versions/1.8.7-p352/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:60:in `require'
  from foo.rb:8

However, this was fixed by using the gem "test-unit" statement that you suggested.

So I'm going to go ahead and merge your pull request. Thanks.

floehopper added a commit that referenced this pull request Mar 4, 2013

Merge pull request #140 from tmiller/master
Use `require` instead of `load` with `test/unit/version`.

Other gems also require `test/unit/version`. When mocha loads the
same file it over writes the constant and causes a warning.

I tried to reproduce the original problem [1] with more recent
versions of the test-unit and mocha gems, but I ran into a
different exception. This exception was fixed by adding a
`gem "test-unit"` statement before the `require "test/unit"`
statement.

[1] http://floehopper.lighthouseapp.com/projects/22289-mocha/tickets/50#ticket-50-13

@floehopper floehopper merged commit e623a26 into freerange:master Mar 4, 2013

1 check passed

default The Travis build passed
Details
@floehopper

This comment has been minimized.

Show comment
Hide comment
@floehopper

floehopper Mar 4, 2013

Member

I've updated the original Lighthouse ticket with a note about this.

Member

floehopper commented Mar 4, 2013

I've updated the original Lighthouse ticket with a note about this.

@floehopper

This comment has been minimized.

Show comment
Hide comment
@floehopper

floehopper Mar 7, 2013

Member

This has now been released in Mocha 0.13.3. It would be great if you could check that it solves your problem.

Member

floehopper commented Mar 7, 2013

This has now been released in Mocha 0.13.3. It would be great if you could check that it solves your problem.

@tmiller

This comment has been minimized.

Show comment
Hide comment
@tmiller

tmiller Mar 7, 2013

Contributor

It appears to have removed the warning thanks!

Contributor

tmiller commented Mar 7, 2013

It appears to have removed the warning thanks!

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