Before-Filter are executed twice
Reported by Kostia | June 19th, 2008 @ 11:39 AM | in 1.1.5
Before-Filter in application.rb are exected twice.
$> cd /tmp
$> rails test
$> cd test
$> ./script/plugin install git://github.com/dchelimsky/rspec.git
$> ./script/plugin install git://github.com/dchelimsky/rspec-rails.git
$> ./script/generate rspec
$> ./script/generate rspec_scaffold User name:string
$> rake db:migrate
"app/controllers/aplication.rb":
class ApplicationController < ActionController::Base
helper :all # include all helpers, all the time
protect_from_forgery # :secret => 'c4dfc3d116d130ee911ec43dbbe448c3'
before_filter :foo
def foo; end
end
"spec/controllers/user_controller_spec.rb":
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe UsersController do
describe "handling GET /users" do
before(:each) do
@user = mock_model(User)
User.stub!(:find).and_return([@user])
end
def do_get
get :index
end
it "should be successful" do
do_get
response.should be_success
end
it 'should call before filter foo only once' do
controller.should_receive :foo
do_get
end
(...)
$> rake spec
Will fail, because foo will be called twice.
Example in attachment.
Comments and changes to this ticket
-

Martin Sadler June 19th, 2008 @ 12:29 PM
Hi Kostia,
This patch to the rspec-rails plugin will stop application.rb from being required if it is already loaded.
Hope that helps,
Martin.
-
David Chelimsky June 19th, 2008 @ 01:30 PM
- → State changed from new to open
- → Assigned user changed from to David Chelimsky
Hey Martin - thanks for the patch. If you add an example I'd gladly apply it.
-
Matt Patterson July 10th, 2008 @ 06:48 PM
- → Tag changed from to bug controllers rails rspec_controller rspec_on_rails
I've had run-ins with this in the past. I'm happy to write examples for it.
-
Brandon Keepers July 11th, 2008 @ 06:01 PM
- no changes were found...
-
Brandon Keepers July 11th, 2008 @ 06:01 PM
Per this thread on Rails-core, I think the proper solution is to use require_dependency instead of simply require.
It's not ideal, but until the #require_dependency code is fixed so that #require doesn't cause the file to be loaded again, that seems to be the cleanest solution.
David, I'm not exactly sure how to spec this, do you have any suggestions?
-
Matt Patterson July 11th, 2008 @ 06:19 PM
I'm guessing it depends on where the require happens - if it's rspec-rails then require-dependency is a possible, otherwise it's not...
-
-
David Chelimsky July 12th, 2008 @ 04:47 AM
Hey Brandon,
This ticket says before filters are executed twice, so I'd think a good example (failing pre-patch of course) would be one that expects a before_filter to be executed only once. That'd work for me. Probably belongs in rspec-rails/spec/rails/example/controller_spec_spec.rb.
Also, not sure how you made that patch, but if you use git-format-patch it'll include you in the commit log. That would make me happy.
Cheers,
David
-
Michael Koziarski July 12th, 2008 @ 09:34 AM
Just to be clear on the reason you need to make this change. require_dependency does some path normalisation to ensure that it's in the auto_load_paths. So it ends up doing require 'app/controllers/application'.
Calling require 'application' will then trigger the double require as require doesn't attempt to ensure the same file isn't required twice, merely that passing the same arguments to require doesn't trigger two requirements.
-
Brandon Keepers July 14th, 2008 @ 03:35 PM
David,
Writing an example for this is not quite that easy. The before filters being executed twice due to application.rb being loaded a second time when 'spec/rails' is required. The specs don't have an application.rb (spec controller extends directly from AC::Base), nor do they require 'spec/rails' to bootstrap. Do you want me to modify them so that there is an application.rb and the specs require 'spec/rails'?
-
Matt Patterson July 15th, 2008 @ 10:36 AM
I've attached a patch with a failing example - I added a controller and spec to example_rails_app, since this seems like the only easy way to get the needed environment... (also stored as http://github.com/fidothe/rspec-... )
-
David Chelimsky July 15th, 2008 @ 02:14 PM
Thanks for that Matt. I'd like to change a couple of things in this though:
1. move it down to rspec-rails (in the controller_spec_spec)
2. use pending("fix to http://rspec.lighthouseapp.com/p...") instead of commenting the lines in the example.
Would you like to do that and re-submit? If not, I'll do it myself.
Either way, Brandon, this should suit our needs for a failing example once we get it committed.
Thanks,
David
-
Matt Patterson July 15th, 2008 @ 04:01 PM
- no changes were found...
-
Matt Patterson July 15th, 2008 @ 04:01 PM
Aha. pre_commit uses rake:spec:plugins to run the specs, so i can have a controller in rspec-rails/spec which inherits from ApplicationController... Apologies for being slightly brain dead earlier.
Unfortunately, the before filter still needs to be defined on ApplicationController, so I've left that in there, which means two patches: one for rspec-dev and one for rspec-rails.
I can't see a way round that - reopening ApplicationController, defining the filter method and calling before_filter to add it won't cause the problem behaviour, since application.rb will already have been required twice... (I'm open to suggestions though...)
-
Matt Patterson July 15th, 2008 @ 04:02 PM
- no changes were found...
-
-
David Chelimsky July 16th, 2008 @ 06:53 AM
Hey Matt - I applied the your patches - thanks - I screwed up your name in my haste to get this committed though, so it says Peterson instead of Patterson in the commit logs. Sorry. For future, I'd recommend using git-format-patch so this is not left up to me to screw up :)
Brandon - you have a failing example (pending right now). Have at it!
Cheers,
David
-
David Chelimsky July 16th, 2008 @ 06:56 AM
Matt - PS - I added you to the contributor page, so even though the commit logs have the wrong name, the website will have the right one (after the 1.1.5 release).
Cheers,
David
-
Brandon Keepers July 18th, 2008 @ 04:27 PM
- no changes were found...
-
Brandon Keepers July 18th, 2008 @ 04:27 PM
Sorry, didn't see that you were waiting for me to submit a fix.
Whew, that was a tough one! ;)
Matt, thanks for the failing spec.
-
David Chelimsky July 18th, 2008 @ 04:51 PM
I must be missing something somewhere - I applied both of Matt's latest patches and then yours Brandon, but I get this:
ActionController::UnknownAction in 'A controller example running in isolation mode should only have a before filter inherited from ApplicationController run once...' No action responded to action_with_inherited_before_filter ./vendor/plugins/rspec-rails/spec/rails/example/controller_spec_spec.rb:223: -
Brandon Keepers July 18th, 2008 @ 05:01 PM
Hmm, Besides 2 errors related to FlexMock, pre_commit passes for me. See attached output.
I ran the specs against Rails 2.1.0, does the Rails version matter?
-
David Chelimsky July 18th, 2008 @ 05:43 PM
We run the suite against Rails 1.2.6, 2.0.2, 2.1.0 and edge.
I got those errors against 2.1.0, but as I search through my code I can't find any instance of 'def action_with_inherited_before_filter', so I think I'm missing a patch from either you or Matt. The three I applied were:
http://rspec.lighthouseapp.com/a...
-
Matt Patterson July 18th, 2008 @ 07:51 PM
Whoops. When I moved the specs out of rspec-dev/example_rails_app I renamed the action in the spec because of the change in context, but forgot in application.rb. New patches have been attached, and my program of making myself look inept will continue as scheduled...
They apply on top of the three previously-applied patches... (one to rspec-dev and one to rspec-rails - file names have the repo name)
-
Matt Patterson July 18th, 2008 @ 07:51 PM
- no changes were found...
-
David Chelimsky July 19th, 2008 @ 03:16 AM
- → Milestone changed from No-Milestone-Assigned to 1.1.5
- → State changed from open to resolved
(from [86afd94e57af4eb5e79a35dc781b7d2899aa0bc0]) use require_dependency instead of require to prevent application.rb from being loaded twice [#440 state:resolved milestone:"1.1.5"]
-
David Chelimsky July 19th, 2008 @ 03:17 AM
w00t! Finally - committed.
Brandon/Matt - thank you both for sticking w/ this.
Cheers,
David
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Behaviour Driven Development for Ruby.
