diff options
92 files changed, 971 insertions, 421 deletions
@@ -16,6 +16,8 @@ gem "rake", ">= 11.1" # be loaded after loading the test library. gem "mocha", "~> 0.14", require: false +gem "capybara", "~> 2.7.0" + gem "rack-cache", "~> 1.2" gem "jquery-rails" gem "coffee-rails" diff --git a/Gemfile.lock b/Gemfile.lock index c40730a33d..402eae60f6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -125,6 +125,13 @@ GEM bunny (2.6.2) amq-protocol (>= 2.0.1) byebug (9.0.6) + capybara (2.7.1) + addressable + mime-types (>= 1.16) + nokogiri (>= 1.3.3) + rack (>= 1.0.0) + rack-test (>= 0.5.4) + xpath (~> 2.0) childprocess (0.5.9) ffi (~> 1.0, >= 1.0.11) coffee-rails (4.2.1) @@ -356,6 +363,8 @@ GEM websocket-driver (0.6.4) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) + xpath (2.0.0) + nokogiri (~> 1.3) PLATFORMS ruby @@ -373,6 +382,7 @@ DEPENDENCIES blade blade-sauce_labs_plugin byebug + capybara (~> 2.7.0) coffee-rails dalli (>= 2.2.1) delayed_job diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a7a4aabc98..327852e75e 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add `ActionDispatch::SystemTestCase` to Action Pack + + Adds Capybara integration directly into Rails through Action Pack! + + See PR [#26703](https://github.com/rails/rails/pull/26703) + + *Eileen M. Uchitelle* + * Remove deprecated `.to_prepare`, `.to_cleanup`, `.prepare!` and `.cleanup!` from `ActionDispatch::Reloader`. *Rafael Mendonça França* diff --git a/actionpack/lib/abstract_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index 13fa2b393d..c85b4adba1 100644 --- a/actionpack/lib/abstract_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -136,7 +136,7 @@ module AbstractController def instrument_fragment_cache(name, key) # :nodoc: payload = instrument_payload(key) - ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}".freeze, payload) { yield } + ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield } end end end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 4dfcf4da28..a349841082 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -56,7 +56,7 @@ module ActionController self.status = _extract_redirect_to_status(options, response_status) self.location = _compute_redirect_to_location(request, options) - self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" + self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>" end # Redirects the browser to the page that issued the request (the referrer) diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 028177ace2..303790e96d 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -97,6 +97,8 @@ module ActionDispatch autoload :TestResponse autoload :AssertionResponse end + + autoload :SystemTestCase, "action_dispatch/system_test_case" end autoload :Mime, "action_dispatch/http/mime_type" diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb new file mode 100644 index 0000000000..59faf63ce3 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -0,0 +1,119 @@ +require "capybara/dsl" +require "action_controller" +require "action_dispatch/system_testing/driver" +require "action_dispatch/system_testing/server" +require "action_dispatch/system_testing/browser" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "action_dispatch/system_testing/test_helpers/setup_and_teardown" + +module ActionDispatch + class SystemTestCase < IntegrationTest + # = System Testing + # + # System tests let you test applications in the browser. Because system + # tests use a real browser experience you can test all of your JavaScript + # easily from your test suite. + # + # To create a system test in your application, extend your test class + # from <tt>ApplicationSystemTestCase</tt>. System tests use Capybara as a + # base and allow you to configure the settings through your + # <tt>application_system_test_case.rb</tt> file that is generated with a new + # application or scaffold. + # + # Here is an example system test: + # + # require 'application_system_test_case' + # + # class Users::CreateTest < ApplicationSystemTestCase + # test "adding a new user" do + # visit users_path + # click_on 'New User' + # + # fill_in 'Name', with: 'Arya' + # click_on 'Create User' + # + # assert_text 'Arya' + # end + # end + # + # When generating an application or scaffold, an +application_system_test_case.rb+ + # file will also be generated containing the base class for system testing. + # This is where you can change the driver, add Capybara settings, and other + # configuration for your system tests. + # + # require "test_helper" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :selenium, using: :chrome, screen_size: [1400, 1400] + # end + # + # By default, <tt>ActionDispatch::SystemTestCase</tt> is driven by the + # Selenium driver, with the Chrome browser, and a browser size of 1400x1400. + # + # Changing the driver configuration options are easy. Let's say you want to use + # the Firefox browser instead of Chrome. In your +application_system_test_case.rb+ + # file add the following: + # + # require "test_helper" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :selenium, using: :firefox + # end + # + # +driven_by+ has a required argument for the driver name. The keyword + # arguments are +:using+ for the browser and +:screen_size+ to change the + # size of the browser screen. These two options are not applicable for + # headless drivers and will be silently ignored if passed. + # + # To use a headless driver, like Poltergeist, update your Gemfile to use + # Poltergeist instead of Selenium and then declare the driver name in the + # +application_system_test_case.rb+ file. In this case you would leave out the +:using+ + # option because the driver is headless. + # + # require "test_helper" + # require "capybara/poltergeist" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :poltergeist + # end + # + # Because <tt>ActionDispatch::SystemTestCase</tt> is a shim between Capybara + # and Rails, any driver that is supported by Capybara is supported by system + # tests as long as you include the required gems and files. + include Capybara::DSL + include SystemTesting::TestHelpers::SetupAndTeardown + include SystemTesting::TestHelpers::ScreenshotHelper + + def self.start_application # :nodoc: + Capybara.app = Rack::Builder.new do + map "/" do + run Rails.application + end + end + end + + # System Test configuration options + # + # The default settings are Selenium, using Chrome, with a screen size + # of 1400x1400. + # + # Examples: + # + # driven_by :poltergeist + # + # driven_by :selenium, using: :firefox + # + # driven_by :selenium, screen_size: [800, 800] + def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400]) + SystemTesting::Driver.new(driver).run + SystemTesting::Server.new.run + SystemTesting::Browser.new(using, screen_size).run if selenium?(driver) + end + + def self.selenium?(driver) # :nodoc: + driver == :selenium + end + end + + SystemTestCase.start_application +end diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb new file mode 100644 index 0000000000..c9a6628516 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -0,0 +1,28 @@ +module ActionDispatch + module SystemTesting + class Browser # :nodoc: + def initialize(name, screen_size) + @name = name + @screen_size = screen_size + end + + def run + register + setup + end + + private + def register + Capybara.register_driver @name do |app| + Capybara::Selenium::Driver.new(app, browser: @name).tap do |driver| + driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) + end + end + end + + def setup + Capybara.default_driver = @name.to_sym + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb new file mode 100644 index 0000000000..7c2ad84e19 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -0,0 +1,18 @@ +module ActionDispatch + module SystemTesting + class Driver # :nodoc: + def initialize(name) + @name = name + end + + def run + register + end + + private + def register + Capybara.default_driver = @name + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb new file mode 100644 index 0000000000..4a214ef713 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -0,0 +1,32 @@ +require "rack/handler/puma" + +module ActionDispatch + module SystemTesting + class Server # :nodoc: + def run + register + setup + end + + private + def register + Capybara.register_server :rails_puma do |app, port, host| + Rack::Handler::Puma.run(app, Port: port, Threads: "0:1") + end + end + + def setup + set_server + set_port + end + + def set_server + Capybara.server = :rails_puma + end + + def set_port + Capybara.always_include_port = true + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb new file mode 100644 index 0000000000..784005cb93 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -0,0 +1,57 @@ +module ActionDispatch + module SystemTesting + module TestHelpers + # Screenshot helper for system testing + module ScreenshotHelper + # Takes a screenshot of the current page in the browser. + # + # +take_screenshot+ can be used at any point in your system tests to take + # a screenshot of the current state. This can be useful for debugging or + # automating visual testing. + def take_screenshot + save_image + puts "[Screenshot]: #{image_path}" + puts display_image + end + + # Takes a screenshot of the current page in the browser if the test + # failed. + # + # +take_failed_screenshot+ is included in <tt>application_system_test_case.rb</tt> + # that is generated with the application. To take screenshots when a test + # fails add +take_failed_screenshot+ to the teardown block before clearing + # sessions. + def take_failed_screenshot + take_screenshot unless passed? + end + + private + def image_name + passed? ? method_name : "failures_#{method_name}" + end + + def image_path + "tmp/screenshots/#{image_name}.png" + end + + def save_image + page.save_screenshot(Rails.root.join(image_path)) + end + + def display_image + if ENV["CAPYBARA_INLINE_SCREENSHOT"] == "artifact" + "\e]1338;url=artifact://#{image_path}\a" + else + name = inline_base64(File.basename(image_path)) + image = inline_base64(File.read(image_path)) + "\e]1337;File=name=#{name};height=400px;inline=1:#{image}\a" + end + end + + def inline_base64(path) + Base64.encode64(path).gsub("\n", "") + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb new file mode 100644 index 0000000000..491559eedf --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -0,0 +1,20 @@ +module ActionDispatch + module SystemTesting + module TestHelpers + module SetupAndTeardown # :nodoc: + DEFAULT_HOST = "127.0.0.1" + + def before_setup + host! DEFAULT_HOST + super + end + + def after_teardown + super + take_failed_screenshot + Capybara.reset_sessions! + end + end + end + end +end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index e4e968dfdb..f06a1f4d23 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -21,8 +21,8 @@ end class RedirectController < ActionController::Base # empty method not used anywhere to ensure methods like # `status` and `location` aren't called on `redirect_to` calls - def status; render plain: "called status"; end - def location; render plain: "called location"; end + def status; raise "Should not be called!"; end + def location; raise "Should not be called!"; end def simple_redirect redirect_to action: "hello_world" diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index e084e45373..891ce0e905 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -134,7 +134,7 @@ XML end def create - head :created, location: "created resource" + head :created, location: "/resource" end def render_cookie @@ -893,12 +893,12 @@ XML assert_response :created # Redirect url doesn't care that it wasn't a :redirect response. - assert_equal "created resource", @response.redirect_url + assert_equal "/resource", @response.redirect_url assert_equal @response.redirect_url, redirect_to_url # Must be a :redirect response. assert_raise(ActiveSupport::TestCase::Assertion) do - assert_redirected_to "created resource" + assert_redirected_to "/resource" end end diff --git a/actionpack/test/dispatch/system_testing/browser_test.rb b/actionpack/test/dispatch/system_testing/browser_test.rb new file mode 100644 index 0000000000..b0ad309492 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/browser_test.rb @@ -0,0 +1,10 @@ +require "abstract_unit" +require "action_dispatch/system_testing/browser" + +class BrowserTest < ActiveSupport::TestCase + test "initializing the browser" do + browser = ActionDispatch::SystemTesting::Browser.new(:chrome, [ 1400, 1400 ]) + assert_equal :chrome, browser.instance_variable_get(:@name) + assert_equal [ 1400, 1400 ], browser.instance_variable_get(:@screen_size) + end +end diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb new file mode 100644 index 0000000000..f0ebdb38db --- /dev/null +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -0,0 +1,9 @@ +require "abstract_unit" +require "action_dispatch/system_testing/driver" + +class DriverTest < ActiveSupport::TestCase + test "initializing the driver" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium) + assert_equal :selenium, driver.instance_variable_get(:@name) + end +end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb new file mode 100644 index 0000000000..8c14f799b0 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -0,0 +1,18 @@ +require "abstract_unit" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" + +class ScreenshotHelperTest < ActiveSupport::TestCase + test "image path is saved in tmp directory" do + new_test = ActionDispatch::SystemTestCase.new("x") + + assert_equal "tmp/screenshots/x.png", new_test.send(:image_path) + end + + test "image path includes failures text if test did not pass" do + new_test = ActionDispatch::SystemTestCase.new("x") + + new_test.stub :passed?, false do + assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path) + end + end +end diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb new file mode 100644 index 0000000000..10412d6367 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/server_test.rb @@ -0,0 +1,17 @@ +require "abstract_unit" +require "capybara/dsl" +require "action_dispatch/system_testing/server" + +class ServerTest < ActiveSupport::TestCase + setup do + ActionDispatch::SystemTesting::Server.new.run + end + + test "initializing the server port" do + assert_includes Capybara.servers, :rails_puma + end + + test "port is always included" do + assert Capybara.always_include_port, "expected Capybara.always_include_port to be true" + end +end diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb new file mode 100644 index 0000000000..a384902a14 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -0,0 +1,21 @@ +require "abstract_unit" + +class SystemTestCaseTest < ActiveSupport::TestCase + test "driven_by sets Capybara's default driver to poltergeist" do + ActionDispatch::SystemTestCase.driven_by :poltergeist + + assert_equal :poltergeist, Capybara.default_driver + end + + test "driven_by sets Capybara's drivers respectively" do + ActionDispatch::SystemTestCase.driven_by :selenium, using: :chrome + + assert_includes Capybara.drivers, :selenium + assert_includes Capybara.drivers, :chrome + assert_equal :chrome, Capybara.default_driver + end + + test "selenium? returns false if driver is poltergeist" do + assert_not ActionDispatch::SystemTestCase.selenium?(:poltergeist) + end +end diff --git a/actionview/app/assets/javascripts/config.coffee b/actionview/app/assets/javascripts/config.coffee index 3d4706b0e1..a93325e903 100644 --- a/actionview/app/assets/javascripts/config.coffee +++ b/actionview/app/assets/javascripts/config.coffee @@ -1,21 +1,21 @@ #= export Rails @Rails = - # Link elements bound by jquery-ujs + # Link elements bound by rails-ujs linkClickSelector: 'a[data-confirm], a[data-method], a[data-remote]:not([disabled]), a[data-disable-with], a[data-disable]' - # Button elements bound by jquery-ujs + # Button elements bound by rails-ujs buttonClickSelector: selector: 'button[data-remote]:not([form]), button[data-confirm]:not([form])' exclude: 'form button' - # Select elements bound by jquery-ujs + # Select elements bound by rails-ujs inputChangeSelector: 'select[data-remote], input[data-remote], textarea[data-remote]' - # Form elements bound by jquery-ujs + # Form elements bound by rails-ujs formSubmitSelector: 'form' - # Form input elements bound by jquery-ujs + # Form input elements bound by rails-ujs formInputClickSelector: 'form input[type=submit], form input[type=image], form button[type=submit], form button:not([type]), input[type=submit][form], input[type=image][form], button[type=submit][form], button[form]:not([type])' # Form input elements disabled during form submission @@ -24,9 +24,6 @@ # Form input elements re-enabled after form submission formEnableSelector: 'input[data-disable-with]:disabled, button[data-disable-with]:disabled, textarea[data-disable-with]:disabled, input[data-disable]:disabled, button[data-disable]:disabled, textarea[data-disable]:disabled' - # Form required input elements - requiredInputSelector: 'input[name][required]:not([disabled]), textarea[name][required]:not([disabled])' - # Form file input elements fileInputSelector: 'input[name][type=file]:not([disabled])' diff --git a/actionview/app/assets/javascripts/features/remote.coffee b/actionview/app/assets/javascripts/features/remote.coffee index 30a5dc21fa..852587042c 100644 --- a/actionview/app/assets/javascripts/features/remote.coffee +++ b/actionview/app/assets/javascripts/features/remote.coffee @@ -4,7 +4,7 @@ matches, getData, setData fire, stopEverything ajax, isCrossDomain - blankInputs, serializeElement + serializeElement } = Rails # Checks "data-remote" if true to handle the request through a XHR request. @@ -71,16 +71,6 @@ Rails.handleRemote = (e) -> ) stopEverything(e) -# Check whether any required fields are empty -# In both ajax mode and normal mode -Rails.validateForm = (e) -> - form = this - return if form.noValidate or getData(form, 'ujs:formnovalidate-button') - # Skip other logic when required values are missing or file upload is present - blankRequiredInputs = blankInputs(form, Rails.requiredInputSelector, false) - if blankRequiredInputs.length > 0 and fire(form, 'ajax:aborted:required', [blankRequiredInputs]) - stopEverything(e) - Rails.formSubmitButtonClick = (e) -> button = this form = button.form diff --git a/actionview/app/assets/javascripts/rails-ujs.coffee b/actionview/app/assets/javascripts/rails-ujs.coffee index f96d2eb6fd..df889ce067 100644 --- a/actionview/app/assets/javascripts/rails-ujs.coffee +++ b/actionview/app/assets/javascripts/rails-ujs.coffee @@ -14,7 +14,7 @@ refreshCSRFTokens, CSRFProtection enableElement, disableElement handleConfirm - handleRemote, validateForm, formSubmitButtonClick, handleMetaClick + handleRemote, formSubmitButtonClick, handleMetaClick handleMethod } = Rails @@ -25,9 +25,9 @@ if jQuery? and not jQuery.rails CSRFProtection(xhr) unless options.crossDomain Rails.start = -> - # Cut down on the number of issues from people inadvertently including jquery_ujs twice - # by detecting and raising an error when it happens. - throw new Error('jquery-ujs has already been loaded!') if window._rails_loaded + # Cut down on the number of issues from people inadvertently including + # rails-ujs twice by detecting and raising an error when it happens. + throw new Error('rails-ujs has already been loaded!') if window._rails_loaded # This event works the same as the load event, except that it fires every # time the page is loaded. @@ -58,7 +58,6 @@ Rails.start = -> delegate document, Rails.inputChangeSelector, 'change', handleRemote delegate document, Rails.formSubmitSelector, 'submit', handleConfirm - delegate document, Rails.formSubmitSelector, 'submit', validateForm delegate document, Rails.formSubmitSelector, 'submit', handleRemote # Normal mode submit # Slight timeout so that the submit button gets properly serialized diff --git a/actionview/app/assets/javascripts/utils/event.coffee b/actionview/app/assets/javascripts/utils/event.coffee index d25fe8e546..8d3ff007ea 100644 --- a/actionview/app/assets/javascripts/utils/event.coffee +++ b/actionview/app/assets/javascripts/utils/event.coffee @@ -6,7 +6,7 @@ # https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent#Polyfill CustomEvent = window.CustomEvent -if typeof CustomEvent is 'function' +if typeof CustomEvent isnt 'function' CustomEvent = (event, params) -> evt = document.createEvent('CustomEvent') evt.initCustomEvent(event, params.bubbles, params.cancelable, params.detail) diff --git a/actionview/app/assets/javascripts/utils/form.coffee b/actionview/app/assets/javascripts/utils/form.coffee index 251113deda..5fa337b518 100644 --- a/actionview/app/assets/javascripts/utils/form.coffee +++ b/actionview/app/assets/javascripts/utils/form.coffee @@ -14,7 +14,7 @@ Rails.serializeElement = (element, additionalParam) -> if matches(input, 'select') toArray(input.options).forEach (option) -> params.push(name: input.name, value: option.value) if option.selected - else if input.type isnt 'radio' and input.type isnt 'checkbox' or input.checked + else if input.checked or ['radio', 'checkbox', 'submit'].indexOf(input.type) == -1 params.push(name: input.name, value: input.value) params.push(additionalParam) if additionalParam @@ -34,28 +34,3 @@ Rails.formElements = (form, selector) -> toArray(form.elements).filter (el) -> matches(el, selector) else toArray(form.querySelectorAll(selector)) - -# Helper function which checks for blank inputs in a form that match the specified CSS selector -Rails.blankInputs = (form, selector, nonBlank) -> - foundInputs = [] - requiredInputs = toArray(form.querySelectorAll(selector or 'input, textarea')) - checkedRadioButtonNames = {} - - requiredInputs.forEach (input) -> - if input.type is 'radio' - # Don't count unchecked required radio as blank if other radio with same name is checked, - # regardless of whether same-name radio input has required attribute or not. The spec - # states https://www.w3.org/TR/html5/forms.html#the-required-attribute - radioName = input.name - # Skip if we've already seen the radio with this name. - unless checkedRadioButtonNames[radioName] - # If none checked - if form.querySelectorAll("input[type=radio][name='#{radioName}']:checked").length == 0 - radios = form.querySelectorAll("input[type=radio][name='#{radioName}']") - foundInputs = foundInputs.concat(toArray(radios)) - # We only need to check each name once. - checkedRadioButtonNames[radioName] = radioName - else - valueToCheck = if input.type is 'checkbox' then input.checked else !!input.value - foundInputs.push(input) if valueToCheck is nonBlank - foundInputs diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index c067031d2d..b0e2f1e54e 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -345,7 +345,7 @@ module ActionView end def instrument(action, &block) # :doc: - ActiveSupport::Notifications.instrument("#{action}.action_view".freeze, instrument_payload, &block) + ActiveSupport::Notifications.instrument("#{action}.action_view", instrument_payload, &block) end def instrument_render_template(&block) diff --git a/actionview/test/activerecord/polymorphic_routes_test.rb b/actionview/test/activerecord/polymorphic_routes_test.rb index dcad0b424c..e99c769dc2 100644 --- a/actionview/test/activerecord/polymorphic_routes_test.rb +++ b/actionview/test/activerecord/polymorphic_routes_test.rb @@ -86,8 +86,7 @@ class PolymorphicRoutesTest < ActionController::TestCase def test_string with_test_routes do - # FIXME: why are these different? Symbol case passes through to - # `polymorphic_url`, but the String case doesn't. + assert_equal "/projects", polymorphic_path("projects") assert_equal "http://example.com/projects", polymorphic_url("projects") assert_equal "projects", url_for("projects") end diff --git a/actionview/test/template/erb/tag_helper_test.rb b/actionview/test/template/erb/tag_helper_test.rb index 84e328d8be..233f37c48a 100644 --- a/actionview/test/template/erb/tag_helper_test.rb +++ b/actionview/test/template/erb/tag_helper_test.rb @@ -18,8 +18,8 @@ module ERBTest end test "percent equals works with form tags" do - expected_output = %r{<form.*action="foo".*method="post">.*hello*</form>} - assert_match expected_output, render_content("form_tag('foo')", "<%= 'hello' %>") + expected_output = %r{<form.*action="/foo".*method="post">.*hello*</form>} + assert_match expected_output, render_content("form_tag('/foo')", "<%= 'hello' %>") end test "percent equals works with fieldset tags" do diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb index 490df5e4d9..b3a180b28a 100644 --- a/actionview/test/template/form_helper_test.rb +++ b/actionview/test/template/form_helper_test.rb @@ -257,11 +257,11 @@ class FormHelperTest < ActionView::TestCase end def test_label_with_non_active_record_object - form_for(OpenStruct.new(name: "ok"), as: "person", url: "an_url", html: { id: "create-person" }) do |f| + form_for(OpenStruct.new(name: "ok"), as: "person", url: "/an", html: { id: "create-person" }) do |f| f.label(:name) end - expected = whole_form("an_url", "create-person", "new_person", method: "post") do + expected = whole_form("/an", "create-person", "new_person", method: "post") do '<label for="person_name">Name</label>' end diff --git a/actionview/test/ujs/public/test/call-remote-callbacks.js b/actionview/test/ujs/public/test/call-remote-callbacks.js index 082d10bfbd..707e21541d 100644 --- a/actionview/test/ujs/public/test/call-remote-callbacks.js +++ b/actionview/test/ujs/public/test/call-remote-callbacks.js @@ -108,202 +108,6 @@ asyncTest('stopping the "ajax:beforeSend" event aborts the request', 1, function }) }) -asyncTest('blank required form input field should abort request and trigger "ajax:aborted:required" event', 5, function() { - $(document).bind('iframe:loading', function() { - ok(false, 'form should not get submitted') - }) - - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .append($('<textarea name="user_bio" required="required"></textarea>')) - .bindNative('ajax:beforeSend', function() { - ok(false, 'ajax:beforeSend should not run') - }) - .bindNative('ajax:aborted:required', function(e, data) { - data = $(data) - ok(data.length == 2, 'ajax:aborted:required event is passed all blank required inputs (jQuery objects)') - ok(data.first().is('input[name="user_name"]'), 'ajax:aborted:required adds blank required input to data') - ok(data.last().is('textarea[name="user_bio"]'), 'ajax:aborted:required adds blank required textarea to data') - ok(true, 'ajax:aborted:required should run') - }) - .triggerNative('submit') - - setTimeout(function() { - form.find('input[required],textarea[required]').val('Tyler') - form.unbind('ajax:beforeSend') - submit() - }, 13) -}) - -asyncTest('blank required form input for non-remote form should abort normal submission', 1, function() { - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .removeAttr('data-remote') - .bindNative('ujs:everythingStopped', function() { - ok(true, 'ujs:everythingStopped should run') - }) - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('form should be submitted with blank required fields if handler is bound to "ajax:aborted:required" event that returns false', 1, function() { - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .bindNative('ajax:beforeSend', function() { - ok(true, 'ajax:beforeSend should run') - }) - .bindNative('ajax:aborted:required', function() { - return false - }) - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('disabled fields should not be included in blank required check', 2, function() { - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required" disabled="disabled">')) - .append($('<textarea name="user_bio" required="required" disabled="disabled"></textarea>')) - .bindNative('ajax:beforeSend', function() { - ok(true, 'ajax:beforeSend should run') - }) - .bindNative('ajax:aborted:required', function() { - ok(false, 'ajax:aborted:required should not run') - }) - - submit() -}) - -asyncTest('form should be submitted with blank required fields if it has the "novalidate" attribute', 2, function() { - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .attr('novalidate', 'novalidate') - .bindNative('ajax:beforeSend', function() { - ok(true, 'ajax:beforeSend should run') - }) - .bindNative('ajax:aborted:required', function() { - ok(false, 'ajax:aborted:required should not run') - }) - - submit() -}) - -asyncTest('form should be submitted with blank required fields if the button has the "formnovalidate" attribute', 2, function() { - var submit_button = $('<input type="submit" formnovalidate>') - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .append(submit_button) - .bindNative('ajax:beforeSend', function() { - ok(true, 'ajax:beforeSend should run') - }) - .bindNative('ajax:aborted:required', function() { - ok(false, 'ajax:aborted:required should not run') - }) - - submit_with_button(submit_button) -}) - -asyncTest('blank required form input for non-remote form with "novalidate" attribute should not abort normal submission', 1, function() { - $(document).bind('iframe:loading', function() { - ok(true, 'form should get submitted') - }) - - var form = $('form[data-remote]') - .append($('<input type="text" name="user_name" required="required">')) - .removeAttr('data-remote') - .attr('novalidate', 'novalidate') - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('unchecked required checkbox should abort form submission', 1, function() { - var form = $('form[data-remote]') - .append($('<input type="checkbox" name="agree" required="required">')) - .removeAttr('data-remote') - .bindNative('ujs:everythingStopped', function() { - ok(true, 'ujs:everythingStopped should run') - }) - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('unchecked required radio should abort form submission', 3, function() { - var form = $('form[data-remote]') - .append($('<input type="radio" name="yes_no_none" required="required" value=1>')) - .append($('<input type="radio" name="yes_no_none" required="required" value=2>')) - .removeAttr('data-remote') - .bindNative('ujs:everythingStopped', function() { - ok(true, 'ujs:everythingStopped should run') - }) - .bindNative('ajax:aborted:required', function(e, data) { - data = $(data) - equal(data.length, 2, 'blankRequiredInputs should include both radios') - ok(data.first().is('input[type=radio][value=1]'), 'blankRequiredInputs[0] should be the first radio') - }) - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('required radio should only require one to be checked', 1, function() { - $(document).bind('iframe:loading', function() { - ok(true, 'form should get submitted') - }) - - var form = $('form[data-remote]') - .append($('<input type="radio" name="yes_no" required="required" value=1 id="checkme">')) - .append($('<input type="radio" name="yes_no" required="required" value=2>')) - .removeAttr('data-remote') - .bindNative('ujs:everythingStopped', function() { - ok(false, 'ujs:everythingStopped should not run') - }) - .find('#checkme').prop('checked', true) - .end() - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - -asyncTest('required radio should only require one to be checked if not all radios are required', 1, function() { - $(document).bind('iframe:loading', function() { - ok(true, 'form should get submitted') - }) - - var form = $('form[data-remote]') - // Check the radio that is not required - .append($('<input type="radio" name="yes_no_maybe" value=1 >')) - // Check the radio that is not required - .append($('<input type="radio" name="yes_no_maybe" value=2 id="checkme">')) - // Only one needs to be required - .append($('<input type="radio" name="yes_no_maybe" required="required" value=3>')) - .removeAttr('data-remote') - .bindNative('ujs:everythingStopped', function() { - ok(false, 'ujs:everythingStopped should not run') - }) - .find('#checkme').prop('checked', true) - .end() - .triggerNative('submit') - - setTimeout(function() { - start() - }, 13) -}) - function skipIt() { // This test cannot work due to the security feature in browsers which makes the value // attribute of file input fields readonly, so it cannot be set with default value. diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index a51aa10417..b756add24e 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -73,7 +73,6 @@ asyncTest('clicking on a link with data-remote attribute', 5, function() { .bindNative('ajax:success', function(e, data, status, xhr) { App.assertCallbackInvoked('ajax:success') App.assertRequestPath(data, '/echo') - console.log(data.params) equal(data.params.data1, 'value1', 'ajax arguments should have key data1 with right value') equal(data.params.data2, 'value2', 'ajax arguments should have key data2 with right value') App.assertGetRequest(data) @@ -398,3 +397,19 @@ asyncTest('form should be serialized correctly', 6, function() { }) .triggerNative('submit') }) + +asyncTest('form buttons should only be serialized when clicked', 4, function() { + $('form') + .append('<input type="submit" name="submit1" value="submit1" />') + .append('<button name="submit2" value="submit2" />') + .append('<button name="submit3" value="submit3" />') + .bindNative('ajax:success', function(e, data, status, xhr) { + equal(data.params.submit1, undefined) + equal(data.params.submit2, 'submit2') + equal(data.params.submit3, undefined) + equal(data['rack.request.form_vars'], 'user_name=john&submit2=submit2') + + start() + }) + .find('[name=submit2]').triggerNative('click') +}) diff --git a/actionview/test/ujs/public/test/override.js b/actionview/test/ujs/public/test/override.js index be6ec7749b..299c7018cc 100644 --- a/actionview/test/ujs/public/test/override.js +++ b/actionview/test/ujs/public/test/override.js @@ -46,7 +46,7 @@ asyncTest('the event selector strings are overridable', 1, function() { start() }) -asyncTest('including jquery-ujs multiple times throws error', 1, function() { +asyncTest('including rails-ujs multiple times throws error', 1, function() { throws(function() { Rails.start() }, 'appending rails.js again throws error') diff --git a/actionview/test/ujs/public/test/settings.js b/actionview/test/ujs/public/test/settings.js index c68ca24d6a..299c71bb00 100644 --- a/actionview/test/ujs/public/test/settings.js +++ b/actionview/test/ujs/public/test/settings.js @@ -63,12 +63,12 @@ $(document).bind('submit', function(e) { } }) -var MouseEvent = window.MouseEvent +var _MouseEvent = window.MouseEvent try { - new MouseEvent() + new _MouseEvent() } catch (e) { - MouseEvent = function(type, options) { + _MouseEvent = function(type, options) { var evt = document.createEvent('MouseEvents') evt.initMouseEvent(type, options.bubbles, options.cancelable, window, options.detail, 0, 0, 80, 20, options.ctrlKey, options.altKey, options.shiftKey, options.metaKey, 0, null) return evt @@ -81,7 +81,7 @@ $.fn.extend({ var el = this[0], event, Evt = { - 'click': MouseEvent, + 'click': _MouseEvent, 'change': Event, 'pageshow': PageTransitionEvent, 'submit': Event diff --git a/actionview/test/ujs/views/layouts/application.html.erb b/actionview/test/ujs/views/layouts/application.html.erb index 74fa3bd06d..e09b213b72 100644 --- a/actionview/test/ujs/views/layouts/application.html.erb +++ b/actionview/test/ujs/views/layouts/application.html.erb @@ -21,7 +21,7 @@ <%= script_tag jquery_src %> <script> // This is for test in override.js. - // Must go after jQuery is loaded, but before jquery-ujs. + // Must go before rails-ujs. $(document).bind('rails:attachBindings', function() { $.rails.linkClickSelector += ', a[data-custom-remote-link]'; // Hijacks link click before ujs binds any handlers diff --git a/actionview/test/ujs/views/tests/index.html.erb b/actionview/test/ujs/views/tests/index.html.erb index 393a5ee235..2ac44eeb81 100644 --- a/actionview/test/ujs/views/tests/index.html.erb +++ b/actionview/test/ujs/views/tests/index.html.erb @@ -1,4 +1,4 @@ -<% @title = "jquery-ujs test" %> +<% @title = "rails-ujs test" %> <%= test_to 'data-confirm', 'data-remote', 'data-disable', 'data-disable-with', 'call-remote', 'call-remote-callbacks', 'data-method', 'override', 'csrf-refresh', 'csrf-token' %> diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 4bfc402069..30a9ef472d 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -63,27 +63,27 @@ module ActiveModel private - def is_number?(raw_value) # :doc: + def is_number?(raw_value) !parse_raw_value_as_a_number(raw_value).nil? rescue ArgumentError, TypeError false end - def parse_raw_value_as_a_number(raw_value) # :doc: + def parse_raw_value_as_a_number(raw_value) Kernel.Float(raw_value) if raw_value !~ /\A0[xX]/ end - def is_integer?(raw_value) # :doc: + def is_integer?(raw_value) /\A[+-]?\d+\z/ === raw_value.to_s end - def filtered_options(value) # :doc: + def filtered_options(value) filtered = options.except(*RESERVED_OPTIONS) filtered[:value] = value filtered end - def allow_only_integer?(record) # :doc: + def allow_only_integer?(record) case options[:only_integer] when Symbol record.send(options[:only_integer]) diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 84d0493a60..1cb2b2d7c6 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -83,7 +83,7 @@ module ActiveRecord end def scope - target_scope.merge(association_scope) + target_scope.merge!(association_scope) end # The scope for this association. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0437a79b84..77282e6463 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -30,13 +30,7 @@ module ActiveRecord reload end - if null_scope? - # Cache the proxy separately before the owner has an id - # or else a post-save proxy will still lack the id - @null_proxy ||= CollectionProxy.create(klass, self) - else - @proxy ||= CollectionProxy.create(klass, self) - end + CollectionProxy.create(klass, self) end # Implements the writer method, e.g. foo.items= for Foo.has_many :items @@ -315,9 +309,9 @@ module ActiveRecord record end - def scope(opts = {}) - scope = super() - scope.none! if opts.fetch(:nullify, true) && null_scope? + def scope + scope = super + scope.none! if null_scope? scope end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 0d84805b4d..55bf2e0ff0 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -28,12 +28,9 @@ module ActiveRecord # is computed directly through SQL and does not trigger by itself the # instantiation of the actual post records. class CollectionProxy < Relation - delegate :exists?, :update_all, :arel, to: :scope - def initialize(klass, association) #:nodoc: @association = association super klass, klass.arel_table, klass.predicate_builder - merge! association.scope(nullify: false) end def target @@ -956,19 +953,10 @@ module ActiveRecord @association end - # We don't want this object to be put on the scoping stack, because - # that could create an infinite loop where we call an @association - # method, which gets the current scope, which is this object, which - # delegates to @association, and so on. - def scoping - @association.scope.scoping { yield } - end - # Returns a <tt>Relation</tt> object for the records in this association def scope - @association.scope + @scope ||= @association.scope end - alias spawn scope # Equivalent to <tt>Array#==</tt>. Returns +true+ if the two arrays # contain the same number of elements and if each element is equal @@ -1100,6 +1088,7 @@ module ActiveRecord # person.pets(true) # fetches pets from the database # # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>] def reload + @scope = nil proxy_association.reload self end @@ -1121,11 +1110,21 @@ module ActiveRecord # person.pets # fetches pets from the database # # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>] def reset + @scope = nil proxy_association.reset proxy_association.reset_scope self end + delegate_methods = [ + QueryMethods, + SpawnMethods, + ].flat_map { |klass| + klass.public_instance_methods(false) + } - self.public_instance_methods(false) + [:scoping] + + delegate(*delegate_methods, to: :scope) + private def find_nth_with_limit(index, limit) @@ -1149,6 +1148,18 @@ module ActiveRecord def exec_queries load_target end + + def respond_to_missing?(method, _) + scope.respond_to?(method) || super + end + + def method_missing(method, *args, &block) + if scope.respond_to?(method) + scope.public_send(method, *args, &block) + else + super + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index ce4721c99d..3f2e86a98d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -353,6 +353,16 @@ module ActiveRecord @threads_blocking_new_connections = 0 @available = ConnectionLeasingQueue.new self + + @lock_thread = false + end + + def lock_thread=(lock_thread) + if lock_thread + @lock_thread = Thread.current + else + @lock_thread = nil + end end # Retrieve the connection associated with the current thread, or call @@ -361,7 +371,7 @@ module ActiveRecord # #connection can be called any number of times; the connection is # held in a cache keyed by a thread. def connection - @thread_cached_conns[connection_cache_key(Thread.current)] ||= checkout + @thread_cached_conns[connection_cache_key(@lock_thread || Thread.current)] ||= checkout end # Returns true if there is an open connection being used for the current thread. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 7eab7de5d3..e53ba4e666 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -83,7 +83,9 @@ module ActiveRecord # the same SQL query and repeatedly return the same result each time, silently # undermining the randomness you were expecting. def clear_query_cache - @query_cache.clear + @lock.synchronize do + @query_cache.clear + end end def select_all(arel, name = nil, binds = [], preparable: nil) @@ -99,21 +101,23 @@ module ActiveRecord private def cache_sql(sql, name, binds) - result = - if @query_cache[sql].key?(binds) - ActiveSupport::Notifications.instrument( - "sql.active_record", - sql: sql, - binds: binds, - name: name, - connection_id: object_id, - cached: true, - ) - @query_cache[sql][binds] - else - @query_cache[sql][binds] = yield - end - result.dup + @lock.synchronize do + result = + if @query_cache[sql].key?(binds) + ActiveSupport::Notifications.instrument( + "sql.active_record", + sql: sql, + binds: binds, + name: name, + connection_id: object_id, + cached: true, + ) + @query_cache[sql][binds] + else + @query_cache[sql][binds] = yield + end + result.dup + end end # If arel is locked this is a SELECT ... FOR UPDATE or somesuch. Such diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index bdcdfe4982..3686ad8b54 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -273,8 +273,8 @@ module ActiveRecord yield td if block_given? - if options[:force] && data_source_exists?(table_name) - drop_table(table_name, options) + if options[:force] + drop_table(table_name, **options, if_exists: true) end result = execute schema_creation.accept td diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 6b14a498df..b31ce0a181 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -107,6 +107,7 @@ module ActiveRecord @schema_cache = SchemaCache.new self @quoted_column_names, @quoted_table_names = {}, {} @visitor = arel_visitor + @lock = Monitor.new if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true @@ -605,7 +606,11 @@ module ActiveRecord binds: binds, type_casted_binds: type_casted_binds, statement_name: statement_name, - connection_id: object_id) { yield } + connection_id: object_id) do + @lock.synchronize do + yield + end + end rescue => e raise translate_exception_class(e, sql) end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 36c9815547..c89e29ba44 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -236,7 +236,9 @@ module ActiveRecord # Clears the prepared statements cache. def clear_cache! - @statements.clear + @lock.synchronize do + @statements.clear + end end def truncate(table_name, name = nil) @@ -637,8 +639,10 @@ module ActiveRecord if in_transaction? raise ActiveRecord::PreparedStatementCacheExpired.new(e.cause.message) else - # outside of transactions we can simply flush this query and retry - @statements.delete sql_key(sql) + @lock.synchronize do + # outside of transactions we can simply flush this query and retry + @statements.delete sql_key(sql) + end retry end end @@ -674,19 +678,21 @@ module ActiveRecord # Prepare the statement if it hasn't been prepared, return # the statement key. def prepare_statement(sql) - sql_key = sql_key(sql) - unless @statements.key? sql_key - nextkey = @statements.next_key - begin - @connection.prepare nextkey, sql - rescue => e - raise translate_exception_class(e, sql) + @lock.synchronize do + sql_key = sql_key(sql) + unless @statements.key? sql_key + nextkey = @statements.next_key + begin + @connection.prepare nextkey, sql + rescue => e + raise translate_exception_class(e, sql) + end + # Clear the queue + @connection.get_last_result + @statements[sql_key] = nextkey end - # Clear the queue - @connection.get_last_result - @statements[sql_key] = nextkey + @statements[sql_key] end - @statements[sql_key] end # Connects to a PostgreSQL server and sets up the adapter depending on the diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 91d8054ef2..e79167d568 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -970,6 +970,7 @@ module ActiveRecord @fixture_connections = enlist_fixture_connections @fixture_connections.each do |connection| connection.begin_transaction joinable: false + connection.pool.lock_thread = true end # When connections are established in the future, begin a transaction too @@ -985,6 +986,7 @@ module ActiveRecord if connection && !@fixture_connections.include?(connection) connection.begin_transaction joinable: false + connection.pool.lock_thread = true @fixture_connections << connection end end @@ -1007,6 +1009,7 @@ module ActiveRecord ActiveSupport::Notifications.unsubscribe(@connection_subscriber) if @connection_subscriber @fixture_connections.each do |connection| connection.rollback_transaction if connection.transaction_open? + connection.pool.lock_thread = false end @fixture_connections.clear else diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index cbecfa84ff..ede3a44090 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -611,21 +611,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_update_all_on_association_accessed_before_save firm = Firm.new(name: "Firm") - clients_proxy_id = firm.clients.object_id firm.clients << Client.first firm.save! assert_equal firm.clients.count, firm.clients.update_all(description: "Great!") - assert_not_equal clients_proxy_id, firm.clients.object_id end def test_update_all_on_association_accessed_before_save_with_explicit_foreign_key - # We can use the same cached proxy object because the id is available for the scope firm = Firm.new(name: "Firm", id: 100) - clients_proxy_id = firm.clients.object_id firm.clients << Client.first firm.save! assert_equal firm.clients.count, firm.clients.update_all(description: "Great!") - assert_equal clients_proxy_id, firm.clients.object_id end def test_belongs_to_sanity diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index a223b4338f..26056f6f63 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -220,11 +220,6 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_equal david.projects, david.projects.scope end - test "proxy object is cached" do - david = developers(:david) - assert david.projects.equal?(david.projects) - end - test "inverses get set of subsets of the association" do man = Man.create man.interests.create diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 61e596e208..afe761cb55 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -640,6 +640,8 @@ class TransactionalFixturesOnConnectionNotification < ActiveRecord::TestCase def test_transaction_created_on_connection_notification connection = stub(transaction_open?: false) connection.expects(:begin_transaction).with(joinable: false) + pool = connection.stubs(:pool).returns(ActiveRecord::ConnectionAdapters::ConnectionPool.new(ActiveRecord::Base.connection_pool.spec)) + pool.stubs(:lock_thread=).with(false) fire_connection_notification(connection) end @@ -647,12 +649,16 @@ class TransactionalFixturesOnConnectionNotification < ActiveRecord::TestCase # Mocha is not thread-safe so define our own stub to test connection = Class.new do attr_accessor :rollback_transaction_called + attr_accessor :pool def transaction_open?; true; end def begin_transaction(*args); end def rollback_transaction(*args) @rollback_transaction_called = true end end.new + connection.pool = Class.new do + def lock_thread=(lock_thread); false; end + end.new fire_connection_notification(connection) teardown_fixtures assert(connection.rollback_transaction_called, "Expected <mock connection>#rollback_transaction to be called but was not") diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index d8cf235000..494663eb04 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -532,4 +532,16 @@ class QueryCacheExpiryTest < ActiveRecord::TestCase end end end + + test "threads use the same connection" do + @connection_1 = ActiveRecord::Base.connection.object_id + + thread_a = Thread.new do + @connection_2 = ActiveRecord::Base.connection.object_id + end + + thread_a.join + + assert_equal @connection_1, @connection_2 + end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 3a04f4bf7d..14fb2fbbfa 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -10,6 +10,8 @@ require "concurrent/atomic/cyclic_barrier" class DefaultScopingTest < ActiveRecord::TestCase fixtures :developers, :posts, :comments + self.use_transactional_tests = false + def test_default_scope expected = Developer.all.merge!(order: "salary DESC").to_a.collect(&:salary) received = DeveloperOrderedBySalary.all.collect(&:salary) diff --git a/activesupport/lib/active_support/i18n_railtie.rb b/activesupport/lib/active_support/i18n_railtie.rb index b94368df14..b749913ee9 100644 --- a/activesupport/lib/active_support/i18n_railtie.rb +++ b/activesupport/lib/active_support/i18n_railtie.rb @@ -2,6 +2,8 @@ require "active_support" require "active_support/file_update_checker" require "active_support/core_ext/array/wrap" +# :enddoc: + module I18n class Railtie < Rails::Railtie config.i18n = ActiveSupport::OrderedOptions.new diff --git a/activesupport/lib/active_support/testing/autorun.rb b/activesupport/lib/active_support/testing/autorun.rb index 3108e3e549..a18788f38e 100644 --- a/activesupport/lib/active_support/testing/autorun.rb +++ b/activesupport/lib/active_support/testing/autorun.rb @@ -2,8 +2,8 @@ gem "minitest" require "minitest" -if Minitest.respond_to?(:run_via) && !Minitest.run_via[:rails] - Minitest.run_via[:ruby] = true +if Minitest.respond_to?(:run_via) && !Minitest.run_via.set? + Minitest.run_via = :ruby end Minitest.autorun diff --git a/activesupport/lib/active_support/xml_mini/libxml.rb b/activesupport/lib/active_support/xml_mini/libxml.rb index 44b0bdb7dc..cde2967132 100644 --- a/activesupport/lib/active_support/xml_mini/libxml.rb +++ b/activesupport/lib/active_support/xml_mini/libxml.rb @@ -74,5 +74,7 @@ module LibXML #:nodoc: end end +# :enddoc: + LibXML::XML::Document.include(LibXML::Conversions::Document) LibXML::XML::Node.include(LibXML::Conversions::Node) diff --git a/ci/travis.rb b/ci/travis.rb index c49a87d864..f59ce5406a 100755 --- a/ci/travis.rb +++ b/ci/travis.rb @@ -154,7 +154,6 @@ ENV["GEM"].split(",").each do |gem| build = Build.new(gem, isolated: isolated) results[build.key] = build.run! - end end diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index e1b3b0a42e..5f4be07351 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -242,7 +242,7 @@ Please refer to the [Changelog][railties] for detailed changes. [Pull Request](https://github.com/rails/rails/pull/22288)) * New applications are generated with the evented file system monitor enabled - on Linux and Mac OS X. The feature can be opted out by passing + on Linux and macOS. The feature can be opted out by passing `--skip-listen` to the generator. ([commit](https://github.com/rails/rails/commit/de6ad5665d2679944a9ee9407826ba88395a1003), [commit](https://github.com/rails/rails/commit/94dbc48887bf39c241ee2ce1741ee680d773f202)) diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 666d987f8c..77bd3c97e8 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -288,7 +288,7 @@ Article destroyed Conditional Callbacks --------------------- -As with validations, we can also make the calling of a callback method conditional on the satisfaction of a given predicate. We can do this using the `:if` and `:unless` options, which can take a symbol, a string, a `Proc` or an `Array`. You may use the `:if` option when you want to specify under which conditions the callback **should** be called. If you want to specify the conditions under which the callback **should not** be called, then you may use the `:unless` option. +As with validations, we can also make the calling of a callback method conditional on the satisfaction of a given predicate. We can do this using the `:if` and `:unless` options, which can take a symbol, a `Proc` or an `Array`. You may use the `:if` option when you want to specify under which conditions the callback **should** be called. If you want to specify the conditions under which the callback **should not** be called, then you may use the `:unless` option. ### Using `:if` and `:unless` with a `Symbol` @@ -300,16 +300,6 @@ class Order < ApplicationRecord end ``` -### Using `:if` and `:unless` with a String - -You can also use a string that will be evaluated using `eval` and hence needs to contain valid Ruby code. You should use this option only when the string represents a really short condition: - -```ruby -class Order < ApplicationRecord - before_save :normalize_card_number, if: "paid_with_card?" -end -``` - ### Using `:if` and `:unless` with a `Proc` Finally, it is possible to associate `:if` and `:unless` with a `Proc` object. This option is best suited when writing short validation methods, usually one-liners: diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 665e97c470..32b38cde5e 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -916,18 +916,6 @@ class Order < ApplicationRecord end ``` -### Using a String with `:if` and `:unless` - -You can also use a string that will be evaluated using `eval` and needs to -contain valid Ruby code. You should use this option only when the string -represents a really short condition. - -```ruby -class Person < ApplicationRecord - validates :surname, presence: true, if: "name.nil?" -end -``` - ### Using a Proc with `:if` and `:unless` Finally, it's possible to associate `:if` and `:unless` with a `Proc` object diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 360de9a584..68dde4482f 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -207,7 +207,7 @@ default .coffee and .scss files will not be precompiled on their own. See precompiling works. NOTE: You must have an ExecJS supported runtime in order to use CoffeeScript. -If you are using Mac OS X or Windows, you have a JavaScript runtime installed in +If you are using macOS or Windows, you have a JavaScript runtime installed in your operating system. Check [ExecJS](https://github.com/rails/execjs#readme) documentation to know all supported JavaScript runtimes. You can also disable generation of controller specific asset files by adding the @@ -1117,7 +1117,7 @@ config.assets.js_compressor = :uglifier ``` NOTE: You will need an [ExecJS](https://github.com/rails/execjs#readme) -supported runtime in order to use `uglifier`. If you are using Mac OS X or +supported runtime in order to use `uglifier`. If you are using macOS or Windows you have a JavaScript runtime installed in your operating system. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 251b038ec9..de921e2705 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1308,7 +1308,7 @@ end Otherwise, in every request Rails walks the application tree to check if anything has changed. -On Linux and Mac OS X no additional gems are needed, but some are required +On Linux and macOS no additional gems are needed, but some are required [for *BSD](https://github.com/guard/listen#on-bsd) and [for Windows](https://github.com/guard/listen#on-windows). diff --git a/guides/source/development_dependencies_install.md b/guides/source/development_dependencies_install.md index 16c7e782bc..7ec038eb4d 100644 --- a/guides/source/development_dependencies_install.md +++ b/guides/source/development_dependencies_install.md @@ -46,7 +46,7 @@ $ cd rails The test suite must pass with any submitted code. No matter whether you are writing a new patch, or evaluating someone else's, you need to be able to run the tests. -Install first SQLite3 and its development files for the `sqlite3` gem. Mac OS X +Install first SQLite3 and its development files for the `sqlite3` gem. On macOS users are done with: ```bash diff --git a/guides/source/engines.md b/guides/source/engines.md index 0020112a1c..180a786237 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -59,7 +59,7 @@ only be enhancing it, rather than changing it drastically. To see demonstrations of other engines, check out [Devise](https://github.com/plataformatec/devise), an engine that provides authentication for its parent applications, or -[Forem](https://github.com/radar/forem), an engine that provides forum +[Thredded](https://github.com/thredded/thredded), an engine that provides forum functionality. There's also [Spree](https://github.com/spree/spree) which provides an e-commerce platform, and [RefineryCMS](https://github.com/refinery/refinerycms), a CMS engine. diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 8a451ab793..57b8472462 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -86,7 +86,7 @@ your prompt will look something like `c:\source_code>` ### Installing Rails -Open up a command line prompt. On Mac OS X open Terminal.app, on Windows choose +Open up a command line prompt. On macOS open Terminal.app, on Windows choose "Run" from your Start menu and type 'cmd.exe'. Any commands prefaced with a dollar sign `$` should be run in the command line. Verify that you have a current version of Ruby installed: @@ -98,7 +98,7 @@ ruby 2.3.1p112 TIP: A number of tools exist to help you quickly install Ruby and Ruby on Rails on your system. Windows users can use [Rails Installer](http://railsinstaller.org), -while Mac OS X users can use [Tokaido](https://github.com/tokaido/tokaidoapp). +while macOS users can use [Tokaido](https://github.com/tokaido/tokaidoapp). For more installation methods for most Operating Systems take a look at [ruby-lang.org](https://www.ruby-lang.org/en/documentation/installation/). @@ -206,7 +206,7 @@ folder directly to the Ruby interpreter e.g. `ruby bin\rails server`. TIP: Compiling CoffeeScript and JavaScript asset compression requires you have a JavaScript runtime available on your system, in the absence of a runtime you will see an `execjs` error during asset compilation. -Usually Mac OS X and Windows come with a JavaScript runtime installed. +Usually macOS and Windows come with a JavaScript runtime installed. Rails adds the `therubyracer` gem to the generated `Gemfile` in a commented line for new apps and you can uncomment if you need it. `therubyrhino` is the recommended runtime for JRuby users and is added by @@ -221,7 +221,7 @@ your application in action, open a browser window and navigate to TIP: To stop the web server, hit Ctrl+C in the terminal window where it's running. To verify the server has stopped you should see your command prompt -cursor again. For most UNIX-like systems including Mac OS X this will be a +cursor again. For most UNIX-like systems including macOS this will be a dollar sign `$`. In development mode, Rails does not generally require you to restart the server; changes you make in files will be automatically picked up by the server. diff --git a/guides/source/testing.md b/guides/source/testing.md index 6f783089a9..113314a5b8 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -8,7 +8,7 @@ This guide covers built-in mechanisms in Rails for testing your application. After reading this guide, you will know: * Rails testing terminology. -* How to write unit, functional, and integration tests for your application. +* How to write unit, functional, integration, and system tests for your application. * Other popular testing approaches and plugins. -------------------------------------------------------------------------------- @@ -33,18 +33,27 @@ Rails creates a `test` directory for you as soon as you create a Rails project u ```bash $ ls -F test -controllers/ helpers/ mailers/ test_helper.rb -fixtures/ integration/ models/ +controllers/ helpers/ mailers/ system/ test_helper.rb +fixtures/ integration/ models/ application_system_test_case.rb ``` The `helpers`, `mailers`, and `models` directories are meant to hold tests for view helpers, mailers, and models, respectively. The `controllers` directory is meant to hold tests for controllers, routes, and views. The `integration` directory is meant to hold tests for interactions between controllers. +The system test directory holds system tests, which are used for full browser +testing of your application. System tests allow you to test your application +the way your users experience it and help you test your JavaScript as well. +System tests inherit from Capybara and perform in browser tests for your +application. + Fixtures are a way of organizing test data; they reside in the `fixtures` directory. A `jobs` directory will also be created when an associated test is first generated. The `test_helper.rb` file holds the default configuration for your tests. +The `application_system_test_case.rb` holds the default configuration for your system +tests. + ### The Test Environment @@ -358,6 +367,7 @@ All the basic assertions such as `assert_equal` defined in `Minitest::Assertions * [`ActionView::TestCase`](http://api.rubyonrails.org/classes/ActionView/TestCase.html) * [`ActionDispatch::IntegrationTest`](http://api.rubyonrails.org/classes/ActionDispatch/IntegrationTest.html) * [`ActiveJob::TestCase`](http://api.rubyonrails.org/classes/ActiveJob/TestCase.html) +* [`ActionDispatch::SystemTestCase`](http://api.rubyonrails.org/classes/ActionDispatch/SystemTestCase.html) Each of these classes include `Minitest::Assertions`, allowing us to use all of the basic assertions in our tests. @@ -587,6 +597,182 @@ create test/fixtures/articles.yml Model tests don't have their own superclass like `ActionMailer::TestCase` instead they inherit from [`ActiveSupport::TestCase`](http://api.rubyonrails.org/classes/ActiveSupport/TestCase.html). +System Testing +-------------- + +System tests are full-browser tests that can be used to test your application's +JavaScript and user experience. System tests use Capybara as a base. + +System tests allow for running tests in either a real browser or a headless +driver for testing full user interactions with your application. + +For creating Rails system tests, you use the `test/system` directory in your +application. Rails provides a generator to create a system test skeleton for us. + +```bash +$ bin/rails generate system_test users_create_test.rb + invoke test_unit + create test/system/users_create_test.rb +``` + +Here's what a freshly-generated system test looks like: + +```ruby +require "application_system_test_case" + +class UsersCreateTest < ApplicationSystemTestCase + visit users_url + + assert_selector "h1", text: "Users" +end +``` + +By default, system tests are run with the Selenium driver, using the Chrome +browser, and a screen size of 1400x1400. The next section explains how to +change the default settings. + +### Changing the default settings + +Rails makes changing the default settings for system test very simple. All +the setup is abstracted away so you can focus on writing your tests. + +When you generate a new application or scaffold, an `application_system_test_case.rb` file +is created in the test directory. This is where all the configuration for your +system tests should live. + +If you want to change the default settings you can simply change what the system +tests are "driven by". Say you want to change the driver from Selenium to +Poltergeist. First add the Poltergeist gem to your Gemfile. Then in your +`application_system_test_case.rb` file do the following: + +```ruby +require "test_helper" +require "capybara/poltergeist" + +class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + driven_by :poltergeist +end +``` + +If you want to keep the Selenium driver but change the browser you +can pass Firefox and the port to driven by. The driver is a required +argument, all other arguments are optional. + +```ruby +require "test_helper" + +class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + driven_by :selenium, using: :firefox +end +``` + +The driver name is a required argument for `driven_by`. The optional arguments +that can be passed to `driven_by` are `:using` for the browser (this will only +be used for non-headless drivers like Selenium), `:on` for the port Puma should +use, and `:screen_size` to change the size of the screen for screenshots. + +If your Capybara configuration requires more setup than provided by Rails, all +of that configuration can be put into the `application_system_test_case.rb` file provided +by Rails. + +Please see [Capybara's documentation](https://github.com/teamcapybara/capybara#setup) +for additional settings. + +### Screenshot Helper + +The `ScreenshotHelper` is a helper designed to capture screenshots of your tests. +This can be helpful for viewing the browser at the point a test failed, or +to view screenshots later for debugging. + +Two methods are provided: `take_screenshot` and `take_failed_screenshot`. +`take_failed_screenshot` is automatically included in `after_teardown` inside +Rails. + +The `take_screenshot` helper method can be included anywhere in your tests to +take a screenshot of the browser. + +### Implementing a system test + +Now we're going to add a system test to our blog application. We'll demonstrate +writing a system test by visiting the index page and creating a new blog article. + +If you used the scaffold generator, a system test skeleton is automatically +created for you. If you did not use the generator start by creating a system +test skeleton. + +```bash +$ bin/rails generate system_test articles +``` + +It should have created a test file placeholder for us. With the output of the +previous command we should see: + +```bash + invoke test_unit + create test/system/articles_test.rb +``` + +Now let's open that file and write our first assertion: + +```ruby +require "application_system_test_case" + +class UsersTest < ApplicationSystemTestCase + test "viewing the index" do + visit articles_path + assert_selector "h1", text: "Articles" + end +end +``` + +The test should see that there is an h1 on the articles index and pass. + +Run the system tests. + +```bash +bin/rails test:system +``` + +#### Creating articles system test + +Now let's test the flow for creating a new article in our blog. + +```ruby +test "creating an article" do + visit articles_path + + click_on "New Article" + + fill_in "Title", with: "Creating an Article" + fill_in "Body", with: "Created this article successfully!" + + click_on "Create Article" + + assert_text "Creating an Article" +end +``` + +The first step is to call `visit articles_path`. This will take the test to the +articles index page. + +Then the `click_on "New Article"` will find the "New Article" button on the +index page. This will redirect the browser to `/articles/new`. + +Then the test will fill in the title and body of the article with the specified +text. Once the fields are filled in "Create Article" is clicked on which will +send a POST request to create the new article in the database. + +We will be redirected back to the the articles index page and there we assert +that the text from the article title is on the articles index page. + +#### Taking it further + +The beauty of system testing is that it is similar to integration testing in +that it tests the user's interaction with your controller, model, and view, but +system testing is much more robust and actually tests your application as if +a real user were using it. Going forward you can test anything that the user +themselves would do in your application such as commenting, deleting articles, +publishing draft articles, etc. Integration Testing ------------------- diff --git a/railties/lib/rails/api/generator.rb b/railties/lib/rails/api/generator.rb new file mode 100644 index 0000000000..dcc491783c --- /dev/null +++ b/railties/lib/rails/api/generator.rb @@ -0,0 +1,28 @@ +require "sdoc" + +class RDoc::Generator::API < RDoc::Generator::SDoc # :nodoc: + RDoc::RDoc.add_generator self + + def generate_class_tree_level(classes, visited = {}) + # Only process core extensions on the first visit. + if visited.empty? + core_exts, classes = classes.partition { |klass| core_extension?(klass) } + + super.unshift([ "Core extensions", "", "", build_core_ext_subtree(core_exts, visited) ]) + else + super + end + end + + private + def build_core_ext_subtree(classes, visited) + classes.map do |klass| + [ klass.name, klass.document_self_or_methods ? klass.path : "", "", + generate_class_tree_level(klass.classes_and_modules, visited) ] + end + end + + def core_extension?(klass) + klass.name != "ActiveSupport" && klass.in_files.any? { |file| file.absolute_name.include?("core_ext") } + end +end diff --git a/railties/lib/rails/api/task.rb b/railties/lib/rails/api/task.rb index bc670b1d75..d1f984be1d 100644 --- a/railties/lib/rails/api/task.rb +++ b/railties/lib/rails/api/task.rb @@ -1,4 +1,5 @@ require "rdoc/task" +require "rails/api/generator" module Rails module API @@ -8,8 +9,7 @@ module Rails include: %w( README.rdoc lib/active_support/**/*.rb - ), - exclude: "lib/active_support/vendor/*" + ) }, "activerecord" => { @@ -69,7 +69,11 @@ module Rails README.rdoc lib/**/*.rb ), - exclude: "lib/rails/generators/rails/**/templates/**/*.rb" + exclude: %w( + lib/rails/generators/rails/**/templates/**/*.rb + lib/rails/test_unit/* + lib/rails/api/generator.rb + ) } } @@ -80,7 +84,7 @@ module Rails # Be lazy computing stuff to have as light impact as possible to # the rest of tasks. before_running_rdoc do - load_and_configure_sdoc + configure_sdoc configure_rdoc_files setup_horo_variables end @@ -91,20 +95,15 @@ module Rails # no-op end - def load_and_configure_sdoc - require "sdoc" - + def configure_sdoc self.title = "Ruby on Rails API" self.rdoc_dir = api_dir options << "-m" << api_main options << "-e" << "UTF-8" - options << "-f" << "sdoc" + options << "-f" << "api" options << "-T" << "rails" - rescue LoadError - $stderr.puts %(Unable to load SDoc, please add\n\n gem 'sdoc', require: false\n\nto the Gemfile.) - exit 1 end def configure_rdoc_files @@ -147,7 +146,7 @@ module Rails end class RepoTask < Task - def load_and_configure_sdoc + def configure_sdoc super options << "-g" # link to GitHub, SDoc flag end diff --git a/railties/lib/rails/command/actions.rb b/railties/lib/rails/command/actions.rb index fb80e9d997..8fda1c87c6 100644 --- a/railties/lib/rails/command/actions.rb +++ b/railties/lib/rails/command/actions.rb @@ -8,16 +8,16 @@ module Rails Dir.chdir(File.expand_path("../../", APP_PATH)) unless File.exist?(File.expand_path("config.ru")) end - if defined?(ENGINE_PATH) - def require_application_and_environment! - require ENGINE_PATH + def require_application_and_environment! + require ENGINE_PATH if defined?(ENGINE_PATH) - if defined?(APP_PATH) - require APP_PATH - Rails.application.require_environment! - end + if defined?(APP_PATH) + require APP_PATH + Rails.application.require_environment! end + end + if defined?(ENGINE_PATH) def load_tasks Rake.application.init("rails") Rake.application.load_rakefile @@ -29,11 +29,6 @@ module Rails engine.load_generators end else - def require_application_and_environment! - require APP_PATH - Rails.application.require_environment! - end - def load_tasks Rails.application.load_tasks end diff --git a/railties/lib/rails/commands/test/test_command.rb b/railties/lib/rails/commands/test/test_command.rb index 7bf8f61137..629fb5b425 100644 --- a/railties/lib/rails/commands/test/test_command.rb +++ b/railties/lib/rails/commands/test/test_command.rb @@ -11,7 +11,7 @@ module Rails def perform(*) $LOAD_PATH << Rails::Command.root.join("test") - Minitest.run_via[:rails] = true + Minitest.run_via = :rails require "active_support/testing/autorun" end diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 99bda728ee..85f66cc416 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -62,6 +62,7 @@ module Rails stylesheets: true, stylesheet_engine: :css, scaffold_stylesheet: true, + system_tests: nil, test_framework: false, template_engine: :erb } @@ -151,6 +152,7 @@ module Rails "#{test}:controller", "#{test}:helper", "#{test}:integration", + "#{test}:system", "#{test}:mailer", "#{test}:model", "#{test}:scaffold", diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 61c371d2bc..f365ad05c7 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -82,6 +82,9 @@ module Rails class_option :skip_test, type: :boolean, aliases: "-T", default: false, desc: "Skip test files" + class_option :skip_system_test, type: :boolean, default: false, + desc: "Skip system test files" + class_option :dev, type: :boolean, default: false, desc: "Setup the #{name} with Gemfile pointing to your Rails checkout" diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 3cf923faf0..18e48c8016 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -150,6 +150,12 @@ module Rails template "test/test_helper.rb" end + def system_test + empty_directory_with_keep_file "test/system" + + template "test/application_system_test_case.rb" + end + def tmp empty_directory_with_keep_file "tmp" empty_directory "tmp/cache" @@ -262,6 +268,10 @@ module Rails build(:test) unless options[:skip_test] end + def create_system_test_files + build(:system_test) unless options[:skip_system_test] || options[:skip_test] || options[:api] + end + def create_tmp_files build(:tmp) end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 24d2fa1284..b082d70cba 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -32,6 +32,11 @@ end group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] + <%- unless options.skip_system_test? || options.api? -%> + # Adds support for Capybara system testing and selenium driver + gem 'capybara', '~> 2.7.0' + gem 'selenium-webdriver' + <%- end -%> end group :development do diff --git a/railties/lib/rails/generators/rails/app/templates/test/application_system_test_case.rb b/railties/lib/rails/generators/rails/app/templates/test/application_system_test_case.rb new file mode 100644 index 0000000000..d19212abd5 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/test/application_system_test_case.rb @@ -0,0 +1,5 @@ +require "test_helper" + +class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + driven_by :selenium, using: :chrome, screen_size: [1400, 1400] +end diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 49259f32c8..be211e016d 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -91,6 +91,7 @@ task default: :test opts[:skip_bundle] = true opts[:api] = options.api? opts[:skip_listen] = true + opts[:skip_git] = true invoke Rails::Generators::AppGenerator, [ File.expand_path(dummy_path, destination_root) ], opts @@ -112,7 +113,6 @@ task default: :test def test_dummy_clean inside dummy_path do - remove_file ".gitignore" remove_file "db/seeds.rb" remove_file "doc" remove_file "Gemfile" diff --git a/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt b/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt index c0fbb84a93..35a9bf8c8b 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt +++ b/railties/lib/rails/generators/rails/plugin/templates/bin/test.tt @@ -5,6 +5,6 @@ require 'rails/test_unit/minitest_plugin' Rails::TestUnitReporter.executable = 'bin/test' -Minitest.run_via[:rails] = true +Minitest.run_via = :rails require "active_support/testing/autorun" diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/application_system_test_case.rb b/railties/lib/rails/generators/rails/plugin/templates/test/application_system_test_case.rb new file mode 100644 index 0000000000..d19212abd5 --- /dev/null +++ b/railties/lib/rails/generators/rails/plugin/templates/test/application_system_test_case.rb @@ -0,0 +1,5 @@ +require "test_helper" + +class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + driven_by :selenium, using: :chrome, screen_size: [1400, 1400] +end diff --git a/railties/lib/rails/generators/rails/scaffold/scaffold_generator.rb b/railties/lib/rails/generators/rails/scaffold/scaffold_generator.rb index ed6bf7f7d7..12d6bc85b2 100644 --- a/railties/lib/rails/generators/rails/scaffold/scaffold_generator.rb +++ b/railties/lib/rails/generators/rails/scaffold/scaffold_generator.rb @@ -6,6 +6,7 @@ module Rails remove_hook_for :resource_controller remove_class_option :actions + class_option :api, type: :boolean class_option :stylesheets, type: :boolean, desc: "Generate Stylesheets" class_option :stylesheet_engine, desc: "Engine for Stylesheets" class_option :assets, type: :boolean @@ -15,10 +16,13 @@ module Rails def handle_skip @options = @options.merge(stylesheets: false) unless options[:assets] @options = @options.merge(stylesheet_engine: false) unless options[:stylesheets] && options[:scaffold_stylesheet] + @options = @options.merge(system_tests: false) if options[:api] end hook_for :scaffold_controller, required: true + hook_for :system_tests, as: :system + hook_for :assets do |assets| invoke assets, [controller_name] end diff --git a/railties/lib/rails/generators/rails/system_test/USAGE b/railties/lib/rails/generators/rails/system_test/USAGE new file mode 100644 index 0000000000..f11a99e008 --- /dev/null +++ b/railties/lib/rails/generators/rails/system_test/USAGE @@ -0,0 +1,10 @@ +Description: + Stubs out a new system test. Pass the name of the test, either + CamelCased or under_scored, as an argument. + + This generator invokes the current system tool, which defaults to + TestUnit. + +Example: + `rails generate system_test GeneralStories` creates a GeneralStories + system test in test/system/general_stories_test.rb diff --git a/railties/lib/rails/generators/rails/system_test/system_test_generator.rb b/railties/lib/rails/generators/rails/system_test/system_test_generator.rb new file mode 100644 index 0000000000..901120e892 --- /dev/null +++ b/railties/lib/rails/generators/rails/system_test/system_test_generator.rb @@ -0,0 +1,7 @@ +module Rails + module Generators + class SystemTestGenerator < NamedBase # :nodoc: + hook_for :system_tests, as: :system + end + end +end diff --git a/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb b/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb index dea7e22196..118e0f1271 100644 --- a/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb +++ b/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb @@ -1,7 +1,9 @@ require 'test_helper' +<% module_namespacing do -%> class <%= class_name %>Test < ActionDispatch::IntegrationTest # test "the truth" do # assert true # end end +<% end -%> diff --git a/railties/lib/rails/generators/test_unit/system/system_generator.rb b/railties/lib/rails/generators/test_unit/system/system_generator.rb new file mode 100644 index 0000000000..aec415a4e5 --- /dev/null +++ b/railties/lib/rails/generators/test_unit/system/system_generator.rb @@ -0,0 +1,17 @@ +require "rails/generators/test_unit" + +module TestUnit # :nodoc: + module Generators # :nodoc: + class SystemGenerator < Base # :nodoc: + check_class_collision suffix: "Test" + + def create_test_files + if !File.exist?(File.join("test/application_system_test_case.rb")) + template "application_system_test_case.rb", File.join("test", "application_system_test_case.rb") + end + + template "system_test.rb", File.join("test/system", "#{file_name.pluralize}_test.rb") + end + end + end +end diff --git a/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb b/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb new file mode 100644 index 0000000000..d19212abd5 --- /dev/null +++ b/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb @@ -0,0 +1,5 @@ +require "test_helper" + +class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + driven_by :selenium, using: :chrome, screen_size: [1400, 1400] +end diff --git a/railties/lib/rails/generators/test_unit/system/templates/system_test.rb b/railties/lib/rails/generators/test_unit/system/templates/system_test.rb new file mode 100644 index 0000000000..b5ce2ba5c8 --- /dev/null +++ b/railties/lib/rails/generators/test_unit/system/templates/system_test.rb @@ -0,0 +1,9 @@ +require "application_system_test_case" + +class <%= class_name.pluralize %>Test < ApplicationSystemTestCase + # test "visiting the index" do + # visit <%= plural_table_name %>_url + # + # assert_selector "h1", text: "<%= class_name %>" + # end +end diff --git a/railties/lib/rails/tasks/statistics.rake b/railties/lib/rails/tasks/statistics.rake index ba1697186e..cb569be58b 100644 --- a/railties/lib/rails/tasks/statistics.rake +++ b/railties/lib/rails/tasks/statistics.rake @@ -17,6 +17,7 @@ STATS_DIRECTORIES = [ %w(Mailer\ tests test/mailers), %w(Job\ tests test/jobs), %w(Integration\ tests test/integration), + %w(System\ tests test/system), ].collect do |name, dir| [ name, "#{File.dirname(Rake.application.rakefile_location)}/#{dir}" ] end.select { |name, dir| File.directory?(dir) } diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 5fda160012..75171f2395 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -7,6 +7,7 @@ require "active_support/test_case" require "action_controller" require "action_controller/test_case" require "action_dispatch/testing/integration" +require "action_dispatch/system_test_case" require "rails/generators/test_case" require "active_support/testing/autorun" @@ -14,10 +15,12 @@ require "active_support/testing/autorun" if defined?(ActiveRecord::Base) ActiveRecord::Migration.maintain_test_schema! - class ActiveSupport::TestCase - include ActiveRecord::TestFixtures - self.fixture_path = "#{Rails.root}/test/fixtures/" - self.file_fixture_path = fixture_path + "files" + module ActiveSupport + class TestCase + include ActiveRecord::TestFixtures + self.fixture_path = "#{Rails.root}/test/fixtures/" + self.file_fixture_path = fixture_path + "files" + end end ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path @@ -27,6 +30,8 @@ if defined?(ActiveRecord::Base) end end +# :enddoc: + class ActionController::TestCase def before_setup # :nodoc: @routes = Rails.application.routes @@ -40,3 +45,10 @@ class ActionDispatch::IntegrationTest super end end + +class ActionDispatch::SystemTestCase + def before_setup # :nodoc: + @routes = Rails.application.routes + super + end +end diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index 4df3e7f0f2..7d3da6b529 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -59,18 +59,18 @@ module Minitest options[:color] = true options[:output_inline] = true - options[:patterns] = opts.order! unless run_via[:rake] + options[:patterns] = opts.order! unless run_via.rake? end def self.rake_run(patterns) # :nodoc: - run_via[:rake] = true + self.run_via = :rake unless run_via.set? ::Rails::TestRequirer.require_files(patterns) autorun end module RunRespectingRakeTestopts def run(args = []) - if run_via[:rake] + if run_via.rake? args = Shellwords.split(ENV["TESTOPTS"] || "") end @@ -87,7 +87,7 @@ module Minitest # If run via `ruby` we've been passed the files to run directly, or if run # via `rake` then they have already been eagerly required. - unless run_via[:ruby] || run_via[:rake] + unless run_via.ruby? || run_via.rake? ::Rails::TestRequirer.require_files(options[:patterns]) end @@ -102,7 +102,31 @@ module Minitest reporter << ::Rails::TestUnitReporter.new(options[:io], options) end - mattr_accessor(:run_via) { Hash.new } + def self.run_via=(runner) + if run_via.set? + raise ArgumentError, "run_via already assigned" + else + run_via.runner = runner + end + end + + class RunVia + attr_accessor :runner + alias set? runner + + # Backwardscompatibility with Rails 5.0 generated plugin test scripts. + alias []= runner= + + def ruby? + runner == :ruby + end + + def rake? + runner == :rake + end + end + + mattr_reader(:run_via) { RunVia.new } end # Put Rails as the first plugin minitest initializes so other plugins diff --git a/railties/lib/rails/test_unit/railtie.rb b/railties/lib/rails/test_unit/railtie.rb index 746120e6a1..9cc3f73a9c 100644 --- a/railties/lib/rails/test_unit/railtie.rb +++ b/railties/lib/rails/test_unit/railtie.rb @@ -11,6 +11,7 @@ module Rails fixture_replacement: nil c.integration_tool :test_unit + c.system_tests :test_unit end initializer "test_unit.line_filtering" do diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index 4c157c1262..4dde3d3c97 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -47,4 +47,9 @@ namespace :test do $: << "test" Minitest.rake_run(["test/controllers", "test/mailers", "test/functional"]) end + + task system: "test:prepare" do + $: << "test" + Minitest.rake_run(["test/system"]) + end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 8c0f1a2725..bd1b412b36 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -42,6 +42,7 @@ DEFAULT_APP_FILES = %w( test/helpers test/mailers test/integration + test/system vendor tmp tmp/cache @@ -805,8 +806,26 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_equal 4, @sequence_step end - private + def test_system_tests_directory_generated + run_generator + + assert_file("test/system/.keep") + assert_directory("test/system") + end + + def test_system_tests_are_not_generated_on_system_test_skip + run_generator [destination_root, "--skip-system-test"] + + assert_no_directory("test/system") + end + + def test_system_tests_are_not_generated_on_test_skip + run_generator [destination_root, "--skip-test"] + assert_no_directory("test/system") + end + + private def stub_rails_application(root) Rails.application.config.root = root Rails.application.class.stub(:name, "Myapp") do diff --git a/railties/test/generators/integration_test_generator_test.rb b/railties/test/generators/integration_test_generator_test.rb index 8bcc02440a..9358b63bd4 100644 --- a/railties/test/generators/integration_test_generator_test.rb +++ b/railties/test/generators/integration_test_generator_test.rb @@ -3,10 +3,14 @@ require "rails/generators/rails/integration_test/integration_test_generator" class IntegrationTestGeneratorTest < Rails::Generators::TestCase include GeneratorsTestHelper - arguments %w(integration) def test_integration_test_skeleton_is_created - run_generator + run_generator %w(integration) assert_file "test/integration/integration_test.rb", /class IntegrationTest < ActionDispatch::IntegrationTest/ end + + def test_namespaced_integration_test_skeleton_is_created + run_generator %w(iguchi/integration) + assert_file "test/integration/iguchi/integration_test.rb", /class Iguchi::IntegrationTest < ActionDispatch::IntegrationTest/ + end end diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index ddfbc1a698..eaf1199601 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -491,6 +491,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_no_directory "test/dummy/doc" assert_no_directory "test/dummy/test" assert_no_directory "test/dummy/vendor" + assert_no_directory "test/dummy/.git" end def test_skipping_test_files diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index e2b2acab0f..436fbd5d73 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -62,6 +62,11 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_match(/patch product_line_url\(@product_line\), params: \{ product_line: \{ product_id: @product_line\.product_id, title: @product_line\.title, user_id: @product_line\.user_id \} \}/, test) end + # System tests + assert_file "test/system/product_lines_test.rb" do |test| + assert_match(/class ProductLinesTest < ApplicationSystemTestCase/, test) + end + # Views assert_no_file "app/views/layouts/product_lines.html.erb" diff --git a/railties/test/generators/system_test_generator_test.rb b/railties/test/generators/system_test_generator_test.rb new file mode 100644 index 0000000000..e8e561ec49 --- /dev/null +++ b/railties/test/generators/system_test_generator_test.rb @@ -0,0 +1,12 @@ +require "generators/generators_test_helper" +require "rails/generators/rails/system_test/system_test_generator" + +class SystemTestGeneratorTest < Rails::Generators::TestCase + include GeneratorsTestHelper + arguments %w(user) + + def test_system_test_skeleton_is_created + run_generator + assert_file "test/system/users_test.rb", /class UsersTest < ApplicationSystemTestCase/ + end +end diff --git a/tools/test.rb b/tools/test.rb index ce546b382d..71349a5974 100644 --- a/tools/test.rb +++ b/tools/test.rb @@ -16,5 +16,5 @@ end ActiveSupport::TestCase.extend Rails::LineFiltering Rails::TestUnitReporter.executable = "bin/test" -Minitest.run_via[:rails] = true +Minitest.run_via = :rails require "active_support/testing/autorun" |