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

visit just disappears when using gem. #24

Closed
envygeeks opened this issue Aug 2, 2012 · 28 comments
Closed

visit just disappears when using gem. #24

envygeeks opened this issue Aug 2, 2012 · 28 comments

Comments

@envygeeks
Copy link

I've not time to figure out what is going on but when I use your gem, visit (or actually any normal helper method) just up and takes out. Then when I remove your gem from my rails Gemfile it comes back. I'm using RSpec 2 if that helps any.

@mattheworiordan
Copy link
Owner

What do you mean "just up and takes out"?

@envygeeks
Copy link
Author

I mean none of the helper methods are there, at all. None. Well they are there if I go the hard route. The only reason I figured this out was because the second I removed your gem from my Gemfile all the helper methods came back.

@kirel
Copy link

kirel commented Aug 6, 2012

Same problem with rspec-2.10.0, capybara-webkit-0.12.1, capybara-1.1.2

@phillipoertel
Copy link

Same Problem here :-(
Using RSpec 2.10.0, poltergeist 0.6.0 (with PhantomJS 1.5.0), Capybara 1.1.2

@mattheworiordan
Copy link
Owner

Ok, onto this issue, will look at it tomorrow when I have a spare moment.

@phillipoertel
Copy link

Awesome, thanks. If you need help debugging/testing please let me know.

@adzap
Copy link
Contributor

adzap commented Aug 7, 2012

@envygeeks What you mean by "they are there if I go the hard route"?

I can't seem to reproduce the problem.

So I gather this is in a normal request spec?

Can you output the backtrace with the rspec -b switch.

@envygeeks
Copy link
Author

@adzap you guys are removing my Capybara helpers somehow, as well I find it intriguing the way you guys handle minitest, some people have both minitest and rspec so you could very well end up creating conflicts. https://github.com/mattheworiordan/capybara-screenshot/blob/master/lib/capybara-screenshot.rb#L108 << Bad IMO.

"GET /auth/github"
From: STRIPPED @ line 24 :

    19: end
    20: 
    21: describe '"GET /auth/github"' do
    22: 
    23:   it 'should allow users to create an account' do
 => 24:     binding.pry
    29:     visit '/'

[1] pry(#<RSpec::Core::ExampleGroup::Nested_3>)> visit
#=> NameError: undefined local variable or method `visit'
from gems/rspec-expectations-2.11.2/lib/rspec/matchers/method_missing.rb:9:in `method_missing'
[2] pry(#<RSpec::Core::ExampleGroup::Nested_3>)> Capybara.visit
ArgumentError: wrong number of arguments (0 for 1)
from gems/capybara-1.1.2/lib/capybara/session.rb:156:in `visit'

As you can see above, I can access it via Capybara.visit but not visit, when your gem is gone, the capybara helper include works again. I honestly don't know what you are guys are doing since I've not really looked over the source apart from the one file I found the intriguing way that minitest is handled.

@adzap
Copy link
Contributor

adzap commented Aug 8, 2012

Are you using minitest and RSpec? And why would people use both?

I haven't paid any attention to minitest, any suggestions for improvement would be welcome.

@apotonick
Copy link

Here is the stacktrace for RSpec, no minitest involved.

NoMethodError:
       undefined method `visit' for #<RSpec::Core::ExampleGroup::Nested_1:0x007fd11f1ab448>
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-expectations-2.10.0/lib/rspec/matchers/method_missing.rb:9:in `method_missing'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.6/lib/action_dispatch/testing/assertions/routing.rb:176:in `method_missing'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.6/lib/action_dispatch/testing/integration.rb:387:in `method_missing'
     # ./spec/requests/admin/index_spec.rb:6:in `block (2 levels) in <top (required)>'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example.rb:87:in `instance_eval'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example.rb:87:in `block in run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example.rb:195:in `with_around_each_hooks'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example.rb:84:in `run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example_group.rb:353:in `block in run_examples'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example_group.rb:349:in `map'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example_group.rb:349:in `run_examples'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/example_group.rb:335:in `run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/command_line.rb:28:in `block (2 levels) in run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/command_line.rb:28:in `map'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/command_line.rb:28:in `block in run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/reporter.rb:34:in `report'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/command_line.rb:25:in `run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:66:in `rescue in run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:62:in `run'
     # /Users/nicksutterer/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.10.1/lib/rspec/core/runner.rb:10:in `block in autorun'

@envygeeks
Copy link
Author

@adzap I do not use Minitest and RSpec both in the same project, however I do have both Minitest and RSpec installed on my system because some people I work with prefer Minitest even though I prefer RSpec personally.

@adzap
Copy link
Contributor

adzap commented Aug 8, 2012

Sure, but I don't think it's causing a problem. If you are using bundler and only list rspec, then minitest won't be found and the loaderror is raised and caught.

If you do have both loaded, then you may won't both screen shot extensions loaded. A double require is unlikely, since require with the same path will not load the file again.

@mattheworiordan
Copy link
Owner

@kirel, @envygeeks and @phillipoertel, I have created a branch on my test application at https://github.com/mattheworiordan/capybara-screenshot-test-rails-3.1/tree/missing-rspec to use the version of RSpec, Capybara and Capybara-webkit as you have specified, and everything works OK (see mattheworiordan/capybara-screenshot-test-rails-3.1@8644f5a).

Can you possibly clone that repo and see if you experience the same issues, or if everything works with that repo?

@phillipoertel
Copy link

I think I was able to narrow this down a bit, thanks to your test app.

I cloned it and couldn't reproduce the issue with the existing tests. Then I wrote a test the way we set them up in our app and could reproduce the error

Add it to spec/integration/basic_spec.rb

describe "The link content element" do
  #context "Some context for testing the context block" do

    it "generates HTML showing that Javascript under Capybara-webkit works", :js => true do
      visit '/webkit'
      page.should have_content('Webkit supports Javascript')
      click_link('Does not exist')
    end

  #end
end

Running it like this, I get the following exception:

  1) The link content element generates HTML showing that Javascript under Capybara-webkit works
     Screenshot: /Users/bloom34/capybara-screenshot-test-rails-3.1/tmp/capybara/screenshot-2012-08-13-15-31-42.png
     Failure/Error: visit '/webkit'
     NoMethodError:
       undefined method `visit' for #<RSpec::Core::ExampleGroup::Nested_3:0x00000104c4d628>
     # ./spec/integration/basic_spec.rb:36:in `block (2 levels) in <top (required)>'

Some interesting observations:

  1. when I remove capybara-screenshot from my Gemfile and bundle, visit works as expected.
  2. when I uncomment (=enable) the context wrapped around the test, visit works as expected (wtf?!).
  3. when I add :type => :request after the describe-block, visit works as expected. You may say "of course, someone has to tell it it's a request spec", but I belive this should happen by inferral from the test directory (spec/integration in your case, spec/requests in ours)
  4. the error can be reproduced on the master branch of the test app as well, i.e. not only with the specific gem versions.

@phillipoertel
Copy link

Just confirmed that I can fix the bug in our app by setting :type => :request on the describe block. Nice workaround for now :-)

