[PATCH conductor] Fixed provider's valid_framework check

Imre Farkas ifarkas at redhat.com
Fri Oct 5 09:44:21 UTC 2012


On 10/03/2012 05:08 PM, jprovazn at redhat.com wrote:
> From: Jan Provaznik <jprovazn at redhat.com>
>
> This patch fixes wrong logical check (made by me in an older patch)
> which swaped true/false when teting valid connection to dc-core.
>
> This patch also removes monkey patch for tests which always returned true
> for 'valid_framework?' method.
> ---
>   src/app/models/provider.rb                        | 20 ++++++++++----------
>   src/spec/controllers/providers_controller_spec.rb |  2 ++
>   src/spec/factories/provider.rb                    | 17 +++++++++++++----
>   src/spec/models/provider_spec.rb                  | 11 ++++++++++-
>   src/spec/spec_helper.rb                           |  4 ----
>   5 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
> index 05ff0e4..474041b 100644
> --- a/src/app/models/provider.rb
> +++ b/src/app/models/provider.rb
> @@ -247,6 +247,16 @@ class Provider < ActiveRecord::Base
>
>     end
>
> +  def valid_framework?
> +    begin
> +      connect!.nil? ? false : true
I think it would be more convenient to write this line as !connect!.nil? 
or connect!.present?

> +    rescue Exception => e
> +      logger.error("Error connecting to framework: #{e.message}")
> +      logger.error("Backtrace: #{e.backtrace.join("\n")}")
> +      e.message =~ /401 : Not authorized/ ? true : false
> +    end
> +  end
> +
>     protected
>
>     def stop_instances(user)
> @@ -281,16 +291,6 @@ class Provider < ActiveRecord::Base
>
>     private
>
> -  def valid_framework?
> -    begin
> -      connect!.nil?
> -    rescue Exception => e
> -      logger.error("Error connecting to framework: #{e.message}")
> -      logger.error("Backtrace: #{e.backtrace.join("\n")}")
> -      e.message =~ /401 : Not authorized/ ? true : false
> -    end
> -  end
> -
>     def valid_provider?
>       if !nil_or_empty(deltacloud_provider)
>         client = connect
> diff --git a/src/spec/controllers/providers_controller_spec.rb b/src/spec/controllers/providers_controller_spec.rb
> index 2249ee7..65e73c4 100644
> --- a/src/spec/controllers/providers_controller_spec.rb
> +++ b/src/spec/controllers/providers_controller_spec.rb
> @@ -199,6 +199,7 @@ describe ProvidersController do
>
>           describe "#create" do
>             before(:each) do
> +            Provider.any_instance.stub(:valid_framework?).and_return(true)
>               post :create, :provider => { :name => provider.name,
>                 :url => provider.url,
>                 :provider_type => {
> @@ -293,6 +294,7 @@ describe ProvidersController do
>             context "existing provider" do
>
>               before(:each) do
> +              Provider.any_instance.stub(:valid_framework?).and_return(true)
>                 put :update, :id => orig_provider.id, :provider => provider_data
>               end
>
> diff --git a/src/spec/factories/provider.rb b/src/spec/factories/provider.rb
> index c607540..3f27e9b 100644
> --- a/src/spec/factories/provider.rb
> +++ b/src/spec/factories/provider.rb
> @@ -20,13 +20,16 @@ FactoryGirl.define do
>       sequence(:name) { |n| "mock#{n}" }
>       provider_type { Factory.build :provider_type }
>       url { |p| "http://www." + p.name + ".com/api" }
> +  end
> +
> +  factory :valid_provider, :parent => :provider do
>       after_build do |p|
>         p.stub(:valid_framework?).and_return(true)
>         p.stub(:valid_provider?).and_return(true)
>       end
>     end
>
> -  factory :mock_provider, :parent => :provider do
> +  factory :mock_provider, :parent => :valid_provider do
>       provider_type {ProviderType.find_by_deltacloud_driver("mock")}
>       url 'http://localhost:3002/api'
>       hardware_profiles { |hp| [hp.association(:mock_hwp1), hp.association(:mock_hwp2)] }
> @@ -37,21 +40,21 @@ FactoryGirl.define do
>       name { '' }
>     end
>
> -  factory :mock_provider2, :parent => :provider do
> +  factory :mock_provider2, :parent => :valid_provider do
>       name 'mock2'
>       provider_type { ProviderType.find_by_deltacloud_driver("mock") }
>       url 'http://localhost:3002/api'
>       after_create { |p| p.realms << FactoryGirl.create(:realm3, :provider => p) }
>     end
>
> -  factory :mock_provider_with_unavailable_realm, :parent => :provider do
> +  factory :mock_provider_with_unavailable_realm, :parent => :valid_provider do
>       provider_type {ProviderType.find_by_deltacloud_driver("mock")}
>       url 'http://localhost:3002/api'
>       hardware_profiles { |hp| [hp.association(:mock_hwp1), hp.association(:mock_hwp2)] }
>       after_create { |p| p.realms << FactoryGirl.create(:realm1, :provider => p, :available => false) }
>     end
>
> -  factory :ec2_provider, :parent => :provider do
> +  factory :ec2_provider, :parent => :valid_provider do
>       name 'ec2-amazon'
>       provider_type { ProviderType.find_by_deltacloud_driver("ec2") }
>       url 'http://localhost:3002/api'
> @@ -72,4 +75,10 @@ FactoryGirl.define do
>     factory :unavailable_mock_provider, :parent => :mock_provider do
>       available false
>     end
> +
> +  factory :inaccessible_provider, :parent => :provider do
> +    after_build do |p|
> +      p.stub(:connect!).and_return(nil)
> +    end
> +  end
>   end
> diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb
> index dac9da2..5624ae0 100644
> --- a/src/spec/models/provider_spec.rb
> +++ b/src/spec/models/provider_spec.rb
> @@ -114,7 +114,6 @@ describe Provider do
>         errs.should_not be_empty
>         provider1.enabled.should be_true
>       end
> -
>     end
>
>     context "(using original connect method)" do
> @@ -165,4 +164,14 @@ describe Provider do
>       end
>     end
>
> +  context "inaccessible deltacloud server" do
> +    before(:each) do
> +      @provider = FactoryGirl.create(:inaccessible_provider)
> +    end
> +
> +    it "should not be valid framework" do
> +      @provider.valid_framework?.should be_false
> +    end
> +  end
> +
>   end
> diff --git a/src/spec/spec_helper.rb b/src/spec/spec_helper.rb
> index f9e2840..c24af51 100644
> --- a/src/spec/spec_helper.rb
> +++ b/src/spec/spec_helper.rb
> @@ -74,10 +74,6 @@ end
>   # Without these here, controller specs fail.  These 2 class_evals can
>   # be removed once stubs work and/or are added to the right place.
>   Provider.class_eval do
> -  def valid_framework?
> -    true
> -  end
> -
>     def valid_provider?
>       true
>     end
>

ACK with one inline note



More information about the aeolus-devel mailing list