Skip to content

Rails 5 support patches #1492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Dec 24, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ matrix:
# Rails 5.x only supports 2.2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be 2.2.2+ instead of 2.2?

- rvm: 2.2.2
env: RAILS_VERSION=master
- rvm: 2.2.2
env: RAILS_VERSION=5.0.0.beta1
exclude:
# 3.0.x is not supported on MRI 2.0+
- rvm: 2.0.0
Expand Down
1 change: 1 addition & 0 deletions Gemfile-rails-dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ when /master/
gem "journey", :git => "git://github.com/rails/journey.git"
gem "activerecord-deprecated_finders", :git => "git://github.com/rails/activerecord-deprecated_finders.git"
gem "rails-observers", :git => "git://github.com/rails/rails-observers"
gem "web-console", :git => "git://github.com/rails/web-console", :group => :development
gem 'sass-rails', :git => "git://github.com/rails/sass-rails.git"
gem 'coffee-rails', :git => "git://github.com/rails/coffee-rails.git"
gem 'rack', :git => 'git://github.com/rack/rack.git'
Expand Down
11 changes: 10 additions & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,16 @@ RSpec::Core::RakeTask.new(:spec) do |t|
t.rspec_opts = %w[--color]
end

Cucumber::Rake::Task.new(:cucumber)
Cucumber::Rake::Task.new(:cucumber) do |t|
version = ENV.fetch("RAILS_VERSION", "~> 4.2.0")
cucumber_flag = "--tags ~@rails_post_5"
p version
if /(^| )5(\.|-)0/ === version || version == "master"
cucumber_flag = "--tags ~@rails_pre_5"
end

t.cucumber_opts = cucumber_flag
end

namespace :generate do
desc "generate a fresh app with rspec installed"
Expand Down
5 changes: 5 additions & 0 deletions example_app_generator/generate_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
# Remove the existing rails version so we can properly use master or other
# edge branches
gsub_file 'Gemfile', /^.*\bgem 'rails.*$/, ''
gsub_file "Gemfile", /.*web-console.*/, ''

if Rails::VERSION::STRING >= '5.0.0'
append_to_file('Gemfile', "gem 'rails-controller-testing', :git => 'https://github.com/rails/rails-controller-testing'")
end

# Use our version of RSpec and Rails
append_to_file 'Gemfile', <<-EOT.gsub(/^ +\|/, '')
Expand Down
10 changes: 5 additions & 5 deletions features/controller_specs/render_views.feature
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ Feature: render_views
render_views

describe "GET index" do
it "says 'Listing widgets'" do
it "has a widgets related heading" do
get :index
expect(response.body).to match /Listing widgets/im
expect(response.body).to match /<h1>.*widgets/im
end
end
end
Expand All @@ -34,7 +34,7 @@ Feature: render_views
describe "GET index" do
it "renders the actual template" do
get :index
expect(response.body).to match /Listing widgets/im
expect(response.body).to match /<h1>.*widgets/im
end
end

Expand Down Expand Up @@ -65,7 +65,7 @@ Feature: render_views
describe "GET index" do
it "renders the actual template" do
get :index
expect(response.body).to match /Listing widgets/im
expect(response.body).to match /<h1>.*widgets/im
end
end
end
Expand Down Expand Up @@ -105,7 +105,7 @@ Feature: render_views
describe "GET index" do
it "renders the index template" do
get :index
expect(response.body).to match /Listing widgets/im
expect(response.body).to match /<h1>.*widgets/im
end
end
end
Expand Down
41 changes: 41 additions & 0 deletions features/mailer_specs/url_helpers.feature
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,46 @@ Feature: URL helpers in mailer examples
Mailer specs are marked by `:type => :mailer` or if you have set
`config.infer_spec_type_from_file_location!` by placing them in `spec/mailer`.

@rails_post_5
Scenario: using URL helpers with default options
Given a file named "config/initializers/mailer_defaults.rb" with:
"""ruby
Rails.configuration.action_mailer.default_url_options = { :host => 'example.com' }
"""
And a file named "spec/mailers/notifications_spec.rb" with:
"""ruby
require 'rails_helper'

RSpec.describe NotificationsMailer, :type => :mailer do
it 'should have access to URL helpers' do
expect { gadgets_url }.not_to raise_error
end
end
"""
When I run `rspec spec`
Then the examples should all pass