@envygeeks
Copy link
Author

@phillipoertel Thanks for the tip on that, since I really would like to take screenshots each test so I don't have to browse the site and can archive snapshots of development good and bad. So I assume this leads back to their before(:type => request) block, perhaps this might be an issue with RSpec then and not necessarily this gem?

@phillipoertel
Copy link

@envygeeks Well, remove capybara-screenshot and the error goes away ... so it is at least a part of the problem ...

@adzap
Copy link
Contributor

adzap commented Aug 13, 2012

@phillipoertel good work. So yes, the gem is causing the problem.

My guess is that because it loads capybara/rspec file before rspec-rails has loaded there may be some load order issue. Can you please try this:

  • set the gem to
# Gemfile
gem 'capybara-screenshot', :require => false
#spec_helper.rb
require 'rspec/rails'
require 'capybara/rails'
require 'capybara/rspec'
#... after those, where ever they appear. 
require 'capybara-screenshot'

@phillipoertel
Copy link

@adzap you're right, the error goes away when I set it up this way.

@adzap
Copy link
Contributor

adzap commented Aug 15, 2012

@phillipoertel so the puzzling thing is why it works for me and not you, under spec/requests folder. I'm guessing there is some interaction with another gem or how you or I require the files. Can you please post your Gemfile (test group) and spec_helper requires sections?

