diff options
Diffstat (limited to 'actionpack')
42 files changed, 522 insertions, 146 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 641af029aa..d0662bdae2 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,28 @@ +* Fix `NameError` raised in `ActionController::Renderer#with_defaults` + + *Hiroyuki Ishii* + +* Added `#reverse_merge` and `#reverse_merge!` methods to `ActionController::Parameters` + + *Edouard Chin*, *Mitsutaka Mimura* + +* Fix malformed URLS when using `ApplicationController.renderer` + + The Rack environment variable `rack.url_scheme` was not being set so `scheme` was + returning `nil`. This caused URLs to be malformed with the default settings. + Fix this by setting `rack.url_scheme` when the environment is normalized. + + Fixes #28151. + + *George Vrettos* + +* Commit flash changes when using a redirect route. + + Fixes #27992. + + *Andrew White* + + ## Rails 5.1.0.beta1 (February 23, 2017) ## * Prefer `remove_method` over `undef_method` when reloading routes @@ -13,7 +38,7 @@ ``` ruby resource :basket - resolve(class: "Basket") { [:basket] } + resolve("Basket") { [:basket] } ``` ``` erb @@ -308,7 +333,7 @@ redirects to POST https://example.com/articles (i.e. ArticlesContoller#create) - *Chirag Singhal* + *Chirag Singhal* * Add `:as` option to `ActionController:TestCase#process` and related methods. diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index 5cd8d77ddb..0d1af0d0bd 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -81,10 +81,9 @@ module ActionController # end # end # - # Quite straightforward. Make sure to check the modules included in - # <tt>ActionController::Base</tt> if you want to use any other - # functionality that is not provided by <tt>ActionController::API</tt> - # out of the box. + # Make sure to check the modules included in <tt>ActionController::Base</tt> + # if you want to use any other functionality that is not provided + # by <tt>ActionController::API</tt> out of the box. class API < Metal abstract! diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index ca8066cd82..0fe0853da3 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -8,7 +8,7 @@ module ActionController # on the controller, which will automatically be made accessible to the web-server through \Rails Routes. # # By default, only the ApplicationController in a \Rails application inherits from <tt>ActionController::Base</tt>. All other - # controllers in turn inherit from ApplicationController. This gives you one class to configure things such as + # controllers inherit from ApplicationController. This gives you one class to configure things such as # request forgery protection and filtering of sensitive request parameters. # # A sample controller could look like this: @@ -30,7 +30,7 @@ module ActionController # # Unlike index, the create action will not render a template. After performing its main purpose (creating a # new post), it initiates a redirect instead. This redirect works by returning an external - # "302 Moved" HTTP response that takes the user to the index action. + # <tt>302 Moved</tt> HTTP response that takes the user to the index action. # # These two methods represent the two basic action archetypes used in Action Controllers: Get-and-show and do-and-redirect. # Most actions are variations on these themes. @@ -59,7 +59,7 @@ module ActionController # <input type="text" name="post[name]" value="david"> # <input type="text" name="post[address]" value="hyacintvej"> # - # A request stemming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. + # A request coming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. # If the address input had been named <tt>post[address][street]</tt>, the <tt>params</tt> would have included # <tt>{ "post" => { "address" => { "street" => "hyacintvej" } } }</tt>. There's no limit to the depth of the nesting. # @@ -74,7 +74,7 @@ module ActionController # # session[:person] = Person.authenticate(user_name, password) # - # And retrieved again through the same hash: + # You can retrieve it again through the same hash: # # Hello #{session[:person]} # diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 337718afc0..74c4153cd2 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -138,7 +138,7 @@ module ActionController false end - # Delegates to the class' <tt>controller_name</tt> + # Delegates to the class' <tt>controller_name</tt>. def controller_name self.class.controller_name end @@ -244,7 +244,7 @@ module ActionController end end - # Direct dispatch to the controller. Instantiates the controller, then + # Direct dispatch to the controller. Instantiates the controller, then # executes the action named +name+. def self.dispatch(name, req, res) if middleware_stack.any? diff --git a/actionpack/lib/action_controller/metal/etag_with_flash.rb b/actionpack/lib/action_controller/metal/etag_with_flash.rb index 474d75f02e..7bd338bd7c 100644 --- a/actionpack/lib/action_controller/metal/etag_with_flash.rb +++ b/actionpack/lib/action_controller/metal/etag_with_flash.rb @@ -1,9 +1,9 @@ module ActionController # When you're using the flash, it's generally used as a conditional on the view. # This means the content of the view depends on the flash. Which in turn means - # that the etag for a response should be computed with the content of the flash + # that the ETag for a response should be computed with the content of the flash # in mind. This does that by including the content of the flash as a component - # in the etag that's generated for a response. + # in the ETag that's generated for a response. module EtagWithFlash extend ActiveSupport::Concern diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 9d43e752ac..73e67573ca 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -2,17 +2,17 @@ require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/slice" module ActionController - # This module provides a method which will redirect the browser to use HTTPS - # protocol. This will ensure that user's sensitive information will be + # This module provides a method which will redirect the browser to use the secured HTTPS + # protocol. This will ensure that users' sensitive information will be # transferred safely over the internet. You _should_ always force the browser # to use HTTPS when you're transferring sensitive information such as # user authentication, account information, or credit card information. # # Note that if you are really concerned about your application security, # you might consider using +config.force_ssl+ in your config file instead. - # That will ensure all the data transferred via HTTPS protocol and prevent - # the user from getting their session hijacked when accessing the site over - # unsecured HTTP protocol. + # That will ensure all the data is transferred via HTTPS, and will + # prevent the user from getting their session hijacked when accessing the + # site over unsecured HTTP protocol. module ForceSSL extend ActiveSupport::Concern include AbstractController::Callbacks @@ -23,7 +23,7 @@ module ActionController module ClassMethods # Force the request to this particular controller or specified actions to be - # under HTTPS protocol. + # through the HTTPS protocol. # # If you need to disable this for any reason (e.g. development) then you can use # an +:if+ or +:unless+ condition. @@ -71,7 +71,7 @@ module ActionController # Redirect the existing request to use the HTTPS protocol. # # ==== Parameters - # * <tt>host_or_options</tt> - Either a host name or any of the url & + # * <tt>host_or_options</tt> - Either a host name or any of the url and # redirect options available to the <tt>force_ssl</tt> method. def force_ssl_redirect(host_or_options = nil) unless request.ssl? diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0575360068..d8bc895265 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -445,7 +445,7 @@ module ActionController end end - # Parses the token and options out of the token authorization header. + # Parses the token and options out of the token Authorization header. # The value for the Authorization header is expected to have the prefix # <tt>"Token"</tt> or <tt>"Bearer"</tt>. If the header looks like this: # Authorization: Token token="abc", nonce="def" diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index dde924e682..eeb27f99f4 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -2,11 +2,11 @@ module ActionController # Handles implicit rendering for a controller action that does not # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. # - # For API controllers, the implicit response is always 204 No Content. + # For API controllers, the implicit response is always <tt>204 No Content</tt>. # # For all other controllers, we use these heuristics to decide whether to # render a template, raise an error for a missing template, or respond with - # 204 No Content: + # <tt>204 No Content</tt>: # # First, if we DO find a template, it's rendered. Template lookup accounts # for the action name, locales, format, variant, template handlers, and more @@ -23,7 +23,7 @@ module ActionController # <tt>ActionView::UnknownFormat</tt> with an explanation. # # Finally, if we DON'T find a template AND the request isn't a browser page - # load, then we implicitly respond with 204 No Content. + # load, then we implicitly respond with <tt>204 No Content</tt>. module ImplicitRender # :stopdoc: include BasicImplicitRender diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 924686218f..2485d27cec 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -3,7 +3,7 @@ require "abstract_controller/logger" module ActionController # Adds instrumentation to several ends in ActionController::Base. It also provides - # some hooks related with process_action, this allows an ORM like Active Record + # some hooks related with process_action. This allows an ORM like Active Record # and/or DataMapper to plug in ActionController and show related information. # # Check ActiveRecord::Railties::ControllerRuntime for an example. diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fed99e6c82..a607ee2309 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -239,8 +239,8 @@ module ActionController error = nil # This processes the action in a child thread. It lets us return the - # response code and headers back up the rack stack, and still process - # the body in parallel with sending data to the client + # response code and headers back up the Rack stack, and still process + # the body in parallel with sending data to the client. new_controller_thread { ActiveSupport::Dependencies.interlock.running do t2 = Thread.current @@ -278,9 +278,9 @@ module ActionController raise error if error end - # Spawn a new thread to serve up the controller in. This is to get + # Spawn a new thread to serve up the controller in. This is to get # around the fact that Rack isn't based around IOs and we need to use - # a thread to stream data from the response bodies. Nobody should call + # a thread to stream data from the response bodies. Nobody should call # this method except in Rails internals. Seriously! def new_controller_thread # :nodoc: Thread.new { diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index f6aabcb102..7b4c7b923e 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -181,8 +181,8 @@ module ActionController #:nodoc: # # request.variant = [:tablet, :phone] # - # which will work similarly to formats and MIME types negotiation. If there will be no - # +:tablet+ variant declared, +:phone+ variant will be picked: + # This will work similarly to formats and MIME types negotiation. If there + # is no +:tablet+ variant declared, +:phone+ variant will be picked: # # respond_to do |format| # format.html.none diff --git a/actionpack/lib/action_controller/metal/parameter_encoding.rb b/actionpack/lib/action_controller/metal/parameter_encoding.rb index 962532ff09..ecc691619e 100644 --- a/actionpack/lib/action_controller/metal/parameter_encoding.rb +++ b/actionpack/lib/action_controller/metal/parameter_encoding.rb @@ -39,7 +39,7 @@ module ActionController # end # # The show action in the above controller would have all parameter values - # encoded as ASCII-8BIT. This is useful in the case where an application + # encoded as ASCII-8BIT. This is useful in the case where an application # must handle data but encoding of the data is unknown, like file system data. def skip_parameter_encoding(action) @_parameter_encodings[action.to_s] = true diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 7fc898f034..3cca5e8906 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -213,7 +213,7 @@ module ActionController end # Sets the default wrapper key or model which will be used to determine - # wrapper key and attribute names. Will be called automatically when the + # wrapper key and attribute names. Called automatically when the # module is inherited. def inherited(klass) if klass._wrapper_options.format.any? @@ -225,7 +225,7 @@ module ActionController end end - # Performs parameters wrapping upon the request. Will be called automatically + # Performs parameters wrapping upon the request. Called automatically # by the metal call stack. def process_action(*args) if _wrapper_enabled? @@ -238,11 +238,11 @@ module ActionController wrapped_keys = request.request_parameters.keys wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys) - # This will make the wrapped hash accessible from controller and view + # This will make the wrapped hash accessible from controller and view. request.parameters.merge! wrapped_hash request.request_parameters.merge! wrapped_hash - # This will display the wrapped hash in the log file + # This will display the wrapped hash in the log file. request.filtered_parameters.merge! wrapped_filtered_hash end super diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index a349841082..fdfe82f96b 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -22,7 +22,7 @@ module ActionController # redirect_to posts_url # redirect_to proc { edit_post_url(@post) } # - # The redirection happens as a "302 Found" header unless otherwise specified using the <tt>:status</tt> option: + # The redirection happens as a <tt>302 Found</tt> header unless otherwise specified using the <tt>:status</tt> option: # # redirect_to post_url(@post), status: :found # redirect_to action: 'atom', status: :moved_permanently @@ -36,7 +36,7 @@ module ActionController # If you are using XHR requests other than GET or POST and redirecting after the # request then some browsers will follow the redirect using the original request # method. This may lead to undesirable behavior such as a double DELETE. To work - # around this you can return a <tt>303 See Other</tt> status code which will be + # around this you can return a <tt>303 See Other</tt> status code which will be # followed using a GET request. # # redirect_to posts_url, status: :see_other @@ -50,6 +50,9 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # + # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. + # To terminate the execution of the function immediately after the +redirect_to+, use return. + # redirect_to post_url(@post) and return def redirect_to(options = {}, response_status = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 6b17719381..67f207afc2 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -36,7 +36,7 @@ module ActionController super end - # Overwrite render_to_string because body can now be set to a rack body. + # Overwrite render_to_string because body can now be set to a Rack body. def render_to_string(*) result = super if result.respond_to?(:each) diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e8965a6561..d9a8b9c12d 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -262,9 +262,9 @@ module ActionController #:nodoc: # Returns true or false if a request is verified. Checks: # - # * Is it a GET or HEAD request? Gets should be safe and idempotent + # * Is it a GET or HEAD request? GETs should be safe and idempotent # * Does the form_authenticity_token match the given token value from the params? - # * Does the X-CSRF-Token header match the form_authenticity_token + # * Does the X-CSRF-Token header match the form_authenticity_token? def verified_request? # :doc: !protect_against_forgery? || request.get? || request.head? || (valid_request_origin? && any_authenticity_token_valid?) @@ -327,7 +327,7 @@ module ActionController #:nodoc: if masked_token.length == AUTHENTICITY_TOKEN_LENGTH # This is actually an unmasked token. This is expected if # you have just upgraded to masked tokens, but should stop - # happening shortly after installing this gem + # happening shortly after installing this gem. compare_with_real_token masked_token, session elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2 @@ -336,13 +336,13 @@ module ActionController #:nodoc: compare_with_real_token(csrf_token, session) || valid_per_form_csrf_token?(csrf_token, session) else - false # Token is malformed + false # Token is malformed. end end def unmask_token(masked_token) # :doc: # Split the token into the one-time pad and the encrypted - # value and decrypt it + # value and decrypt it. one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] xor_byte_strings(one_time_pad, encrypted_csrf_token) diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 2d99e4045b..25757938f5 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -10,7 +10,7 @@ module ActionController #:nodoc: # exceptions must be shown. This method is only called when # consider_all_requests_local is false. By default, it returns # false, but someone may set it to `request.local?` so local - # requests in production still shows the detailed exception pages. + # requests in production still show the detailed exception pages. def show_detailed_exceptions? false end diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 877a08b222..58cf60ad2a 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -3,7 +3,7 @@ require "rack/chunked" module ActionController #:nodoc: # Allows views to be streamed back to the client as they are rendered. # - # The default way Rails renders views is by first rendering the template + # By default, Rails renders views by first rendering the template # and then the layout. The response is sent to the client after the whole # template is rendered, all queries are made, and the layout is processed. # diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d304dcf468..1190e0ed69 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -112,6 +112,77 @@ module ActionController cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false + ## + # :method: as_json + # + # :call-seq: + # as_json(options=nil) + # + # Returns a hash that can be used as the JSON representation for the parameters. + + ## + # :method: empty? + # + # :call-seq: + # empty?() + # + # Returns true if the parameters have no key/value pairs. + + ## + # :method: has_key? + # + # :call-seq: + # has_key?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: has_value? + # + # :call-seq: + # has_value?(value) + # + # Returns true if the given value is present for some key in the parameters. + + ## + # :method: include? + # + # :call-seq: + # include?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: key? + # + # :call-seq: + # key?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: keys + # + # :call-seq: + # keys() + # + # Returns a new array of the keys of the parameters. + + ## + # :method: value? + # + # :call-seq: + # value?(value) + # + # Returns true if the given value is present for some key in the parameters. + + ## + # :method: values + # + # :call-seq: + # values() + # + # Returns a new array of the values of the parameters. delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters @@ -191,7 +262,7 @@ module ActionController alias_method :to_unsafe_hash, :to_unsafe_h # Convert all hashes in values into parameters, then yield each pair in - # the same way as <tt>Hash#each_pair</tt> + # the same way as <tt>Hash#each_pair</tt>. def each_pair(&block) @parameters.each_pair do |key, value| yield key, convert_hashes_to_parameters(key, value) @@ -339,7 +410,7 @@ module ActionController # # params.permit(preferences: {}) # - # but be careful because this opens the door to arbitrary input. In this + # Be careful because this opens the door to arbitrary input. In this # case, +permit+ ensures values in the returned structure are permitted # scalars and filters out anything else. # @@ -575,20 +646,35 @@ module ActionController end # Returns a new <tt>ActionController::Parameters</tt> with all keys from - # +other_hash+ merges into current hash. + # +other_hash+ merged into current hash. def merge(other_hash) new_instance_with_inherited_permitted_status( @parameters.merge(other_hash.to_h) ) end - # Returns current <tt>ActionController::Parameters</tt> instance which - # +other_hash+ merges into current hash. + # Returns current <tt>ActionController::Parameters</tt> instance with + # +other_hash+ merged into current hash. def merge!(other_hash) @parameters.merge!(other_hash.to_h) self end + # Returns a new <tt>ActionController::Parameters</tt> with all keys from + # current hash merged into +other_hash+. + def reverse_merge(other_hash) + new_instance_with_inherited_permitted_status( + other_hash.to_h.merge(@parameters) + ) + end + + # Returns current <tt>ActionController::Parameters</tt> instance with + # current hash merged into +other_hash+. + def reverse_merge!(other_hash) + @parameters.merge!(other_hash.to_h) { |key, left, right| left } + self + end + # This is required by ActiveModel attribute assignment, so that user can # pass +Parameters+ to a mass assignment methods in a model. It should not # matter as we are using +HashWithIndifferentAccess+ internally. @@ -629,7 +715,7 @@ module ActionController undef_method :to_param - # Returns duplicate of object including all parameters + # Returns duplicate of object including all parameters. def deep_dup self.class.new(@parameters.deep_dup).tap do |duplicate| duplicate.permitted = @permitted @@ -849,7 +935,7 @@ module ActionController # whitelisted. # # In addition, parameters can be marked as required and flow through a - # predefined raise/rescue flow to end up as a 400 Bad Request with no + # predefined raise/rescue flow to end up as a <tt>400 Bad Request</tt> with no # effort. # # class PeopleController < ActionController::Base @@ -862,7 +948,7 @@ module ActionController # end # # # This will pass with flying colors as long as there's a person key in the - # # parameters, otherwise it'll raise an ActionController::MissingParameter + # # parameters, otherwise it'll raise an ActionController::ParameterMissing # # exception, which will get caught by ActionController::Base and turned # # into a 400 Bad Request reply. # def update @@ -873,7 +959,7 @@ module ActionController # # private # # Using a private method to encapsulate the permissible parameters is - # # just a good pattern since you'll be able to reuse the same permit + # # a good pattern since you'll be able to reuse the same permit # # list between create and update. Also, you can specialize this method # # with per-user checking of permissible attributes. # def person_params diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 9f3cc099d6..21ed5b4ec8 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -3,7 +3,7 @@ module ActionController # the <tt>_routes</tt> method. Otherwise, an exception will be raised. # # In addition to <tt>AbstractController::UrlFor</tt>, this module accesses the HTTP layer to define - # url options like the +host+. In order to do so, this module requires the host class + # URL options like the +host+. In order to do so, this module requires the host class # to implement +env+ which needs to be Rack-compatible and +request+ # which is either an instance of +ActionDispatch::Request+ or an object # that responds to the +host+, +optional_port+, +protocol+ and diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index a7cdfe6a98..fadfc8de60 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -42,7 +42,7 @@ module ActionController options.javascripts_dir ||= paths["public/javascripts"].first options.stylesheets_dir ||= paths["public/stylesheets"].first - # Ensure readers methods get compiled + # Ensure readers methods get compiled. options.asset_host ||= app.config.asset_host options.relative_url_root ||= app.config.relative_url_root diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index acb400cd15..cbb719d8b2 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -5,7 +5,7 @@ module ActionController # without requirement of being in controller actions. # # You get a concrete renderer class by invoking ActionController::Base#renderer. - # For example, + # For example: # # ApplicationController.renderer # @@ -18,7 +18,7 @@ module ActionController # ApplicationController.render template: '...' # # #render allows you to use the same options that you can use when rendering in a controller. - # For example, + # For example: # # FooController.render :action, locals: { ... }, assigns: { ... } # @@ -56,7 +56,7 @@ module ActionController # Create a new renderer for the same controller but with new defaults. def with_defaults(defaults) - self.class.new controller, env, self.defaults.merge(defaults) + self.class.new controller, @env, self.defaults.merge(defaults) end # Accepts a custom Rack environment to render templates in. @@ -85,6 +85,7 @@ module ActionController def normalize_keys(env) new_env = {} env.each_pair { |k, v| new_env[rack_key_for(k)] = rack_value_for(k, v) } + new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" new_env end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 7229c67f30..72e29c2c9d 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -13,10 +13,10 @@ module ActionController end module Live - # Disable controller / rendering threads in tests. User tests can access + # Disable controller / rendering threads in tests. User tests can access # the database on the main thread, so they could open a txn, then the # controller thread will open a new connection and try to access data - # that's only visible to the main thread's txn. This is the problem in #23483 + # that's only visible to the main thread's txn. This is the problem in #23483. remove_method :new_controller_thread def new_controller_thread # :nodoc: yield @@ -35,7 +35,7 @@ module ActionController attr_reader :controller_class - # Create a new test request with default `env` values + # Create a new test request with default `env` values. def self.create(controller_class) env = {} env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application @@ -131,7 +131,7 @@ module ActionController include Rack::Test::Utils def should_multipart?(params) - # FIXME: lifted from Rack-Test. We should push this separation upstream + # FIXME: lifted from Rack-Test. We should push this separation upstream. multipart = false query = lambda { |value| case value @@ -300,7 +300,7 @@ module ActionController # assert_equal "Dave", cookies[:name] # makes sure that a cookie called :name was set as "Dave" # assert flash.empty? # makes sure that there's nothing in the flash # - # On top of the collections, you have the complete url that a given action redirected to available in <tt>redirect_to_url</tt>. + # On top of the collections, you have the complete URL that a given action redirected to available in <tt>redirect_to_url</tt>. # # For redirects within the same controller, you can even call follow_redirect and the redirect will be followed, triggering another # action call which can then be asserted against. diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 10d733e477..dea6c4482e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1904,7 +1904,7 @@ module ActionDispatch ast = Journey::Parser.parse path mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) - @set.add_route(mapping, ast, as, anchor) + @set.add_route(mapping, as) end def match_root_route(options) @@ -2054,7 +2054,7 @@ module ActionDispatch # your url helper definition, e.g: # # direct :browse, page: 1, size: 10 do |options| - # [ :products, options.merge(params.permit(:page, :size)) ] + # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # # In this instance the `params` object comes from the context in which the the diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index dabc045007..e8f47b8640 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -36,6 +36,8 @@ module ActionDispatch uri.host ||= req.host uri.port ||= req.port unless req.standard_port? + req.commit_flash + body = %(<html><body>You are being <a href="#{ERB::Util.unwrapped_html_escape(uri.to_s)}">redirected</a>.</body></html>) headers = { diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2672cd24ed..118dec2ad4 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -565,7 +565,7 @@ module ActionDispatch routes.empty? end - def add_route(mapping, path_ast, name, anchor) + def add_route(mapping, name) raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i) if name && named_routes[name] @@ -860,8 +860,7 @@ module ActionDispatch params[key] = URI.parser.unescape(value) end end - old_params = req.path_parameters - req.path_parameters = old_params.merge params + req.path_parameters = params app = route.app if app.matches?(req) && app.dispatcher? begin diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 70a5b75781..1bf47d2556 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -2,7 +2,6 @@ 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" @@ -84,12 +83,19 @@ module ActionDispatch include SystemTesting::TestHelpers::SetupAndTeardown include SystemTesting::TestHelpers::ScreenshotHelper + def initialize(*) # :nodoc: + super + self.class.superclass.driver.use + end + def self.start_application # :nodoc: Capybara.app = Rack::Builder.new do map "/" do run Rails.application end end + + SystemTesting::Server.new.run end # System Test configuration options @@ -105,13 +111,12 @@ module ActionDispatch # # 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) + @driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size) end - def self.selenium?(driver) # :nodoc: - driver == :selenium + # Returns the driver object for the initialized system test + def self.driver + @driver ||= SystemTestCase.driven_by(:selenium) end end diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb deleted file mode 100644 index c9a6628516..0000000000 --- a/actionpack/lib/action_dispatch/system_testing/browser.rb +++ /dev/null @@ -1,28 +0,0 @@ -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 index 7c2ad84e19..72d132d64f 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -1,17 +1,32 @@ module ActionDispatch module SystemTesting class Driver # :nodoc: - def initialize(name) + def initialize(name, **options) @name = name + @browser = options[:using] + @screen_size = options[:screen_size] end - def run - register + def use + register if selenium? + setup end private + def selenium? + @name == :selenium + end + def register - Capybara.default_driver = @name + Capybara.register_driver @name do |app| + Capybara::Selenium::Driver.new(app, browser: @browser).tap do |driver| + driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) + end + end + end + + def setup + Capybara.current_driver = @name 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 index ddc961cf84..3078e035a3 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -8,9 +8,20 @@ module ActionDispatch # +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. + # + # The screenshot will be displayed in your console, if supported. + # + # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT+ environment variable to + # control the output. Possible values are: + # * [+inline+ (default)] display the screenshot in the terminal using the + # iTerm image protocol (http://iterm2.com/documentation-images.html). + # * [+simple+] only display the screenshot path. + # This is the default value if the +CI+ environment variables + # is defined. + # * [+artifact+] display the screenshot in the terminal, using the terminal + # artifact format (http://buildkite.github.io/terminal/inline-images/). def take_screenshot save_image - puts "[Screenshot]: #{image_path}" puts display_image end @@ -22,7 +33,7 @@ module ActionDispatch # fails add +take_failed_screenshot+ to the teardown block before clearing # sessions. def take_failed_screenshot - take_screenshot if failed? + take_screenshot if failed? && supports_screenshot? end private @@ -38,14 +49,32 @@ module ActionDispatch page.save_screenshot(Rails.root.join(image_path)) end + def output_type + # Environment variables have priority + output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] || ENV["CAPYBARA_INLINE_SCREENSHOT"] + + # If running in a CI environment, default to simple + output_type ||= "simple" if ENV["CI"] + + # Default + output_type ||= "inline" + + output_type + end + def display_image - if ENV["CAPYBARA_INLINE_SCREENSHOT"] == "artifact" - "\e]1338;url=artifact://#{image_path}\a" - else + message = "[Screenshot]: #{image_path}\n" + + case output_type + when "artifact" + message << "\e]1338;url=artifact://#{image_path}\a\n" + when "inline" 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" + message << "\e]1337;File=name=#{name};height=400px;inline=1:#{image}\a\n" end + + message end def inline_base64(path) @@ -55,6 +84,10 @@ module ActionDispatch def failed? !passed? && !skipped? end + + def supports_screenshot? + Capybara.current_driver != :rack_test + 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 index 491559eedf..187ba2cc5f 100644 --- 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 @@ -10,9 +10,9 @@ module ActionDispatch end def after_teardown - super take_failed_screenshot Capybara.reset_sessions! + super end end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 5fa0b727ab..a3430e210e 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -572,7 +572,7 @@ module ActionDispatch # end # # assert_response :success - # assert_equal({ id: Arcticle.last.id, title: "Ahoy!" }, response.parsed_body) + # assert_equal({ id: Article.last.id, title: "Ahoy!" }, response.parsed_body) # end # end # diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 459b0d6c54..4185ce1a1f 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -439,3 +439,11 @@ class ActiveSupport::TestCase skip message if defined?(JRUBY_VERSION) end end + +class DrivenByRackTest < ActionDispatch::SystemTestCase + driven_by :rack_test +end + +class DrivenBySeleniumWithChrome < ActionDispatch::SystemTestCase + driven_by :selenium, using: :chrome +end diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 2893eb7b91..f17e93a431 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -53,6 +53,15 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } end + test "empty? returns true when params contains no key/value pairs" do + params = ActionController::Parameters.new + assert params.empty? + end + + test "empty? returns false when any params are present" do + refute @params.empty? + end + test "except retains permitted status" do @params.permit! assert @params.except(:person).permitted? @@ -75,6 +84,45 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params[:person].fetch(:name).permitted? end + test "has_key? returns true if the given key is present in the params" do + assert @params.has_key?(:person) + end + + test "has_key? returns false if the given key is not present in the params" do + refute @params.has_key?(:address) + end + + test "has_value? returns true if the given value is present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert params.has_value?("Chicago") + end + + test "has_value? returns false if the given value is not present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + refute @params.has_value?("New York") + end + + test "include? returns true if the given key is present in the params" do + assert @params.include?(:person) + end + + test "include? returns false if the given key is not present in the params" do + refute @params.include?(:address) + end + + test "key? returns true if the given key is present in the params" do + assert @params.key?(:person) + end + + test "key? returns false if the given key is not present in the params" do + refute @params.key?(:address) + end + + test "keys returns an array of the keys of the params" do + assert_equal ["person"], @params.keys + assert_equal ["age", "name", "addresses"], @params[:person].keys + end + test "reject retains permitted status" do assert_not @params.reject { |k| k == "person" }.permitted? end @@ -120,6 +168,21 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params.transform_values { |v| v }.permitted? end + test "value? returns true if the given value is present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert params.value?("Chicago") + end + + test "value? returns false if the given value is not present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + refute params.value?("New York") + end + + test "values returns an array of the values of the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert_equal ["Chicago", "Illinois"], params.values + end + test "values_at retains permitted status" do @params.permit! assert @params.values_at(:person).first.permitted? diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 8920914af1..9f3025587e 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -302,6 +302,31 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "32", @params[:person][:age] end + test "#reverse_merge with parameters" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + merged_params = @params.reverse_merge(default_params) + + assert_equal "1234", merged_params[:id] + refute_predicate merged_params[:person], :empty? + end + + test "not permitted is sticky beyond reverse_merge" do + refute_predicate @params.reverse_merge(a: "b"), :permitted? + end + + test "permitted is sticky beyond reverse_merge" do + @params.permit! + assert_predicate @params.reverse_merge(a: "b"), :permitted? + end + + test "#reverse_merge! with parameters" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + @params.reverse_merge!(default_params) + + assert_equal "1234", @params[:id] + refute_predicate @params[:person], :empty? + end + test "modifying the parameters" do @params[:person][:hometown] = "Chicago" @params[:person][:family] = { brother: "Jonas" } diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index 866600b935..052c974d68 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -19,6 +19,16 @@ class RendererTest < ActiveSupport::TestCase assert_equal controller, renderer.controller end + test "creating with new defaults" do + renderer = ApplicationController.renderer + + new_defaults = { https: true } + new_renderer = renderer.with_defaults(new_defaults).new + content = new_renderer.render(inline: "<%= request.ssl? %>") + + assert_equal "true", content + end + test "rendering with a class renderer" do renderer = ApplicationController.renderer content = renderer.render template: "ruby_template" @@ -103,6 +113,20 @@ class RendererTest < ActiveSupport::TestCase assert_equal "true", content end + test "return valid asset url with defaults" do + renderer = ApplicationController.renderer + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "http://example.org/asset.jpg", content + end + + test "return valid asset url when https is true" do + renderer = ApplicationController.renderer.new https: true + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "https://example.org/asset.jpg", content + end + private def render @render ||= ApplicationController.renderer.method(:render) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 53758a4fbc..64818e6ca1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4913,3 +4913,113 @@ class TestInternalRoutingParams < ActionDispatch::IntegrationTest ) end end + +class FlashRedirectTest < ActionDispatch::IntegrationTest + SessionKey = "_myapp_session" + Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") + + class KeyGeneratorMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.key_generator"] ||= Generator + @app.call(env) + end + end + + class FooController < ActionController::Base + def bar + render plain: (flash[:foo] || "foo") + end + end + + Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + get "/foo", to: redirect { |params, req| req.flash[:foo] = "bar"; "/bar" } + get "/bar", to: "flash_redirect_test/foo#bar" + end + + APP = build_app Routes do |middleware| + middleware.use KeyGeneratorMiddleware + middleware.use ActionDispatch::Session::CookieStore, key: SessionKey + middleware.use ActionDispatch::Flash + middleware.delete ActionDispatch::ShowExceptions + end + + def app + APP + end + + include Routes.url_helpers + + def test_block_redirect_commits_flash + get "/foo", env: { "action_dispatch.key_generator" => Generator } + assert_response :redirect + + follow_redirect! + assert_equal "bar", response.body + end +end + +class TestRecognizePath < ActionDispatch::IntegrationTest + class PageConstraint + attr_reader :key, :pattern + + def initialize(key, pattern) + @key = key + @pattern = pattern + end + + def matches?(request) + request.path_parameters[key] =~ pattern + end + end + + stub_controllers do |routes| + Routes = routes + routes.draw do + get "/hash/:foo", to: "pages#show", constraints: { foo: /foo/ } + get "/hash/:bar", to: "pages#show", constraints: { bar: /bar/ } + + get "/proc/:foo", to: "pages#show", constraints: proc { |r| r.path_parameters[:foo] =~ /foo/ } + get "/proc/:bar", to: "pages#show", constraints: proc { |r| r.path_parameters[:bar] =~ /bar/ } + + get "/class/:foo", to: "pages#show", constraints: PageConstraint.new(:foo, /foo/) + get "/class/:bar", to: "pages#show", constraints: PageConstraint.new(:bar, /bar/) + end + end + + APP = build_app Routes + def app + APP + end + + def test_hash_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/hash/bar") + + assert_equal expected_params, actual_params + end + + def test_proc_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/proc/bar") + + assert_equal expected_params, actual_params + end + + def test_class_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/class/bar") + + assert_equal expected_params, actual_params + end + + private + + def recognize_path(*args) + Routes.recognize_path(*args) + end +end diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index bd8318f5f6..3082d1072b 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -224,7 +224,7 @@ module StaticTests def assert_gzip(file_name, response) expected = File.read("#{FIXTURE_LOAD_PATH}/#{public_path}" + file_name) - actual = Zlib::GzipReader.new(StringIO.new(response.body)).read + actual = ActiveSupport::Gzip.decompress(response.body) assert_equal expected, actual end diff --git a/actionpack/test/dispatch/system_testing/browser_test.rb b/actionpack/test/dispatch/system_testing/browser_test.rb deleted file mode 100644 index b0ad309492..0000000000 --- a/actionpack/test/dispatch/system_testing/browser_test.rb +++ /dev/null @@ -1,10 +0,0 @@ -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 index f0ebdb38db..8f8777b19f 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -6,4 +6,15 @@ class DriverTest < ActiveSupport::TestCase driver = ActionDispatch::SystemTesting::Driver.new(:selenium) assert_equal :selenium, driver.instance_variable_get(:@name) end + + test "initializing the driver with a browser" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400]) + assert_equal :selenium, driver.instance_variable_get(:@name) + assert_equal :chrome, driver.instance_variable_get(:@browser) + assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) + end + + test "selenium? returns false if driver is poltergeist" do + assert_not ActionDispatch::SystemTesting::Driver.new(:poltergeist).send(:selenium?) + end end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index 3b4ea96c4f..a83818fd80 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -1,15 +1,16 @@ require "abstract_unit" require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "capybara/dsl" class ScreenshotHelperTest < ActiveSupport::TestCase test "image path is saved in tmp directory" do - new_test = ActionDispatch::SystemTestCase.new("x") + new_test = DrivenBySeleniumWithChrome.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 = DrivenBySeleniumWithChrome.new("x") new_test.stub :passed?, false do assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path) @@ -17,7 +18,7 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end test "image path does not include failures text if test skipped" do - new_test = ActionDispatch::SystemTestCase.new("x") + new_test = DrivenBySeleniumWithChrome.new("x") new_test.stub :passed?, false do new_test.stub :skipped?, true do @@ -26,3 +27,15 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end end end + +class RackTestScreenshotsTest < DrivenByRackTest + test "rack_test driver does not support screenshot" do + assert_not self.send(:supports_screenshot?) + end +end + +class SeleniumScreenshotsTest < DrivenBySeleniumWithChrome + test "selenium driver supports screenshot" do + assert self.send(:supports_screenshot?) + 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 index a384902a14..1a9421c098 100644 --- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -1,21 +1,13 @@ 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 +class SetDriverToRackTestTest < DrivenByRackTest + test "uses rack_test" do + assert_equal :rack_test, Capybara.current_driver end +end - test "selenium? returns false if driver is poltergeist" do - assert_not ActionDispatch::SystemTestCase.selenium?(:poltergeist) +class SetDriverToSeleniumTest < DrivenBySeleniumWithChrome + test "uses selenium" do + assert_equal :selenium, Capybara.current_driver end end |