@rails_post_5
Scenario: using URL helpers without default options
Given a file named "config/initializers/mailer_defaults.rb" with:
"""ruby
# no default options
"""
And a file named "spec/mailers/notifications_spec.rb" with:
"""ruby
require 'rails_helper'

RSpec.describe NotificationsMailer, :type => :mailer do
it 'should have access to URL helpers' do
expect { gadgets_url :host => 'example.com' }.not_to raise_error
expect { gadgets_url }.to raise_error
end
end
"""
When I run `rspec spec`
Then the examples should all pass

@rails_pre_5
Scenario: using URL helpers with default options
Given a file named "config/initializers/mailer_defaults.rb" with:
"""ruby
Expand All @@ -21,6 +61,7 @@ Feature: URL helpers in mailer examples
When I run `rspec spec`
Then the examples should all pass

@rails_pre_5
Scenario: using URL helpers without default options
Given a file named "config/initializers/mailer_defaults.rb" with:
"""ruby
Expand Down
4 changes: 2 additions & 2 deletions lib/generators/rspec/mailer/templates/mailer_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
require "rails_helper"

<% module_namespacing do -%>
RSpec.describe <%= class_name %>, <%= type_metatag(:mailer) %> do
RSpec.describe <%= class_name %><%= Rails.version.to_f >= 5.0 ? "Mailer" : "" %>, <%= type_metatag(:mailer) %> do
<% for action in actions -%>
describe "<%= action %>" do
let(:mail) { <%= class_name %>.<%= action %> }
let(:mail) { <%= class_name %><%= Rails.version.to_f >= 5.0 ? "Mailer" : "" %>.<%= action %> }

it "renders the headers" do
expect(mail.subject).to eq(<%= action.to_s.humanize.inspect %>)
Expand Down
29 changes: 22 additions & 7 deletions lib/rspec/rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ class Configuration
:feature => %w[spec features]
}

# Sets up the different example group modules for the different spec types
#
# @api private
def self.add_test_type_configurations(config)
config.include RSpec::Rails::ControllerExampleGroup, :type => :controller
config.include RSpec::Rails::HelperExampleGroup, :type => :helper
config.include RSpec::Rails::ModelExampleGroup, :type => :model
config.include RSpec::Rails::RequestExampleGroup, :type => :request
config.include RSpec::Rails::RoutingExampleGroup, :type => :routing
config.include RSpec::Rails::ViewExampleGroup, :type => :view
config.include RSpec::Rails::FeatureExampleGroup, :type => :feature
end

# @private
def self.initialize_configuration(config)
config.backtrace_exclusion_patterns << /vendor\//
Expand Down Expand Up @@ -98,13 +111,15 @@ def filter_rails_from_backtrace!
end
end

config.include RSpec::Rails::ControllerExampleGroup, :type => :controller
config.include RSpec::Rails::HelperExampleGroup, :type => :helper
config.include RSpec::Rails::ModelExampleGroup, :type => :model
config.include RSpec::Rails::RequestExampleGroup, :type => :request
config.include RSpec::Rails::RoutingExampleGroup, :type => :routing
config.include RSpec::Rails::ViewExampleGroup, :type => :view
config.include RSpec::Rails::FeatureExampleGroup, :type => :feature
add_test_type_configurations(config)

if defined?(::Rails::Controller::Testing)
[:controller, :view, :request].each do |type|
config.include ::Rails::Controller::Testing::TestProcess, :type => type
config.include ::Rails::Controller::Testing::TemplateAssertions, :type => type
config.include ::Rails::Controller::Testing::Integration, :type => type
end
end

if defined?(ActionMailer)
config.include RSpec::Rails::MailerExampleGroup, :type => :mailer
Expand Down
10 changes: 8 additions & 2 deletions lib/rspec/rails/example/view_example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,14 @@ def _include_controller_helpers
end

controller.controller_path = _controller_path
controller.request.path_parameters[:controller] = _controller_path
controller.request.path_parameters[:action] = _inferred_action unless _inferred_action =~ /^_/

path_params_to_merge = {}
path_params_to_merge[:controller] = _controller_path
path_params_to_merge[:action] = _inferred_action unless _inferred_action =~ /^_/

path_params = controller.request.path_parameters

