Get the test suite passing against Rails 5 #4254

seanlinsley · 2015-12-27T17:20:07Z

This is very much a WIP.

~~It's currently based on #4253. Once that is merged I'll rebase this PR.~~
#4177

seanlinsley · 2015-12-27T17:23:09Z

The current failures:

Failures:

  1) ActiveAdmin::ResourceController::Sidebars without before_filter does not set @skip_sidebar
     Failure/Error: get :index

     ActionController::UrlGenerationError:
       No route matches {:action=>"index", :controller=>"admin/posts"}
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/journey/formatter.rb:45:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:619:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:650:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:645:in `generate_extras'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:499:in `process'
     # devise-643144584684/lib/devise/test_helpers.rb:19:in `block in process'
     # devise-643144584684/lib/devise/test_helpers.rb:75:in `catch'
     # devise-643144584684/lib/devise/test_helpers.rb:75:in `_catch_warden'
     # devise-643144584684/lib/devise/test_helpers.rb:19:in `process'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:618:in `process_with_kwargs'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:366:in `get'
     # ./spec/unit/resource_controller/sidebars_spec.rb:17:in `block (3 levels) in <top (required)>'

  2) ActiveAdmin::ResourceController::Sidebars#skip_sidebar! works
     Failure/Error: get :index

     ActionController::UrlGenerationError:
       No route matches {:action=>"index", :controller=>"admin/posts"}
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/journey/formatter.rb:45:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:619:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:650:in `generate'
     # rails-46bbdf58b50a/actionpack/lib/action_dispatch/routing/route_set.rb:645:in `generate_extras'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:499:in `process'
     # devise-643144584684/lib/devise/test_helpers.rb:19:in `block in process'
     # devise-643144584684/lib/devise/test_helpers.rb:75:in `catch'
     # devise-643144584684/lib/devise/test_helpers.rb:75:in `_catch_warden'
     # devise-643144584684/lib/devise/test_helpers.rb:19:in `process'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:618:in `process_with_kwargs'
     # rails-46bbdf58b50a/actionpack/lib/action_controller/test_case.rb:366:in `get'
     # ./spec/unit/resource_controller/sidebars_spec.rb:31:in `block (3 levels) in <top (required)>'

  3) ActiveAdmin Routing root path helper when in admin namespace should be admin_root_path
     Failure/Error: expect(admin_root_path).to eq "/admin"

     NameError:
       undefined local variable or method admin_root_path for #<RSpec::ExampleGroups::ActiveAdminRouting::RootPathHelper::WhenInAdminNamespace:0x007f8c4c2f55f0>
     # ./spec/unit/routing_spec.rb:23:in `block (4 levels) in <top (required)>'

Finished in 22.29 seconds (files took 3.97 seconds to load)
1407 examples, 3 failures

Failed examples:

rspec ./spec/unit/resource_controller/sidebars_spec.rb:16 # ActiveAdmin::ResourceController::Sidebars without before_filter does not set @skip_sidebar
rspec ./spec/unit/resource_controller/sidebars_spec.rb:30 # ActiveAdmin::ResourceController::Sidebars#skip_sidebar! works
rspec ./spec/unit/routing_spec.rb:22 # ActiveAdmin Routing root path helper when in admin namespace should be admin_root_path

The first two failures don't happen if you run that file on its own. If you run routing_spec on its own, however, you get a new test failure:

Failures:

  1) ActiveAdmin Routing should only have the namespaces necessary for route testing
     Failure/Error: expect(ActiveAdmin.application.namespaces.names).to eq [:admin, :root]

       expected: [:admin, :root]
            got: [:admin]

       (compared using ==)

       Diff:
       @@ -1,2 +1,2 @@
       -[:admin, :root]
       +[:admin]
     # ./spec/unit/routing_spec.rb:13:in `block (2 levels) in <top (required)>'

  2) ActiveAdmin Routing root path helper when in admin namespace should be admin_root_path
     Failure/Error: expect(admin_root_path).to eq "/admin"

     NameError:
       undefined local variable or method admin_root_path for #<RSpec::ExampleGroups::ActiveAdminRouting::RootPathHelper::WhenInAdminNamespace:0x007ff053c5d348>
     # ./spec/unit/routing_spec.rb:23:in `block (4 levels) in <top (required)>'

Finished in 3.35 seconds (files took 3.25 seconds to load)
23 examples, 2 failures

Failed examples:

rspec ./spec/unit/routing_spec.rb:12 # ActiveAdmin Routing should only have the namespaces necessary for route testing
rspec ./spec/unit/routing_spec.rb:22 # ActiveAdmin Routing root path helper when in admin namespace should be admin_root_path

seanlinsley · 2016-01-02T18:04:11Z

This is going to require too many branching conditionals in our codebase to make sense... It looks like we'll have to drop support for Rails 3.2 in order to support Rails 5.