@phillipoertel
Copy link

? I can reproduce the problem in the example app @mattheworiordan mentiones above -- see Gemfile there. I've inserted the above test in the existing file at spec/integration/basic_spec.rb.

We're running MRI Ruby 1.9.3 (patch level 194) with rvm and it's own gemset ...

@adzap
Copy link
Contributor

adzap commented Aug 16, 2012

@phillipoertel yes, but that is the integration folder and I can reproduce that problem in my own app. But why is it that my spec/requests work fine and yours don't.

Anyway, perhaps it's not worth exploring that line any further. We have a load order issue, my guess is that it caused by requiring 'capybara/rspec' before 'capybara/rails'. The order detailed by capybara is rails then rspec, naturally enough. Adding a require to 'capybara/rails' to the gem just fails, since rails hasn't initialized I think. I didn't go too deep on that.

So, I think we need to take out all test library requires and make them manual require statements. This is usually how it is done in most testing library extensions which have a dependency on load order, which we do.

This would be a minor backward incompatible change requiring people to add the require, not a big deal. I'll make a commit to do it.

@mattheworiordan
Copy link
Owner

@adzap, sorry I'm not entirely clear of the implications to existing users. In what circumstances will users applying this upgrade need to change existing code?

@envygeeks
Copy link
Author

@mattheworiordan in all circumstances because now instead of this library loading your other libraries for you, you will be required to add those other libraries and then this one at the end. So most users will have to change their [test|spec]_helper.rb to fix this issue. Though as @adzap already implied the implications are small, a minor readme change to imply the change and the reason would solve most problems.

@mattheworiordan
Copy link
Owner

I understand that the change is a small one, but it's really not great for everyone who is using the Gem already (around 20k according to http://rubygems.org/gems/capybara-screenshot). When I get a chance, I'm going to see if I can think of a work around of some sort.

@adzap
Copy link
Contributor

adzap commented Aug 18, 2012

@mattheworiordan Everyone already using will only be effected if they upgrade the gem. You can bump it to 1 if you want conform to a vaguely semantic versioning scheme, though setup is included is not really the API.

Explicit require is how it should be done to be a good testing gem citizen. I have tried and failed to do auto testing gem loading before, and just stepped on more toes the more clever I got. Test loading it not as well managed or intelligent as loading the app is. Bad things can happen when you presume you can things in any order.

It is better not to be clever and have a minor install step of a require or two (Cucumber and/or RSpec/Minitest).

@adzap
Copy link
Contributor

adzap commented Aug 28, 2012

Update: I found the circumstances to make it fail even in the requests folder. I was not seeing this issue but I then created a request spec with an example block at the top level, and this caused the error. If you have an example in a nested context or describe it will not fail. I had up until then had all my examples nested.

This is crazy behaviour. I can get around it with my earlier suggestion of changing the Gemfile to add :require => false and do an explicit require.

I really don't think you can/should get around this without an explicit require setup. In my opinion it's broken currently.

@mattheworiordan
Copy link
Owner

Hi all

Sorry for the stupidly slow reply, I've just been absolutely flat out recently and not had time to look at this.

I found a more elegant solution IMH than @adzap's solution to fix the problem with the load order, see commit 84cea19

Thanks for all your help, and I'm sorry it took so long to get this sorted.

Please do send me your feedback if you find any problems with the new release 0.2.3

Matt

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

6 participants