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

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

> 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

>
> >
> >
> > > 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

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.

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 :slight_smile:

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

> 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 :slight_smile:

Yes, and I presume end users would like to be notified when they do
silly things. :slight_smile:

> 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!