Remove skip statements from scaffold controller tests during smoke

[JonRowe]

No description provided.

[fables-tales]

I like this. @JonRowe is this good to merge?

[JonRowe]

I'm not positive this is doing what I want yet @samphippen, please bear with me :)

[yujinakayama]

Now non-Rails 5 builds are properly failing.

[yujinakayama]

@JonRowe Is this good to merge?

[JonRowe]

@yujinakayama no this isn't ready to go yet, if you look at the build failure's it's failing because of routing issues rather than keyword arguments, you can see it's correctly grabbing params as a hash:

Failures:
  1) GadgetsController DELETE #destroy destroys the requested gadget
     Failure/Error: delete :destroy, params: {id: gadget.to_param}, session: valid_session
     
     ActionController::UrlGenerationError:
       No route matches {:action=>"destroy", :controller=>"gadgets", :params=>{:id=>"1"}, :session=>{}}
     # ./spec/controllers/gadgets_controller_spec.rb:130:in `block (4 levels) in <top (required)>'
     # ./spec/controllers/gadgets_controller_spec.rb:129:in `block (3 levels) in <top (required)>'

[yujinakayama]

you can see it's correctly grabbing params as a hash:

No, Rails isn't correctly interpreting the keyword arguments.

You see the :params in the error message:

No route matches {:action=>"destroy", :controller=>"gadgets", :params=>{:id=>"1"}, :session=>{}}

... just because the :params is literally described as the hash key:

get :show, params: {id: gadget.to_param}, session: valid_session
#          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#          This entire hash is interpreted as a param

If we change the get invocation to the following:

get :show, foo: {id: gadget.to_param}, bar: valid_session

... you'll see the following error message:

No route matches {:action=>"show", :bar=>{}, :controller=>"gadgets", :foo=>{:id=>"1"}}

So that means, on Rails 4, the get :show, params: {id: gadget.to_param} is handled as params[:params] in the controller.

[JonRowe]

I'm not convinced this is ready but merge this into your branch and see if it goes green, if so the combined branch is good to go, (I don't feel that strongly about it) but otherwise it's not. We can't merge this as is as it'll break the build.

[yujinakayama]

Then I'll close this PR and merge #1710, which is already based on this branch and is green.

[JonRowe]

Thanks for taking care of this @yujinakayama :)