controller.request.path_parameters = path_params.reverse_merge(path_params_to_merge)
controller.request.path = ViewPathBuilder.new(::Rails.application.routes).path_for(controller.request.path_parameters)
ViewSpecMethods.add_to(::ActionView::TestCase::TestController)
end
Expand Down
6 changes: 4 additions & 2 deletions lib/rspec/rails/matchers/have_http_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ def as_test_response(obj)
# Capybara or catch `NameError`s for the undefined constants
obj = ActionDispatch::Response.new.tap do |resp|
resp.status = obj.status_code
resp.headers = obj.response_headers
resp.headers.clear
resp.headers.merge!(obj.response_headers)
resp.body = obj.body
resp.request = ActionDispatch::Request.new({})
end
::ActionDispatch::TestResponse.from_response(obj)
else
Expand Down Expand Up @@ -84,7 +86,7 @@ def initialize(code)
# @return [Boolean] `true` if the numeric code matched the `response` code
def matches?(response)
test_response = as_test_response(response)
@actual = test_response.response_code
@actual = test_response.response_code.to_i
expected == @actual
rescue TypeError => _ignored
@invalid_response = response
Expand Down
7 changes: 6 additions & 1 deletion spec/generators/rspec/mailer/mailer_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
end
it { is_expected.to exist }
it { is_expected.to contain(/require "rails_helper"/) }
it { is_expected.to contain(/^RSpec.describe Posts, #{type_metatag(:mailer)}/) }
if Rails.version.to_f >= 5.0
# Rails 5 automatically appends Mailer to the provided constant so we do too
it { is_expected.to contain(/^RSpec.describe PostsMailer, #{type_metatag(:mailer)}/) }
else
it { is_expected.to contain(/^RSpec.describe Posts, #{type_metatag(:mailer)}/) }
end
it { is_expected.to contain(/describe "index" do/) }
it { is_expected.to contain(/describe "show" do/) }
end
Expand Down
1 change: 1 addition & 0 deletions spec/rspec/rails/example/mailer_example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module RSpec::Rails
module ::Rails; end
before do
allow(Rails).to receive_message_chain(:application, :routes, :url_helpers).and_return(Rails)
allow(Rails.application).to receive(:config).and_return(double("Rails.application.config").as_null_object)
allow(Rails).to receive_message_chain(:configuration, :action_mailer, :default_url_options).and_return({})
end

Expand Down
21 changes: 13 additions & 8 deletions spec/rspec/rails/matchers/have_http_status_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,27 @@
include RSpec::Rails::Matchers

def create_response(opts = {})
ActionDispatch::TestResponse.new(opts.fetch(:status))
ActionDispatch::TestResponse.new(opts.fetch(:status)).tap {|x|
x.request = ActionDispatch::Request.new({})
}
end

shared_examples_for "supports different response instances" do
context "given an ActionDispatch::Response" do
it "returns true for a response with the same code" do
response = ::ActionDispatch::Response.new(code)
response = ::ActionDispatch::Response.new(code).tap {|x|
x.request = ActionDispatch::Request.new({})
}

expect( matcher.matches?(response) ).to be(true)
end
end

context "given an ActionDispatch::TestResponse" do
it "returns true for a response with the same code" do
response = ::ActionDispatch::TestResponse.new(code)
response = ::ActionDispatch::TestResponse.new(code).tap {|x|
x.request = ActionDispatch::Request.new({})
}

expect( matcher.matches?(response) ).to be(true)
end
Expand Down Expand Up @@ -382,12 +388,12 @@ def create_response(opts = {})
it_behaves_like "supports different response instances" do
subject(:matcher) { have_redirect_status }

let(:code) { 333 }
let(:code) { 308 }
end

describe "matching a response" do
it "returns true for a response with a 3xx status code" do
any_3xx_code = 333
any_3xx_code = 308
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be dynamic depending on version? Or does 308 play ok for rails 4 etc too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

308 is actually a valid code in the spec. 333 is not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I made this change is that somewhere along the way, rails 5 no-longer accepts 3xx codes that aren't part of the spec in a bunch of places. 333 is not part of the spec. Given that all old versions of rails support any http response code that is in the spec, this seemed like the easiest change to make for this PR to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been resolved

response = create_response(:status => any_3xx_code)

expect( have_redirect_status.matches?(response) ).to be(true)
Expand Down Expand Up @@ -416,12 +422,12 @@ def create_response(opts = {})
end

it "has a negated failure message reporting the expected and actual status codes" do
any_3xx_code = 333
any_3xx_code = 308
response = create_response(:status => any_3xx_code)

expect{ have_redirect_status.matches? response }.
to change(have_redirect_status, :failure_message_when_negated).
to(/not to have a redirect status code \(3xx\) but it was 333/)
to(/not to have a redirect status code \(3xx\) but it was 308/)
end
end

Expand All @@ -430,5 +436,4 @@ def create_response(opts = {})
expect{ have_http_status(nil) }.to raise_error ArgumentError
end
end

end
2 changes: 2 additions & 0 deletions spec/support/ar_classes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def self.extended(host)
nonexistent_model_id integer
)
eosql

host.reset_column_information
end
end

Expand Down