seanlinsley · 2016-01-02T19:51:59Z

I'm currently getting this exception when the Devise login page renders:

Formtastic::UnknownInputError - Unable to find input class Input:
  formtastic (3.1.3) lib/formtastic/helpers/input_helper.rb:334:in `rescue in namespaced_input_class'
  formtastic (3.1.3) lib/formtastic/helpers/input_helper.rb:331:in `namespaced_input_class'
  formtastic (3.1.3) lib/formtastic/helpers/input_helper.rb:340:in `input_class'
  formtastic (3.1.3) lib/formtastic/helpers/input_helper.rb:238:in `input'
  arbre (1.0.3) lib/arbre/rails/forms.rb:30:in `proxy_call_to_form'
  ./lib/active_admin/views/components/active_admin_form.rb:65:in `input'
  ./app/views/active_admin/devise/sessions/new.html.erb:10
  ./lib/active_admin/views/components/active_admin_form.rb:53:in `block (2 levels) in inputs'
   () rails-a0a62da235e8/actionview/lib/action_view/helpers/capture_helper.rb:39:in `block in capture'
   () rails-a0a62da235e8/actionview/lib/action_view/helpers/capture_helper.rb:203:in `with_output_buffer'
   () rails-a0a62da235e8/actionview/lib/action_view/helpers/capture_helper.rb:39:in `capture'
  ./lib/active_admin/views/components/active_admin_form.rb:52:in `block in inputs'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
  arbre (1.0.3) lib/arbre/context.rb:92:in `with_current_arbre_element'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:26:in `build_tag'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:39:in `insert_tag'
  ./lib/active_admin/views/components/active_admin_form.rb:58:in `inputs'
  ./app/views/active_admin/devise/sessions/new.html.erb:6
  ./lib/active_admin/views/components/active_admin_form.rb:35:in `build'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:28:in `block in build_tag'
  arbre (1.0.3) lib/arbre/context.rb:92:in `with_current_arbre_element'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:26:in `build_tag'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:39:in `insert_tag'
  arbre (1.0.3) lib/arbre/element/builder_methods.rb:14:in `active_admin_form_for'
  ./lib/active_admin/view_helpers/form_helper.rb:7:in `block in active_admin_form_for'
  arbre (1.0.3) lib/arbre/context.rb:45:in `initialize'
  ./lib/active_admin/view_helpers/form_helper.rb:6:in `active_admin_form_for'
  ./app/views/active_admin/devise/sessions/new.html.erb:5

It's blocked by: heartcombo/devise#3880

agrobbin · 2016-01-02T21:28:13Z

This is going to require too many branching conditionals in our codebase to make sense... It looks like we'll have to drop support for Rails 3.2 in order to support Rails 5.

Just a suggestion @seanlinsley, but is there any reason to support anything older than the oldest currently supported Rails version? Rails doesn't even support 4.0 anymore, let alone 3.2 (with the limited exception of "severe security issues". Now that Rails 5.0 is in beta and soon to be released, 4.1 support could also be dropped.

seanlinsley · 2016-01-02T23:27:58Z

I could see Active Admin 2.0 requiring Rails >= 4.2 and Ruby >= 2

jtomaszewski · 2016-01-11T07:21:11Z

Formtastic::UnknownInputError - Unable to find input class Input: propably will be fixed via formtastic/formtastic#1184 .

Sprockets 4 currently doesn’t work with AA, failing to load our assets inside of a test app when it calls `@import`

just call them instead, by actually hitting the controller via its public API this change was necessary because the internal callbacks object has changed quite a bit

While pretty complicated, this implementation should allow us to support Rails 3.2 to Rails 5. Any callback inside of the controller itself still requires an if condition to determine which method to call, since our methods are defined on the resource object, which wraps the controller.

Controller tests now have to scope query params inside of a params key (it also now lets you pass session variables & headers). This test had been updated with the new syntax, but we still need to support Rails 4.

& call puts for deprecation in Cucumber, because they sometimes won’t be printed and you’ll be left with a failed test but no failure message

timoschilling · 2016-02-08T07:59:11Z

Gemfile

+if rails_version != 'master'
+  gem 'devise'
+  gem 'draper'
+end


why this? this gems are already in line 19 and 22

The problem is Bundler raises an error if you define a gem multiple times in your Gemfile.

I tried to keep the Github gem requirements in a single section of the file, so that once Rails is released that section can be deleted, and then we can remove the if rails_version != 'master' conditionals in the rest of the file.

timoschilling · 2016-02-09T08:27:11Z

Maybe you want to move all if version or if feature stuff into the adapter, before merging this.

I copied the `> 5.x` version requirement from rails/activemodel-serializers-xml@b8173f0

also the final line was an unintentional change

seanlinsley · 2016-02-13T22:29:10Z

https://twitter.com/seanlinsley/status/698633817121234945

