Re: [Rails-core] ActionController::TestCase controller construction
On Sun, Jul 29, 2012 at 02:49:55PM -0700, David Chelimsky wrote: > > Perfect. That's what I would expect. Will `controller_class` ever > > return something that can't be constructed via `new`? > > > > Not by rspec, but I can't account for end users doing silly things :) Yes, and I presume end users would like to be notified when they do silly things. :-) > > 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 seems completely fine. Let it error out if there's a problem > instantiating. Great, thanks! -- Aaron Patterson http://tenderlovemaking.com/ pgpCU0b5SSzci.pgp Description: PGP signature
Re: [Rails-core] ActionController::TestCase controller construction
On Sunday, July 29, 2012 5:22:05 PM UTC-4, Aaron Patterson wrote: > > 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`? > Not by rspec, but I can't account for end users doing silly things :) > 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 seems completely fine. Let it error out if there's a problem instantiating. Cheers, David > > > That help? > > Sure does! Thanks! <3<3 > > --
Re: [Rails-core] ActionController::TestCase controller construction
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
Re: [Rails-core] ActionController::TestCase controller construction
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. That help? [1] https://github.com/rspec/rspec-rails/blob/master/lib/rspec/rails/example/controller_example_group.rb#L17-19 [2] https://github.com/rspec/rspec-rails/blob/master/lib/rspec/rails/example/controller_example_group.rb#L56-74 -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/csUWt_zSGkMJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] ActionController::TestCase controller construction
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 -- Aaron Patterson http://tenderlovemaking.com/ pgpUjNlGPEmgI.pgp Description: PGP signature
Re: [Rails-core] ActionController::TestCase controller construction
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. 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 by handling the case where the controller-under-test isn't a named constant. --Matt Jones -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
[Rails-core] ActionController::TestCase controller construction
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 -- Aaron Patterson http://tenderlovemaking.com/ pgpgcDpDCUzhs.pgp Description: PGP signature
