Jiri Suchomel write:
> ref: refs/heads/master
> commit fbbae0661d2849bcf063c68aa242e65540689f12
> Author: Jiri Suchomel <[email protected]>
> 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 - [email protected]
> +
> +- enhanced test suite
> +- required postfix
> +- 0.0.3
> +
> +-------------------------------------------------------------------
> Thu Oct 15 11:06:33 CEST 2009 - [email protected]
>
> - 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: [email protected]
For additional commands, e-mail: [email protected]