On 02/06/2012 07:11 PM, Matt Wagner wrote:
On Mon, Feb 06, 2012 at 05:58:44PM +0000, mtaylor(a)redhat.com wrote:
> From: Martyn Taylor<mtaylor(a)redhat.com>
>
> ---
> src/app/models/provider_account.rb | 8 ++++++--
> src/spec/models/provider_account_spec.rb | 8 ++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
> index 89009aa..af107de 100644
> --- a/src/app/models/provider_account.rb
> +++ b/src/app/models/provider_account.rb
> @@ -86,8 +86,12 @@ class ProviderAccount< ActiveRecord::Base
> end
>
> def validate_credentials
> - unless valid_credentials?
> - errors.add(:base, "Login Credentials are Invalid for this
Provider")
> + begin
> + unless valid_credentials?
> + errors.add(:base, "Login Credentials are Invalid for this
Provider")
> + end
> + rescue
> + errors.add(:base, "An error occured when checking provider credentials
please check your setup and try again")
This reads like a run-on sentence. "Please check your setup..." should
begin a new sentence.
Sure
Similarly, since this is Conductor, I think this should be in en.yml.
All error messages are defined in this way in conductor atm, I just
followed suit. I guess in the future though we need some effort in
porting them all to en.ym
> end
> end
>
> diff --git a/src/spec/models/provider_account_spec.rb
b/src/spec/models/provider_account_spec.rb
> index 04b4b1e..49039f4 100644
> --- a/src/spec/models/provider_account_spec.rb
> +++ b/src/spec/models/provider_account_spec.rb
> @@ -64,6 +64,14 @@ describe ProviderAccount do
> provider_account.save.should == false
> end
>
> + it "should add errors when testing credentials fails" do
> + provider_account = Factory.build(:mock_provider_account)
> + provider_account.credentials_hash = {'password' =>
"wrong_password"}
> +
provider_account.stub(:valid_credentials).and_raise("DeltacloudError")
> + provider_account.save.should == false
> + provider_account.errors[:base].should == ["An error occured when checking
provider credentials please check your setup and try again"]
This should be changed to reference en.yml if the above is changed.
> + end
> +
> it "should fail to create a cloud account if fetching of hw profiles
fails" do
> provider_account = Factory.build(:mock_provider_account)
>
provider_account.stub(:populate_hardware_profiles).and_raise(ActiveRecord::RecordInvalid)
> --
> 1.7.6.4
This looks good, but with two nits inline. Since you seem to have taken
off and today is code freeze, I will make these changes myself, test,
and then push.
Ahh great, thank you
-- Matt