Jiri Suchomel write:
ref: refs/heads/master commit fbbae0661d2849bcf063c68aa242e65540689f12 Author: Jiri Suchomel <jsuchome@suse.cz> Date: Thu Oct 22 10:30:04 2009 +0200
enhanced test suite required postfix --- plugins/mail_settings/app/models/mail_settings.rb | 18 +++-- .../package/yast2-webservice-mailsettings.changes | 7 ++ .../package/yast2-webservice-mailsettings.spec | 4 +- .../functional/mail_settings_controller_test.rb | 70 ++++++++++++++++++++ .../mail_settings/test/unit/mail_settings_test.rb | 65 +++++++++++++----- 5 files changed, 137 insertions(+), 27 deletions(-)
diff --git a/plugins/mail_settings/app/models/mail_settings.rb b/plugins/mail_settings/app/models/mail_settings.rb index e0a1276..76203b7 100644 --- a/plugins/mail_settings/app/models/mail_settings.rb +++ b/plugins/mail_settings/app/models/mail_settings.rb @@ -15,22 +15,23 @@ class MailSettings include Singleton
def initialize - @password = "" - @user = "" - @smtp_server= "" - @transport_layer_security = "NONE" end
# read the settings from system def read + # reset the settings before read + @password = "" + @user = "" + @smtp_server= "" + @transport_layer_security = "NONE"
^^^ If you must comment some code it is good point where is appropriate to separate it to own function with good name. I think you could create internal method clear or init, which is called from read. (Or you can revert old to old behavior and just call initialize in read such a def read initialize ....
yapi_ret = YastService.Call("YaPI::MailSettings::Read") if yapi_ret.has_key? "SendingMail" sending_mail = yapi_ret["SendingMail"] if sending_mail.has_key? "RelayHost" relay_host = sending_mail["RelayHost"] @smtp_server = relay_host["Name"] - @user = relay_host["Account"] - @password = relay_host["Password"] + @user = relay_host["Account"] if relay_host.has_key? "Account" + @password = relay_host["Password"] if relay_host.has_key? "Password" end @transport_layer_security = sending_mail["TLS"] if sending_mail.has_key? "TLS" end @@ -40,8 +41,9 @@ class MailSettings # Save new mail settings def save(settings)
- settings.each do |k, v| - settings[k] = "" if v.nil? + # fill settings hash if it misses some keys + ["transport_layer_security", "smtp_server", "user", "password"].each do |key| + settings[key] = "" if (!settings.has_key? key) || settings[key].nil? end
if settings["transport_layer_security"] == @transport_layer_security && diff --git a/plugins/mail_settings/package/yast2-webservice-mailsettings.changes b/plugins/mail_settings/package/yast2-webservice-mailsettings.changes index 6f0458c..d076d43 100644 --- a/plugins/mail_settings/package/yast2-webservice-mailsettings.changes +++ b/plugins/mail_settings/package/yast2-webservice-mailsettings.changes @@ -1,4 +1,11 @@ ------------------------------------------------------------------- +Thu Oct 22 10:05:17 CEST 2009 - jsuchome@suse.cz + +- enhanced test suite +- required postfix +- 0.0.3 + +------------------------------------------------------------------- Thu Oct 15 11:06:33 CEST 2009 - jsuchome@suse.cz
- save MaximumMailSize, restart postfix diff --git a/plugins/mail_settings/package/yast2-webservice-mailsettings.spec b/plugins/mail_settings/package/yast2-webservice-mailsettings.spec index ef83baf..d06e58b 100644 --- a/plugins/mail_settings/package/yast2-webservice-mailsettings.spec +++ b/plugins/mail_settings/package/yast2-webservice-mailsettings.spec @@ -15,7 +15,7 @@ Provides: yast2-webservice:/srv/www/yastws/app/controllers/mail_settings_c License: MIT Group: Productivity/Networking/Web/Utilities Autoreqprov: on -Version: 0.0.2 +Version: 0.0.3 Release: 0 Summary: YaST2 - Webservice - Mail Settings Source: www.tar.bz2 @@ -32,7 +32,7 @@ BuildRequires: yast2 yast2-mail %endif
# YaPI::MailServer (standard edition) -Requires: yast2-mail +Requires: yast2-mail postfix
# YaPI::SERVICES (for postfix) %if 0%{?suse_version} == 0 || %suse_version > 1110 diff --git a/plugins/mail_settings/test/functional/mail_settings_controller_test.rb b/plugins/mail_settings/test/functional/mail_settings_controller_test.rb new file mode 100644 index 0000000..2f5f589 --- /dev/null +++ b/plugins/mail_settings/test/functional/mail_settings_controller_test.rb @@ -0,0 +1,70 @@ +require File.expand_path(File.dirname(__FILE__) + "/../test_helper") +require 'test/unit' + + +class MailSettingsControllerTest < ActionController::TestCase + fixtures :accounts + + def setup + @controller = MailSettingsController.new + @request = ActionController::TestRequest.new + # http://railsforum.com/viewtopic.php?id=1719 + @request.session[:account_id] = 1 # defined in fixtures + + @model = MailSettings.instance + @model.stubs(:read).returns(true) + end + + test "check 'show' result" do + + ret = get :show + # success (200 OK) + assert_response :success + + # is returned a valid XML? + ret_hash = Hash.from_xml(ret.body) + assert ret_hash + assert ret_hash.has_key?("mail_settings") + assert ret_hash["mail_settings"].has_key?("smtp_server") + end + + test "put success" do + @model.stubs(:save).with({'smtp_server' => "newserver"}).returns(true) + + ret = put :update, :mail_settings => {:smtp_server => "newserver"} + ret_hash = Hash.from_xml(ret.body) + + assert_response :success + assert ret_hash + assert ret_hash.has_key?("mail_settings") + end + + test "put failure" do + YastService.stubs(:Call).with('YaPI::MailSettings::Write', { + "Changed" => [ "i", 1], + "MaximumMailSize" => [ "i", 10485760], + "SendingMail" => ["a{sv}", { + "Type" => [ "s", "relayhost"], + "TLS" => [ "s", ""], + "RelayHost" => [ "a{sv}", { + "Name" => [ "s", "smtp.newdomain.com"], + "Auth" => [ "i", 0], + "Account" => [ "s", ""], + "Password"=> [ "s", ""] + }] + }] + + }).returns("Unknown mail sending TLS type. Allowed values are: NONE | MAY | MUST | MUST_NOPEERMATCH") + + ret = put :update, :mail_settings => {:smtp_server => "smtp.newdomain.com"} + + ret_hash = Hash.from_xml(ret.body) + + assert_response 503 + assert ret_hash + assert ret_hash.has_key?("error") + assert ret_hash["error"]["type"] == "MAIL_SETTINGS_ERROR" + end + + +end diff --git a/plugins/mail_settings/test/unit/mail_settings_test.rb b/plugins/mail_settings/test/unit/mail_settings_test.rb index f33b9c4..df19b90 100644 --- a/plugins/mail_settings/test/unit/mail_settings_test.rb +++ b/plugins/mail_settings/test/unit/mail_settings_test.rb @@ -11,6 +11,52 @@ class MailSettingsTest < ActiveSupport::TestCase @model.read end
+ def test_read_notls + YastService.stubs(:Call).with('YaPI::MailSettings::Read').returns({ + "SendingMail" => { + "RelayHost" => { + "Name" => "smtp.domain.com" + } + } + }) + ret = @model.read + assert @model.transport_layer_security == "NONE" + end + + def test_read + YastService.stubs(:Call).with('YaPI::MailSettings::Read').returns({ + "SendingMail" => { + "TLS" => "MUST", + "RelayHost" => { + "Name" => "smtp.domain.com" + } + } + }) + ret = @model.read + assert @model.smtp_server == "smtp.domain.com" + assert @model.transport_layer_security == "MUST" + end + + + def test_save_no_change + YastService.stubs(:Call).with('YaPI::MailSettings::Read').returns({ + "SendingMail" => { + "TLS" => "MUST", + "RelayHost" => { + "Name" => "smtp.domain.com" + } + } + }) + ret = @model.read + ret = @model.save({ + "smtp_server" => "smtp.domain.com", + "user" => "", + "password" => "", + "transport_layer_security" => "MUST" + }) + assert ret + end + def test_save YastService.stubs(:Call).with('YaPI::MailSettings::Write', { "Changed" => [ "i", 1], @@ -36,21 +82,6 @@ class MailSettingsTest < ActiveSupport::TestCase assert ret end
- def test_read - YastService.stubs(:Call).with('YaPI::MailSettings::Read').returns({ - "SendingMail" => { - "TLS" => "MUST", - "RelayHost" => { - "Name" => "smtp.domain.com" - } - } - }) - ret = @model.read - assert ret - assert @model.smtp_server == "smtp.domain.com" - assert @model.transport_layer_security == "MUST" - end -
def test_save_failure YastService.stubs(:Call).with('YaPI::MailSettings::Write', { @@ -58,7 +89,7 @@ class MailSettingsTest < ActiveSupport::TestCase "MaximumMailSize" => [ "i", 10485760], "SendingMail" => ["a{sv}", { "Type" => [ "s", "relayhost"], - "TLS" => [ "s", nil], + "TLS" => [ "s", ""], "RelayHost" => [ "a{sv}", { "Name" => [ "s", "smtp.newdomain.com"], "Auth" => [ "i", 0], @@ -67,7 +98,7 @@ class MailSettingsTest < ActiveSupport::TestCase }] }]
- }).returns("TLS cannot be empty") + }).returns("Unknown mail sending TLS type. Allowed values are: NONE | MAY | MUST | MUST_NOPEERMATCH") assert_raise MailSettingsError do @model.save({ "smtp_server" => "smtp.newdomain.com",
I suggest move all DATA from test to Constants, then you can easily see what options is tested and add new specific data which should be tested. like WRITE_DBUS_DATA= { "Changed" => [ "i", 1], "MaximumMailSize" => [ "i", 10485760], "SendingMail" => ["a{sv}", { "Type" => [ "s", "relayhost"], "TLS" => [ "s", ""], "RelayHost" => [ "a{sv}", { "Name" => [ "s", "smtp.newdomain.com"], "Auth" => [ "i", 0], "Account" => [ "s", ""], "Password"=> [ "s", ""] }] }] } -- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, webyast modules language and time -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org