Mailinglist Archive: yast-devel (163 mails)

< Previous Next >
[yast-devel] Re: [yast-commit] <rest-service> master : more unit tests, exception handling
  • From: josef reidinger <jreidinger@xxxxxxx>
  • Date: Fri, 18 Sep 2009 12:35:40 +0200
  • Message-id: <4AB3627C.4020608@xxxxxxx>
Jiri Suchomel napsal(a):
ref: refs/heads/master
commit 942d45274e1c9ca229bba8768436cced98d17e56
Author: Jiri Suchomel <jsuchome@xxxxxxx>
Date: Fri Sep 18 09:41:46 2009 +0200

more unit tests, exception handling
---
.../app/controllers/administrator_controller.rb | 2 +-
plugins/administrator/app/models/administrator.rb | 4 +-
.../administrator/test/unit/administrator_test.rb | 27
+++++++++++++++++++-
3 files changed, 29 insertions(+), 4 deletions(-)

diff --git
a/plugins/administrator/app/controllers/administrator_controller.rb
b/plugins/administrator/app/controllers/administrator_controller.rb
index 5a06f0a..3478c45 100644
--- a/plugins/administrator/app/controllers/administrator_controller.rb
+++ b/plugins/administrator/app/controllers/administrator_controller.rb
@@ -37,7 +37,7 @@ class AdministratorController < ApplicationController
begin
@admin.save_aliases(data[:aliases])
rescue Exception => e
- render ErrorResult.error(500, 2, e.message) and return
+ render ErrorResult.error(404, 2, e.message) and return

Please don't return 404 when error occure, this is quite confusing. I
write separate mail about error handling when I mention it. If you use
my second suggestion you don't need catch anything unless you want
somehow rescue from it (which render is not).

end
end
show
diff --git a/plugins/administrator/app/models/administrator.rb
b/plugins/administrator/app/models/administrator.rb
index 2327ebe..7580965 100644
--- a/plugins/administrator/app/models/administrator.rb
+++ b/plugins/administrator/app/models/administrator.rb
@@ -35,7 +35,7 @@ class Administrator
new_aliases = "" if new_aliases.nil? || new_aliases == "NONE"
if @aliases.split(",").sort == new_aliases.split(",").sort
Rails.logger.debug "mail aliases have not been changed"
- return
+ return true
end
parameters = {
"aliases" => ["as", new_aliases.split(",")]
@@ -43,7 +43,7 @@ class Administrator
yapi_ret = YastService.Call("YaPI::ADMINISTRATOR::Write", parameters)
Rails.logger.debug "YaPI returns: '#{yapi_ret}'"

- raise yapi_ret unless yapi_ret.empty?
+ raise Exception.new(yapi_ret) unless yapi_ret.empty?

Don't raise anonymous exception, please use error handling feature and
define own exception with proper handling. for example see how I handle
response from NTP:
http://git.opensuse.org/?p=projects/yast/rest-service.git;a=blob;f=plugins/ntp/app/models/ntp.rb;h=790b01d0513f9e1b8552d48e8e0a8f3dae538074;hb=3227e82ff5129bb3f0bd15af086b6a91fc8cf448

@aliases = new_aliases
end

diff --git a/plugins/administrator/test/unit/administrator_test.rb
b/plugins/administrator/test/unit/administrator_test.rb
index 008977d..374b1d2 100644
--- a/plugins/administrator/test/unit/administrator_test.rb
+++ b/plugins/administrator/test/unit/administrator_test.rb
@@ -6,6 +6,8 @@ class AdministratorTest < ActiveSupport::TestCase

def setup
@model = Administrator.instance
+ YastService.stubs(:Call).with('YaPI::ADMINISTRATOR::Read').returns({
"aliases" => [ "a@b" ] })
+ @model.read_aliases
end

def test_save_password
@@ -15,7 +17,7 @@ class AdministratorTest < ActiveSupport::TestCase
end

def test_read_aliases
- YastService.stubs(:Call).with('YaPI::ADMINISTRATOR::Read').returns({
"aliases" => [ "a@b" ] })
+ YastService.stubs(:Call).with('YaPI::ADMINISTRATOR::Read').returns({
"aliases" => [ "a@xxx" ] })
ret = @model.read_aliases
assert ret
assert @model.aliases.split(",").size == 1
@@ -29,4 +31,27 @@ class AdministratorTest < ActiveSupport::TestCase
assert @model.aliases.split(",").size == 2
end

+ def test_save_empty_aliases
+ new_aliases = "NONE"
+ YastService.stubs(:Call).with('YaPI::ADMINISTRATOR::Write', {"aliases"
=> [ "as", [] ]}).returns("")
+ ret = @model.save_aliases(new_aliases)
+ assert ret
+ assert @model.aliases.empty?
+ end
+
+ def test_save_failure
+ new_aliases = [ "test@xxxxxxxxxx" ];
+ YastService.stubs(:Call).with('YaPI::ADMINISTRATOR::Write', {"aliases"
=> [ "as", new_aliases ]}).returns("YaPI error")
+ assert_raise Exception do
+ ret = @model.save_aliases(new_aliases.join(","))
+ end
+ end
+
+ def test_save_no_change
+ new_aliases = [ "a@b" ];
+ ret = @model.save_aliases(new_aliases.join(","))
+ assert ret
+ assert @model.aliases.split(",").size == 1
+ end
+
end

--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
For additional commands, e-mail: yast-devel+help@xxxxxxxxxxxx

< Previous Next >
This Thread
  • No further messages