On 10/03/2012 05:08 PM, [email protected] wrote:
From: Jan Provaznik <[email protected]>
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
