You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/08/29 13:24:01 UTC

[PATCH core 1/3] Tests: Added tests for Rack extensions

From: Michal Fojtik <mf...@redhat.com>

* These are tests for Rack middleware extensions that
  Deltacloud use.

* Test are part of 'test:base' Rake task

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/Rakefile                                    |    1 +
 server/tests/helpers/rack/common.rb                |    3 +
 server/tests/helpers/rack/rack_accept_test.rb      |   65 ++++++++++++++++++
 server/tests/helpers/rack/rack_date_test.rb        |   27 ++++++++
 .../tests/helpers/rack/rack_driver_select_test.rb  |   53 ++++++++++++++
 server/tests/helpers/rack/rack_etag_test.rb        |   29 ++++++++
 .../tests/helpers/rack/rack_matrix_params_test.rb  |   72 ++++++++++++++++++++
 7 files changed, 250 insertions(+)
 create mode 100644 server/tests/helpers/rack/common.rb
 create mode 100644 server/tests/helpers/rack/rack_accept_test.rb
 create mode 100644 server/tests/helpers/rack/rack_date_test.rb
 create mode 100644 server/tests/helpers/rack/rack_driver_select_test.rb
 create mode 100644 server/tests/helpers/rack/rack_etag_test.rb
 create mode 100644 server/tests/helpers/rack/rack_matrix_params_test.rb

