Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Delay the loading of DRB. #629

Merged
merged 1 commit into from Jun 11, 2012

Conversation

Projects
None yet
3 participants
Owner

myronmarston commented Jun 6, 2012

@dchelimsky -- can you code review this before I merge it in? I've never used rspec's DRB option before (and couldn't figure out how to get it to work in few minutes of futzing with it).

@myronmarston myronmarston Delay the loading of DRB.
- Most of the time users don't use DRB. This'll speed up the start time a bit
  by not unnecessarily loading it when it's not used.
- Apparently jruby 1.7.0-preview1 loads the delegate library when DRB is loaded [1][2].
  This causes expectations on delegate objects to fail because `should` never gets added
  to them. By delaying when delegate gets loaded, it allows a user to load
  `rspec/expectations` and then `delegate` and ensure it works.

[1] rspec/rspec-expectations#148 (comment)
[2] The backtrace at the point delegate is loaded:
/Users/myron/.rvm/rubies/jruby-head/lib/ruby/1.9/drb/drb.rb:1:in `require'
/Users/myron/.rvm/rubies/jruby-head/lib/ruby/1.9/drb/drb.rb:1:in `(root)'
/Users/myron/.rvm/rubies/jruby-head/lib/ruby/1.9/drb/drb.rb:57:in `require'
/Users/myron/.rvm/rubies/jruby-head/lib/ruby/1.9/drb/drb.rb:57:in `(root)'
/Users/myron/.rvm/gems/jruby-head/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:1:in `require'
/Users/myron/.rvm/gems/jruby-head/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:1:in `(root)'
/Users/myron/.rvm/gems/jruby-head/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:1:in `(root)'
file:/Users/myron/.rvm/rubies/jruby-head/lib/jruby.jar!/jruby/kernel19/kernel.rb:1:in `(root)'
file:/Users/myron/.rvm/rubies/jruby-head/lib/jruby.jar!/jruby/kernel19/kernel.rb:19:in `require'
file:/Users/myron/.rvm/rubies/jruby-head/lib/jruby.jar!/jruby/kernel19/kernel.rb:19:in `require_relative'
/Users/myron/.rvm/gems/jruby-head/gems/rspec-core-2.10.1/lib/rspec/core.rb:4:in `require'
/Users/myron/.rvm/gems/jruby-head/gems/rspec-core-2.10.1/lib/rspec/core.rb:4:in `require_rspec'
/Users/myron/.rvm/gems/jruby-head/bin/rspec:23:in `load'
/Users/myron/.rvm/gems/jruby-head/bin/rspec:23:in `(root)'
3646611

This pull request passes (merged 3646611 into 4feeec6).

Owner

dchelimsky commented Jun 9, 2012

@myronmarston no, we don't have any automated scenarios for this, but I checked it manually and it works fine. Curious as to what led you to this?

@myronmarston myronmarston added a commit that referenced this pull request Jun 11, 2012

@myronmarston myronmarston Merge pull request #629 from rspec/delay-drb-loading
Delay the loading of DRB.
9a79082

@myronmarston myronmarston merged commit 9a79082 into master Jun 11, 2012

Owner

myronmarston commented Jun 11, 2012

@myronmarston no, we don't have any automated scenarios for this, but I checked it manually and it works fine. Curious as to what led you to this?

I put a fair bit of detail in the commit message (did you see that?), but to recap: @jeremyevans noticed an rspec regression in his sequel specs: before rspec 2.10, expectations on delegate objects worked. The new expect syntax is the official fix moving forward, but he's kept his test suite passing against rspec 1.3+, and wanted a work-around. The work around I came up with is to manually require rspec/expectations before the delegate stdlib gets loaded (which ensures that should is defined on Kernel before delegate selectively includes methods defined on kernel). The work around worked fine except on jruby 1.7.0-preview1. It loads delegate as part of its DRB implementation, and thus, loading rspec-core on that version loads delegate before users have a chance to load rspec-expectations. Delaying the loading of DRB fixes this, and also should make the loading of rspec a tiny bit faster for the 99% of users who don't use DRB when running rspec.

In general, I think we should favor lazily loading optional dependencies like DRB, as a means to reduce bloat and improve startup time.

Owner

dchelimsky commented Jun 11, 2012

@myronmarston I hadn't seen the commit message since github doesn't display it in the diff for the pull. Makes perfect sense all around, including lazy-loading DRB.

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