Get the test suite passing against Rails 5

seanlinsley added the Rails 5 label Dec 27, 2015

seanlinsley force-pushed the rails-5-rspec branch from d62da5c to 5990259 Compare January 2, 2016 18:03

This was referenced Jan 6, 2016

Rails 5 beta1 support formtastic/formtastic#1183

Closed

Use the new attributes API in Rails 5 heartcombo/devise#3880

Closed

seanlinsley force-pushed the rails-5-rspec branch 3 times, most recently from 34b2b88 to 9b4d233 Compare February 7, 2016 22:23

seanlinsley added 17 commits February 7, 2016 16:59

use Github versions of gems with Rails 5 fixes

fed57d9

Sprockets 4 currently doesn’t work with AA, failing to load our assets inside of a test app when it calls `@import`

test app generation fixes

c56623c

RSpec now requires error messages on expectations

3ed7b14

RSpec methods are no longer available globally

8138b6b

use request.user_agent

3c689a1

render text: is deprecated

efd6387

callstack objects are no longer arrays of strings

8ec21ee

stop using hacks to test that controller actions are called

64ca9b4

just call them instead, by actually hitting the controller via its public API this change was necessary because the internal callbacks object has changed quite a bit

deal with parameterize API change

df7fe4d

fix controller test for Rails < 5

d5ebd04

Controller tests now have to scope query params inside of a params key (it also now lets you pass session variables & headers). This test had been updated with the new syntax, but we still need to support Rails 4.

fix permitted params conditionals for Rails 5

8d2b79f

use database_cleaner from Github

5a47b78

fix transactional tests deprecation

1692666

& call puts for deprecation in Cucumber, because they sometimes won’t be printed and you’ll be left with a failed test but no failure message

uniq -> distinct

e4e77b0

handle redirect_to :back API change

d617e00

more callback fixes

494b0c6

timoschilling reviewed Feb 8, 2016
View reviewed changes

seanlinsley mentioned this pull request Feb 8, 2016

Get test suite passing with Sprockets 3 #4309

Closed

timoschilling mentioned this pull request Feb 9, 2016

Cleanup spec files #4312

Closed

seanlinsley added 3 commits February 13, 2016 13:48

use Rubygems versions where available

68855f4

I copied the `> 5.x` version requirement from rails/activemodel-serializers-xml@b8173f0

test against Ruby 2.3

a83dd28

move strong_parameters conditions into Dependency class

ca5a8b8

seanlinsley force-pushed the rails-5-rspec branch from 7a07a5c to 43789f4 Compare February 13, 2016 20:33

move render key into Dependency class

c740f4a

seanlinsley force-pushed the rails-5-rspec branch from 43789f4 to c740f4a Compare February 13, 2016 22:18

try fixing Travis

e2ba340

also the final line was an unintentional change

seanlinsley added 2 commits February 13, 2016 16:31

"2.3" isn't recognized by RVM / Travis

f4e0629

add documentation to the README

5c50809

seanlinsley added a commit that referenced this pull request Feb 13, 2016

Merge pull request #4254 from activeadmin/rails-5-rspec

0c644ef

Get the test suite passing against Rails 5

seanlinsley merged commit 0c644ef into master Feb 13, 2016

seanlinsley deleted the rails-5-rspec branch February 13, 2016 23:22

This was referenced Feb 18, 2016

Rails 5: In fields_for, use to_unsafe_h on params before flat_map if necessary #4145

Closed

"Generating an URL from non sanitized request parameters is insecure!" #4292

Closed

deivid-rodriguez mentioned this pull request Sep 12, 2016

Add Appraisal #4624

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get the test suite passing against Rails 5 #4254

Get the test suite passing against Rails 5 #4254

seanlinsley commented Dec 27, 2015

seanlinsley commented Dec 27, 2015

seanlinsley commented Jan 2, 2016

seanlinsley commented Jan 2, 2016

agrobbin commented Jan 2, 2016

seanlinsley commented Jan 2, 2016

jtomaszewski commented Jan 11, 2016

timoschilling Feb 8, 2016

seanlinsley Feb 8, 2016

timoschilling commented Feb 9, 2016

seanlinsley commented Feb 13, 2016

Get the test suite passing against Rails 5 #4254

Get the test suite passing against Rails 5 #4254

Conversation

seanlinsley commented Dec 27, 2015

seanlinsley commented Dec 27, 2015

seanlinsley commented Jan 2, 2016

seanlinsley commented Jan 2, 2016

agrobbin commented Jan 2, 2016

seanlinsley commented Jan 2, 2016

jtomaszewski commented Jan 11, 2016

timoschilling Feb 8, 2016

Choose a reason for hiding this comment

seanlinsley Feb 8, 2016

Choose a reason for hiding this comment

timoschilling commented Feb 9, 2016

seanlinsley commented Feb 13, 2016