On Sun, Jul 29, 2012 at 01:47:34PM -0700, David Chelimsky wrote: > On Sunday, July 29, 2012 4:29:59 PM UTC-4, Aaron Patterson wrote: > > > > On Sun, Jul 29, 2012 at 11:54:33AM -0400, Matt Jones wrote: > > > > > > On Jul 28, 2012, at 11:44 PM, Aaron Patterson wrote: > > > > > > > In the case a developer has not constructed a controller, the setup > > > > method of ActionController::TestCase will attempt to construct a > > > > controller object. If it cannot construct a controller object, it > > > > silently fails. > > > > > > > > I added a warning in this case, and I'd like to eventually deprecate > > the > > > > behavior. I can't think of why anyone would want to use > > > > ActionController::TestCase and *not* test a controller. Does anyone > > > > know a reason *why* we would do this? > > > > > > > > > > https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/test_case.rb#L534-542 > > > > > > > > Maybe I'm missing something, but doesn't the call to > > setup_controller_request_and_response happen *before* any user-defined > > setup methods get called? In that case, it's intended to let users do > > unusual things (that don't set, or set to nonsense, controller_class) and > > then set up their own controller object. > > > > Yes, I think that is true. However, there is the `controller_class=` > > and the `tests` class method that you can use when AC::TC cannot intuit > > the > > controller class from your test class name: > > > > > > https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/test_case.rb#L390-393 > > > > > > If you needed a dynamic anonymous controllers, you could just implement > > the `controller_class` method on your test case, e.g.: > > > > class FooTest < ActionController::TestCase > > def self.controller_class > > # new anonymous subclass on every test > > Class.new(ActionController::Base) > > end > > end > > > > > There are some related commits that seem relevant: > > > > > > > > https://github.com/rails/rails/commit/13950a8cc94ab93bb20976f2797b1df9740849b3 > > > > > > > https://github.com/rails/rails/commit/ee82e1c3015392c87c88ee32003763210a75d1ec > > > > > > > https://github.com/rails/rails/commit/529a3ee50eeb7db5a78afeb5da38fd71b42e7ea2 > > > > > > > > I'd say there's a good chance that all of these changes are intended to > > support doing things like this: > > > > > > > > https://www.relishapp.com/rspec/rspec-rails/docs/controller-specs/anonymous-controller > > > > > > I could be mistaken, but I don't think RSpec uses AC::TC behavior > > methods. Maybe Mr. Chelimsky can enlighten us. > > > > > by handling the case where the controller-under-test isn't a named > > constant. > > > > As I demoed above, the controller doesn't need to be a named constant, > > you just need to implement the correct factory method. So I'm still at > > a loss why we would want to support "unconstructable" controllers. > > > > The reason I want to get rid of this code is because there is currently > > a code path that allows `@controller` to be nil during the test run. > > This is annoying because there are *many* methods[1] that depend on this > > instance variable, yet we cannot guarantee that the instance variable is > > set. > > > > If this is truly something that is for RSpec only, then perhaps this > > behavior should be pushed to RSpec rather than maintained in Rails. > > > > 1. > > https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/test_case.rb#L414-525 > > > > > rspec-rails overrides the `controller_class` method, providing its own > based on the object passed to `describe` [1]. For anonymous controller > specs, it generates a subclass of ApplicationController (by default) or a > user defined base class [2]. > > I'm not clear on what you're proposing to change, but as long as Rails > continues to generate a controller using `controller_class`, rspec-rails > should (I think) continue to work as it does without any changes. Of > course, I'd love to verify that if you do make any such changes.
Perfect. That's what I would expect. Will `controller_class` ever return something that can't be constructed via `new`? Basically, I'd like to get rid of this rescue: https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/test_case.rb#L538-540 > That help? Sure does! Thanks! <3<3 -- Aaron Patterson http://tenderlovemaking.com/
pgpKVmE8inx7l.pgp
Description: PGP signature
