[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