diff --git a/server/Rakefile b/server/Rakefile
index f38b05e..fb40f12 100644
--- a/server/Rakefile
+++ b/server/Rakefile
@@ -164,6 +164,7 @@ namespace :test do
     end
     t.test_files = FileList[
       'tests/helpers/core_ext/*test.rb',        # Deltacloud extensions (core_ext) and other helpers
+      'tests/helpers/rack/*test.rb',            # Rack extensions Deltacloud use
       'tests/drivers/base/*test.rb',            # Deltacloud drivers API tests
       'tests/drivers/models/*test.rb',          # Deltacloud models tests
       'tests/deltacloud/*test.rb',              # Deltacloud internal API tests
diff --git a/server/tests/helpers/rack/common.rb b/server/tests/helpers/rack/common.rb
new file mode 100644
index 0000000..5d67108
--- /dev/null
+++ b/server/tests/helpers/rack/common.rb
@@ -0,0 +1,3 @@
+require 'sinatra/base'
+require_relative '../../test_helper.rb'
+require_relative '../../../lib/sinatra.rb'
diff --git a/server/tests/helpers/rack/rack_accept_test.rb b/server/tests/helpers/rack/rack_accept_test.rb
new file mode 100644
index 0000000..30a5fca
--- /dev/null
+++ b/server/tests/helpers/rack/rack_accept_test.rb
@@ -0,0 +1,65 @@
+require 'rubygems'
+require 'require_relative' if RUBY_VERSION < '1.9'
+require_relative '../../test_helper.rb'
+require_relative './common.rb'
+
+class TestAcceptApp < Sinatra::Base
+  use Rack::Accept
+  use Rack::MediaType
+  register Rack::RespondTo
+  helpers Rack::RespondTo::Helpers
+  get '/' do
+    respond_to do |format|
+      format.html { 'html' }
+      format.xml { 'xml' }
+      format.json { 'json' }
+    end
+  end
+end
+
+describe TestAcceptApp do
+
+  before do
+    def app; TestAcceptApp; end
+  end
+
+  it 'should return HTML when Accept header requests HTML media type' do
+    header 'Accept', 'text/html'
+    get '/'
+    headers['Content-Type'].must_equal 'text/html'
+    response_body.strip.must_equal 'html'
+  end
+
+  it 'should return HTML when Accept header is set by Firefox' do
+    header 'Accept', 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
+    get '/'
+    headers['Content-Type'].must_equal 'text/html'
+    response_body.strip.must_equal 'html'
+  end
+
+  it 'should return XML when Accept header requests XML media type' do
+    header 'Accept', 'application/xml'
+    get '/'
+    headers['Content-Type'].must_equal 'application/xml'
+    response_body.strip.must_equal 'xml'
+  end
+
+  it 'should return JSON when Accept header requests JSON media type' do
+    header 'Accept', 'application/json'
+    get '/'
+    headers['Content-Type'].must_equal 'application/json'
+    response_body.strip.must_equal 'json'
+  end
+
+  it 'should return default media type when no Accept header is set' do
+    get '/'
+    headers['Content-Type'].must_equal 'application/xml'
+  end
+
+  it 'should return error when unknown Accept header is set' do
+    header 'Accept', 'unknown/header'
+    get '/'
+    status.must_equal 406
+  end
+
+end
diff --git a/server/tests/helpers/rack/rack_date_test.rb b/server/tests/helpers/rack/rack_date_test.rb
new file mode 100644
index 0000000..c011dce
--- /dev/null
+++ b/server/tests/helpers/rack/rack_date_test.rb
@@ -0,0 +1,27 @@
+require 'rubygems'
+require 'require_relative' if RUBY_VERSION < '1.9'
+require_relative '../../test_helper.rb'
+require_relative './common.rb'
+
+class TestDateApp < Sinatra::Base
+  use Rack::Date
+  get '/' do
+    'OK'
+  end
+end
+
+describe TestDateApp do
+
+  before do
+    def app; TestDateApp; end
+  end
+
+  it 'add the Date header to all responses' do
+    get '/'
+    status.must_equal 200
+    response_body.wont_be_empty
+    headers['Date'].wont_be_empty
+    Time.parse(headers['Date']).must_be_instance_of Time
+  end
+
+end
diff --git a/server/tests/helpers/rack/rack_driver_select_test.rb b/server/tests/helpers/rack/rack_driver_select_test.rb
new file mode 100644
index 0000000..47f52ad
--- /dev/null
+++ b/server/tests/helpers/rack/rack_driver_select_test.rb
@@ -0,0 +1,53 @@
+require 'rubygems'
+require 'require_relative' if RUBY_VERSION < '1.9'
+require_relative '../../test_helper.rb'
+require_relative './common.rb'
+
+class TestDriverApp < Sinatra::Base
+  use Rack::DriverSelect
+  get '/' do
+    headers 'Driver' => Thread.current[:driver]
+    headers 'Provider' => Thread.current[:provider]
+    'OK'
+  end
+end
+
+describe TestDriverApp do
+
+  before do
+    def app; TestDriverApp; end
+  end
+
+  it 'should set the driver correctly when using X-Deltacloud-Driver request header' do
+    header 'X-Deltacloud-Driver', 'ec2'
+    get '/'
+    headers['Driver'].wont_be_empty
+    headers['Driver'].must_equal 'ec2'
+    headers['Provider'].must_be_nil
+    header 'X-Deltacloud-Driver', 'test'
+    get '/'
+    headers['Driver'].wont_be_empty
+    headers['Driver'].must_equal 'test'
+    headers['Provider'].must_be_nil
+  end
+
+  it 'should set the provider correctly when using X-Deltacloud-Provider header' do
+    header 'X-Deltacloud-Provider', 'default'
+    get '/'
+    headers['Provider'].wont_be_empty
+    headers['Provider'].must_equal 'default'
+    header 'X-Deltacloud-Provider', 'http://someurl.com:8774/api;1234-1234-1235-1235'
+    get '/'
+    headers['Provider'].wont_be_nil
+    headers['Provider'].must_equal 'http://someurl.com:8774/api;1234-1234-1235-1235'
+  end
+
+  it 'should set both provider and driver' do
+    header 'X-Deltacloud-Provider', 'default'
+    header 'X-Deltacloud-Driver', 'test'
+    get '/'
+    headers['Provider'].must_equal 'default'
+    headers['Driver'].must_equal 'test'
+  end
+
+end
diff --git a/server/tests/helpers/rack/rack_etag_test.rb b/server/tests/helpers/rack/rack_etag_test.rb
new file mode 100644
index 0000000..ba06067
--- /dev/null
+++ b/server/tests/helpers/rack/rack_etag_test.rb
@@ -0,0 +1,29 @@
+require 'rubygems'
+require 'require_relative' if RUBY_VERSION < '1.9'
+require_relative '../../test_helper.rb'
+require_relative './common.rb'
+
+class TestEtagApp < Sinatra::Base
+  use Rack::ETag
+  get '/' do
+    params[:test]
+  end
+end
+
+describe TestEtagApp do
+
+  before do
+    def app; TestEtagApp; end
+  end
+
+  it 'add the ETag header to all responses' do
+    get '/?test=1'
+    status.must_equal 200
+    response_body.wont_be_empty
+    headers['ETag'].wont_be_empty
+    headers['ETag'].must_equal 'c4ca4238a0b923820dcc509a6f75849b'
+    get '/?test=2'
+    headers['ETag'].must_equal 'c81e728d9d4c2f636f067f89cc14862c'
+  end
+
+end
diff --git a/server/tests/helpers/rack/rack_matrix_params_test.rb b/server/tests/helpers/rack/rack_matrix_params_test.rb
new file mode 100644
index 0000000..135544b
--- /dev/null
+++ b/server/tests/helpers/rack/rack_matrix_params_test.rb
@@ -0,0 +1,72 @@
+require 'rubygems'
+require 'require_relative' if RUBY_VERSION < '1.9'
+require_relative '../../test_helper.rb'
+require_relative './common.rb'
+
+require 'json'
+
+class TestMatrixApp < Sinatra::Base
+  use Rack::MatrixParams
+  get '/' do
+    params.to_json
+  end
+  get '/test' do
+    params.to_json
+  end
+  get '/test/books' do
+    params.to_json
+  end
+end
+
+describe TestMatrixApp do
+
+  before do
+    def app; TestMatrixApp; end
+  end
+
+  it 'should set matrix param for entrypoint' do
+    get '/;test=1'
+    status.must_equal 200
+    json['test'].must_equal '1'
+  end
+
+  it 'should set multiple matrix params for entrypoint' do
+    get '/;test=1;foo=bar'
+    status.must_equal 200
+    json['test'].must_equal '1'
+    json['foo'].must_equal 'bar'
+  end
+
+  it 'should set matrix param for first part of URI' do
+    get '/test;foo=bar'
+    status.must_equal 200
+    json['test']['foo'].must_equal 'bar'
+  end
+
+  it 'should set multiple matrix params for first part of URI' do
+    get '/test;foo=bar;test1=blah'
+    status.must_equal 200
+    json['test']['foo'].must_equal 'bar'
+    json['test']['test1'].must_equal 'blah'
+  end
+
+  it 'should set matrix params for the last part of URI' do
+    get '/test/books;foo=bar'
+    status.must_equal 200
+    json['books']['foo'].must_equal 'bar'
+  end
+
+  it 'should set matrix params for multiple parts of URI' do
+    get '/test;test=1/books;foo=bar'
+    status.must_equal 200
+    json['books']['foo'].must_equal 'bar'
+    json['test']['test'].must_equal '1'
+  end
+
+  it 'should handle matrix params with wrong syntax' do
+    get '/test;;;/books;foo=bar'
+    json['books']['foo'].must_equal 'bar'
+    json['test'].must_be_nil
+  end
+
+end
-- 
1.7.10.2


Re: [PATCH core 1/3] Tests: Added tests for Rack extensions

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-08-29 at 13:24 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * These are tests for Rack middleware extensions that
>   Deltacloud use.
> 
> * Test are part of 'test:base' Rake task

ACK to the series.



Re: [PATCH core 3/3] Core: Removed extra quotes from Rack::ETag output

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-08-29 at 13:24 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/sinatra/rack_etag.rb |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Shouldn't that cause a test change, too ? I assume that change was
already folded into 1/3 - doing that destroys bisectability of the tree.

Still ACK (hoping we'll never bisect through this patchset)

David



[PATCH core 3/3] Core: Removed extra quotes from Rack::ETag output

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/sinatra/rack_etag.rb |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/lib/sinatra/rack_etag.rb b/server/lib/sinatra/rack_etag.rb
index 14a8cd4..95bcc4e 100644
--- a/server/lib/sinatra/rack_etag.rb
+++ b/server/lib/sinatra/rack_etag.rb
@@ -42,7 +42,7 @@ module Rack
 
       if etag_status?(status) && etag_body?(body) && !http_caching?(headers)
         digest, body = digest_body(body)
-        headers['ETag'] = %("#{digest}") if digest
+        headers['ETag'] = digest.to_s if digest
       end
 
       if not headers['Cache-Control'] and digest
-- 
1.7.10.2


Re: [PATCH core 2/3] Core: Fixed formatting and params handling in Rack::Accept

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-08-29 at 13:24 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * Fixed wrong indentation of module
> * Fixed case when this module is used outside Deltacloud
> * Fixed case when there are no params in Rack env

Asa small comment: it's really bad to combine whitespace changes and any
other changes in a patch. Still ACK.

David



[PATCH core 2/3] Core: Fixed formatting and params handling in Rack::Accept

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

* Fixed wrong indentation of module
* Fixed case when this module is used outside Deltacloud
* Fixed case when there are no params in Rack env

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/sinatra/rack_accept.rb |  106 ++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/server/lib/sinatra/rack_accept.rb b/server/lib/sinatra/rack_accept.rb
index bcfb4d0..6b77fa8 100644
--- a/server/lib/sinatra/rack_accept.rb
+++ b/server/lib/sinatra/rack_accept.rb
@@ -29,7 +29,7 @@ module Rack
           begin
             assumed_layout = args[1] == :layout
             args[1] = "#{args[1]}.#{@media_type}".to_sym if args[1].is_a?(::Symbol)
-            render_without_format *args, &block
+            render_without_format(*args, &block)
           rescue Errno::ENOENT => e
             raise "ERROR: Missing template: #{args[1]}.#{args[0]}" unless assumed_layout
             raise e
@@ -39,65 +39,65 @@ module Rack
       end
     end
 
-      module Helpers
-
-        # This code was inherited from respond_to plugin
-        # http://github.com/cehoffman/sinatra-respond_to
-        #
-        # This method is used to overide the default content_type returned from
-        # rack-accept middleware.
-        def self.included(klass)
-          klass.class_eval do
-            alias :content_type_without_save :content_type
-            def content_type(*args)
-              content_type_without_save *args
-              request.env['rack-accept.formats'] = { args.first.to_sym => 1 }
-              response['Content-Type']
-            end
+    module Helpers
+
+      # This code was inherited from respond_to plugin
+      # http://github.com/cehoffman/sinatra-respond_to
+      #
+      # This method is used to overide the default content_type returned from
+      # rack-accept middleware.
+      def self.included(klass)
+        klass.class_eval do
+          alias :content_type_without_save :content_type
+          def content_type(*args)
+            content_type_without_save(*args)
+            request.env['rack-accept.formats'] = { args.first.to_sym => 1 }
+            response['Content-Type']
           end
         end
+      end
 
-        def accepting_formats
-          request.env['rack-accept.formats']
-        end
+      def accepting_formats
+        request.env['rack-accept.formats']
+      end
 
-        def static_file?(path)
-          public_dir = File.expand_path(settings.public)
-          path = File.expand_path(File.join(public_dir, unescape(path)))
-          path[0, public_dir.length] == public_dir && File.file?(path)
+      def static_file?(path)
+        public_dir = File.expand_path(settings.public)
+        path = File.expand_path(File.join(public_dir, unescape(path)))
+        path[0, public_dir.length] == public_dir && File.file?(path)
+      end
+
+      def respond_to(&block)
+        wants = {}
+        def wants.method_missing(type, *args, &handler)
+          self[type] = handler
         end
+        yield wants
 
-        def respond_to(&block)
-          wants = {}
-          def wants.method_missing(type, *args, &handler)
-            self[type] = handler
-          end
-          yield wants
-          if Deltacloud.default_frontend.name == :cimi
-            @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
-          end
-          @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
-            wants.keys.include?(format) == true
-          end.first
-          if @media_type and @media_type.kind_of? Symbol
-            @media_type = [ @media_type ]
-          end
-          if @media_type and @media_type[0]
-            @media_type = @media_type[0]
-            if  Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type]
-              headers 'Content-Type' => Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type][:return]
-            else
-              headers 'Content-Type' => 'application/xml'
-            end
-            wants[@media_type.to_sym].call if wants[@media_type.to_sym]
+        if Deltacloud.default_frontend.name == :cimi
+          @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
+        end if Deltacloud.respond_to? :default_frontend
+
+        @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
+          wants.keys.include?(format) == true
+        end.first
+        if @media_type and @media_type.kind_of? Symbol
+          @media_type = [ @media_type ]
+        end
+        if @media_type and @media_type[0]
+          @media_type = @media_type[0]
+          if  Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type]
+            headers 'Content-Type' => Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type][:return]
           else
-            headers 'Content-Type' => nil
-            status 406
+            headers 'Content-Type' => 'application/xml'
           end
+          wants[@media_type.to_sym].call if wants[@media_type.to_sym]
+        else
+          headers 'Content-Type' => nil
+          status 406
         end
-
+      end
     end
-
   end
 
   class MediaType < Sinatra::Base
@@ -119,7 +119,7 @@ module Rack
       accept, index = env['rack-accept.request'], {}
 
       # Skip everything when 'format' parameter is set in URL
-      if env['rack.request.query_hash']["format"]
+      if env['rack.request.query_hash'] and env['rack.request.query_hash']["format"]
          media_type = case env['rack.request.query_hash']["format"]
             when 'html' then :html
             when 'xml' then :xml
@@ -135,8 +135,8 @@ module Rack
         sorted_media_types << 'application/xml' if sorted_media_types.empty?
         # Choose the right format with the media type according to the priority
         ACCEPTED_MEDIA_TYPES.each do |format, definition|
-          definition[:match].each do |media_type|
-            break if index[format] = sorted_media_types.index(media_type)
+          definition[:match].each do |mt|
+            break if index[format] = sorted_media_types.index(mt)
           end
         end
         # Reject formats with no/nil priority
-- 
1.7.10.2