From 0a6985f3fbe174c3c15d0d191ccf7de2093c5bb7 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 29 Jul 2016 14:35:13 +0200 Subject: [PATCH 01/12] Use a better exception if Popup.Feedback does not get a block Before: NoMethodError: undefined method `call' for nil:NilClass After: ArgumentError: block must be supplied This fixes a RSpec warning about testing a generic exception: Using the `raise_error` matcher without providing a specific error or message --- library/general/src/modules/Popup.rb | 2 ++ library/general/test/popup_test.rb | 40 +++++++++++++++------------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/library/general/src/modules/Popup.rb b/library/general/src/modules/Popup.rb index 988228b6a..2f7acfd00 100644 --- a/library/general/src/modules/Popup.rb +++ b/library/general/src/modules/Popup.rb @@ -1076,6 +1076,8 @@ def ShowFeedback(headline, message) # @param message [String] message with details, displayed below the headline # @param block block to execute def Feedback(headline, message, &block) + raise ArgumentError, "block must be supplied" unless block + ShowFeedback(headline, message) block.call ensure diff --git a/library/general/test/popup_test.rb b/library/general/test/popup_test.rb index 03da42e7b..136c589ff 100755 --- a/library/general/test/popup_test.rb +++ b/library/general/test/popup_test.rb @@ -14,26 +14,30 @@ end describe ".Feedback" do - before do - expect(ui).to receive(:OpenDialog) - expect(ui).to receive(:CloseDialog) - allow(ui).to receive(:BusyCursor) - allow(ui).to receive(:GetDisplayInfo).and_return({}) + context "when arguments are good" do + before do + expect(ui).to receive(:OpenDialog) + expect(ui).to receive(:CloseDialog) + allow(ui).to receive(:BusyCursor) + allow(ui).to receive(:GetDisplayInfo).and_return({}) + end + + it "opens a popup dialog and closes it at the end" do + # just pass an empty block + subject.Feedback("Label", "Message") {} + end + + it "closes the popup even when an exception occurs in the block" do + # raise an exception in the block + expect { subject.Feedback("Label", "Message") { raise "TEST" } }.to raise_error(RuntimeError, "TEST") + end end - it "opens a popup dialog and closes it at the end" do - # just pass an empty block - subject.Feedback("Label", "Message") {} - end - - it "closes the popup even when an exception occurs in the block" do - # raise an exception in the block - expect { subject.Feedback("Label", "Message") { raise "TEST" } }.to raise_error(RuntimeError, "TEST") - end - - it "raises exception when the block parameter is missing" do - # no block passed - expect { subject.Feedback("Label", "Message") }.to raise_error + context "when arguments are bad" do + it "raises exception when the block parameter is missing" do + # no block passed + expect { subject.Feedback("Label", "Message") }.to raise_error(ArgumentError, /block must be supplied/) + end end end end From 339d5153e133cb125e62e900fe1830d3ba8758b8 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 29 Jul 2016 14:55:42 +0200 Subject: [PATCH 02/12] Fixed ArgumentError in Popup.AnyTimedMessage (bsc#988739). It appears that the bug was introduced when we did a large scale change (removing icons from dialogs) which was not covered with tests. d4f52319ccdf9c5b3e0f393fbedd95b3b728f82d This check found one other such bug and no others: ```console $ ruby-lint -a argument_amount ./library/general/src/modules/Popup.rb .../yast2/library/general/src/modules/Popup.rb:1771:7:error:wrong number of arguments (expected 3 but got 4) .../yast2/library/general/src/modules/Popup.rb:1777:7:error:wrong number of arguments (expected 5 but got 6) ``` --- library/general/src/modules/Popup.rb | 3 +-- library/general/test/popup_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/library/general/src/modules/Popup.rb b/library/general/src/modules/Popup.rb index 2f7acfd00..9784e2272 100644 --- a/library/general/src/modules/Popup.rb +++ b/library/general/src/modules/Popup.rb @@ -1778,7 +1778,7 @@ def ModuleError(text) # @return [void] # def AnyTimedMessage(headline, message, timeout) - anyTimedMessageInternal(headline, message, nil, timeout) + anyTimedMessageInternal(headline, message, timeout) nil end @@ -1787,7 +1787,6 @@ def AnyTimedRichMessage(headline, message, timeout) anyTimedRichMessageInternal( headline, message, - nil, timeout, @default_width, @default_height diff --git a/library/general/test/popup_test.rb b/library/general/test/popup_test.rb index 136c589ff..63e664615 100755 --- a/library/general/test/popup_test.rb +++ b/library/general/test/popup_test.rb @@ -40,4 +40,20 @@ end end end + + describe ".AnyTimedMessage" do + it "is an adapter for anyTimedMessageInternal" do + expect(subject).to receive(:anyTimedMessageInternal) + .with("headline", "message", Integer) + expect(subject.AnyTimedMessage("headline", "message", 5)).to eq nil + end + end + + describe ".AnyTimedRichMessage" do + it "is an adapter for anyTimedRichMessageInternal" do + expect(subject).to receive(:anyTimedRichMessageInternal) + .with("headline", "message", Integer, Integer, Integer) + expect(subject.AnyTimedRichMessage("headline", "message", 5)).to eq nil + end + end end From 85814222b5cee5f5c9a7d3c85e76c4233dfd368f Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 29 Jul 2016 15:00:30 +0200 Subject: [PATCH 03/12] version + changelog --- package/yast2.changes | 6 ++++++ package/yast2.spec | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/package/yast2.changes b/package/yast2.changes index 3aa2df522..ffaf1d51c 100644 --- a/package/yast2.changes +++ b/package/yast2.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue May 9 11:44:54 UTC 2017 - mvidner@suse.com + +- Fixed ArgumentError in Popup.AnyTimedMessage (bsc#988739). +- 3.1.155.7 + ------------------------------------------------------------------- Thu Jan 5 11:35:23 UTC 2017 - mfilka@suse.com diff --git a/package/yast2.spec b/package/yast2.spec index 24306ba5e..311c635e7 100644 --- a/package/yast2.spec +++ b/package/yast2.spec @@ -17,7 +17,7 @@ Name: yast2 -Version: 3.1.155.6 +Version: 3.1.155.7 Release: 0 Url: https://github.com/yast/yast-yast2 From e774f0d0e3582ed17645afa293a251a6ad5845f1 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Wed, 10 May 2017 09:49:34 +0200 Subject: [PATCH 04/12] Travis: Require gettext.deb for whatever reason the package that contains /usr/bin/xgettext is no longer getting installed automatically. Compare https://travis-ci.org/yast/yast-yast2/builds/209283467 (past good) and https://travis-ci.org/yast/yast-yast2/jobs/230322482 (now bad) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0e0b99d08..9632f6089 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ before_install: # disable rvm, use system Ruby - rvm reset - wget https://raw.githubusercontent.com/yast/yast-devtools/master/travis-tools/travis_setup.sh - - sh ./travis_setup.sh -p "rake yast2-core yast2-devtools yast2-testsuite yast2-ruby-bindings yast2 yast2-pkg-bindings" -g "rspec:3.3.0 yast-rake gettext coveralls rubocop:0.29.1" + - sh ./travis_setup.sh -p "rake yast2-core yast2-devtools yast2-testsuite yast2-ruby-bindings yast2 yast2-pkg-bindings gettext" -g "rspec:3.3.0 yast-rake gettext coveralls rubocop:0.29.1" script: - rake check:pot - rubocop From 0f30117cb6a24977bd33ba476cd502693a923e44 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Mon, 29 May 2017 10:16:28 +0200 Subject: [PATCH 05/12] bsc#1036440: Warning messages shouldn't open UI in command-line mode --- library/general/src/modules/Report.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/general/src/modules/Report.rb b/library/general/src/modules/Report.rb index 32e57af21..c32f05534 100644 --- a/library/general/src/modules/Report.rb +++ b/library/general/src/modules/Report.rb @@ -45,6 +45,7 @@ def main Yast.import "Mode" Yast.import "Popup" Yast.import "Summary" + Yast.import "CommandLine" # stored messages @errors = [] @@ -481,7 +482,9 @@ def Warning(warning_string) Builtins.y2warning(1, "%1", warning_string) if @log_warnings if @display_warnings - if Ops.greater_than(@timeout_warnings, 0) + if Mode.commandline + CommandLine.Print "Warning: #{warning_string}" + elsif Ops.greater_than(@timeout_warnings, 0) Popup.TimedWarning(warning_string, @timeout_warnings) else Popup.Warning(warning_string) @@ -522,8 +525,7 @@ def Error(error_string) if @display_errors if Mode.commandline - Yast.import "CommandLine" - CommandLine.Print error_string + CommandLine.Print "Error: #{error_string}" elsif Ops.greater_than(@timeout_errors, 0) Popup.TimedError(error_string, @timeout_errors) else From b72de0871c291a194f63a57129a2893eb057f657 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Mon, 29 May 2017 10:21:48 +0200 Subject: [PATCH 06/12] Bump version + changelog --- package/yast2.changes | 7 +++++++ package/yast2.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2.changes b/package/yast2.changes index ffaf1d51c..606b394e1 100644 --- a/package/yast2.changes +++ b/package/yast2.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon May 29 08:19:46 UTC 2017 - gsouza@suse.com + +- bsc#1036440: Warning messages shouldn't open UI in command-line + mode +- 3.1.155.8 + ------------------------------------------------------------------- Tue May 9 11:44:54 UTC 2017 - mvidner@suse.com diff --git a/package/yast2.spec b/package/yast2.spec index 311c635e7..5c2ae9d97 100644 --- a/package/yast2.spec +++ b/package/yast2.spec @@ -17,7 +17,7 @@ Name: yast2 -Version: 3.1.155.7 +Version: 3.1.155.8 Release: 0 Url: https://github.com/yast/yast-yast2 From 214b168c70cae2b073b5d031e1d05fef880713c9 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 11:51:39 +0200 Subject: [PATCH 07/12] Added tests to Error and Warning functions in ReportClass --- library/general/test/report_test.rb | 105 ++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 library/general/test/report_test.rb diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb new file mode 100644 index 000000000..d66df767d --- /dev/null +++ b/library/general/test/report_test.rb @@ -0,0 +1,105 @@ +#! /usr/bin/env rspec +# +require_relative "test_helper" +require "yaml" + +Yast.import "Report" +Yast.import "Mode" + +describe Yast::Report do + before { subject.ClearAll } + + describe ".Warning" do + let(:show) { true } + let (:message) { "Message" } + + context "while in command-line mode" do + before(:each) do + allow(Yast::Mode).to receive(:commandline).and_return(true) + end + + it "prints the message only on console" do + expect(Yast::CommandLine).to receive("Print") + .with(/#{message}/) + expect(Yast::Popup).to_not receive("Warning") + expect(Yast::Popup).to_not receive("TimedWarning") + subject.Warning(message) + end + end + + context "while in UI mode and timeout is disabled" do + let(:timeout) { 0 } + + before(:each) do + subject.DisplayWarnings(show, timeout) + allow(Yast::Mode).to receive(:commandline).and_return(false) + end + + it "shows a popup" do + expect(Yast::Popup).to receive("Warning").with(/#{message}/) + subject.Warning(message) + end + end + + context "while in UI mode and timeout is enabled" do + let(:timeout) { 1 } + + before(:each) do + subject.DisplayWarnings(show, timeout) + allow(Yast::Mode).to receive(:commandline).and_return(false) + end + + it "shows timed popup" do + expect(Yast::Popup).to receive("TimedWarning").with(/#{message}/, timeout) + subject.Warning(message) + end + end + end + + describe ".Error" do + let(:show) { true } + let (:message) { "Message" } + + context "while in command-line mode" do + before(:each) do + allow(Yast::Mode).to receive(:commandline).and_return(true) + end + + it "prints the message only on console" do + expect(Yast::CommandLine).to receive("Print") + .with(/#{message}/) + expect(Yast::Popup).to_not receive("Warning") + expect(Yast::Popup).to_not receive("TimedWarning") + subject.Error(message) + end + end + + context "while in UI mode and timeout is disabled" do + let(:timeout) { 0 } + + before(:each) do + subject.DisplayErrors(show, timeout) + allow(Yast::Mode).to receive(:commandline).and_return(false) + end + + it "shows a popup" do + expect(Yast::Popup).to receive("Error").with(/#{message}/) + subject.Error(message) + end + end + + context "while in UI mode and timeout is enabled" do + let(:timeout) { 1 } + + before(:each) do + subject.DisplayErrors(show, timeout) + allow(Yast::Mode).to receive(:commandline).and_return(false) + end + + it "shows a timed popup" do + expect(Yast::Popup).to receive("TimedError").with(/#{message}/, timeout) + subject.Error(message) + end + end + end +end From cf4831cd32308096a64ed8e344ac39f731b38775 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 12:02:44 +0200 Subject: [PATCH 08/12] Makes rubocop happy --- library/general/test/report_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb index d66df767d..a00306f16 100644 --- a/library/general/test/report_test.rb +++ b/library/general/test/report_test.rb @@ -11,7 +11,7 @@ describe ".Warning" do let(:show) { true } - let (:message) { "Message" } + let(:message) { "Message" } context "while in command-line mode" do before(:each) do @@ -58,7 +58,7 @@ describe ".Error" do let(:show) { true } - let (:message) { "Message" } + let(:message) { "Message" } context "while in command-line mode" do before(:each) do From c6dafdc944c56ba8c186d8b5fd0df086db3ff72e Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 12:44:17 +0200 Subject: [PATCH 09/12] Corrected mistake in copy and paste --- library/general/test/report_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb index a00306f16..b6e6182cd 100644 --- a/library/general/test/report_test.rb +++ b/library/general/test/report_test.rb @@ -68,8 +68,8 @@ it "prints the message only on console" do expect(Yast::CommandLine).to receive("Print") .with(/#{message}/) - expect(Yast::Popup).to_not receive("Warning") - expect(Yast::Popup).to_not receive("TimedWarning") + expect(Yast::Popup).to_not receive("Error") + expect(Yast::Popup).to_not receive("TimedError") subject.Error(message) end end From d06a6f6949980507510fad881e08ab9ec170a506 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 13:09:11 +0200 Subject: [PATCH 10/12] Changes due review. --- library/general/test/report_test.rb | 44 ++++++++++++++++------------- package/yast2.changes | 4 +-- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb index b6e6182cd..e73f3c557 100644 --- a/library/general/test/report_test.rb +++ b/library/general/test/report_test.rb @@ -13,44 +13,46 @@ let(:show) { true } let(:message) { "Message" } + before do + allow(Yast::Mode).to receive(:commandline).and_return(commandline?) + end + context "while in command-line mode" do - before(:each) do - allow(Yast::Mode).to receive(:commandline).and_return(true) - end + let(:commandline?) { true } it "prints the message only on console" do - expect(Yast::CommandLine).to receive("Print") + expect(Yast::CommandLine).to receive(:Print) .with(/#{message}/) - expect(Yast::Popup).to_not receive("Warning") - expect(Yast::Popup).to_not receive("TimedWarning") + expect(Yast::Popup).to_not receive(:Warning) + expect(Yast::Popup).to_not receive(:TimedWarning) subject.Warning(message) end end context "while in UI mode and timeout is disabled" do let(:timeout) { 0 } + let(:commandline?) { false } before(:each) do subject.DisplayWarnings(show, timeout) - allow(Yast::Mode).to receive(:commandline).and_return(false) end it "shows a popup" do - expect(Yast::Popup).to receive("Warning").with(/#{message}/) + expect(Yast::Popup).to receive(:Warning).with(/#{message}/) subject.Warning(message) end end context "while in UI mode and timeout is enabled" do let(:timeout) { 1 } + let(:commandline?) { false } before(:each) do subject.DisplayWarnings(show, timeout) - allow(Yast::Mode).to receive(:commandline).and_return(false) end it "shows timed popup" do - expect(Yast::Popup).to receive("TimedWarning").with(/#{message}/, timeout) + expect(Yast::Popup).to receive(:TimedWarning).with(/#{message}/, timeout) subject.Warning(message) end end @@ -60,44 +62,46 @@ let(:show) { true } let(:message) { "Message" } + before do + allow(Yast::Mode).to receive(:commandline).and_return(commandline?) + end + context "while in command-line mode" do - before(:each) do - allow(Yast::Mode).to receive(:commandline).and_return(true) - end + let(:commandline?) { true } it "prints the message only on console" do - expect(Yast::CommandLine).to receive("Print") + expect(Yast::CommandLine).to receive(:Print) .with(/#{message}/) - expect(Yast::Popup).to_not receive("Error") - expect(Yast::Popup).to_not receive("TimedError") + expect(Yast::Popup).to_not receive(:Error) + expect(Yast::Popup).to_not receive(:TimedError) subject.Error(message) end end context "while in UI mode and timeout is disabled" do let(:timeout) { 0 } + let(:commandline?) { false } before(:each) do subject.DisplayErrors(show, timeout) - allow(Yast::Mode).to receive(:commandline).and_return(false) end it "shows a popup" do - expect(Yast::Popup).to receive("Error").with(/#{message}/) + expect(Yast::Popup).to receive(:Error).with(/#{message}/) subject.Error(message) end end context "while in UI mode and timeout is enabled" do let(:timeout) { 1 } + let(:commandline?) { false } before(:each) do subject.DisplayErrors(show, timeout) - allow(Yast::Mode).to receive(:commandline).and_return(false) end it "shows a timed popup" do - expect(Yast::Popup).to receive("TimedError").with(/#{message}/, timeout) + expect(Yast::Popup).to receive(:TimedError).with(/#{message}/, timeout) subject.Error(message) end end diff --git a/package/yast2.changes b/package/yast2.changes index 606b394e1..8bf96ae1a 100644 --- a/package/yast2.changes +++ b/package/yast2.changes @@ -1,8 +1,8 @@ ------------------------------------------------------------------- Mon May 29 08:19:46 UTC 2017 - gsouza@suse.com -- bsc#1036440: Warning messages shouldn't open UI in command-line - mode +- Warning messages shouldn't open UI in command-line (bsc#1036440) + mode. - 3.1.155.8 ------------------------------------------------------------------- From 369bc5e41f2d8822189aaf4d6cc9e6cfb42786e9 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 13:16:21 +0200 Subject: [PATCH 11/12] Makes rubocop happy --- library/general/test/report_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb index e73f3c557..166941cb8 100644 --- a/library/general/test/report_test.rb +++ b/library/general/test/report_test.rb @@ -14,7 +14,7 @@ let(:message) { "Message" } before do - allow(Yast::Mode).to receive(:commandline).and_return(commandline?) + allow(Yast::Mode).to receive(:commandline).and_return(commandline?) end context "while in command-line mode" do @@ -63,9 +63,9 @@ let(:message) { "Message" } before do - allow(Yast::Mode).to receive(:commandline).and_return(commandline?) + allow(Yast::Mode).to receive(:commandline).and_return(commandline?) end - + context "while in command-line mode" do let(:commandline?) { true } From ef4a4f31fbfbf8193284606753a0e600f5d38101 Mon Sep 17 00:00:00 2001 From: Gilson Souza Date: Tue, 30 May 2017 13:22:36 +0200 Subject: [PATCH 12/12] Makes rubocop happy --- library/general/test/report_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/general/test/report_test.rb b/library/general/test/report_test.rb index 166941cb8..f38ebf9c7 100644 --- a/library/general/test/report_test.rb +++ b/library/general/test/report_test.rb @@ -65,7 +65,7 @@ before do allow(Yast::Mode).to receive(:commandline).and_return(commandline?) end - + context "while in command-line mode" do let(:commandline?) { true }