Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bsc#1036440: Warning messages shouldn't open UI in command-line mode #580

Merged
merged 8 commits into from May 30, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions library/general/src/modules/Report.rb
Expand Up @@ -45,6 +45,7 @@ def main
Yast.import "Mode"
Yast.import "Popup"
Yast.import "Summary"
Yast.import "CommandLine"

# stored messages
@errors = []
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
105 changes: 105 additions & 0 deletions 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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for consistence, I would use :Warning symbol instead of the string.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are going to mock Yast::Mode.commandline in all cases, maybe you would like to move it to a before block and just define the value for each case.

before do
  allow(Yast::Mode).to receive(:commandline).and_return(commandline?)
end

context "while in UI mode and timeout is disabled" do
  let(:timeout) { 0 }
  let(:commandline?) { false }

  it "shows a popup" do
    #...
  end
end

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same than above. I would move this line to the outer before block.

end

it "prints the message only on console" do
expect(Yast::CommandLine).to receive("Print")
.with(/#{message}/)
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 }

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
7 changes: 7 additions & 0 deletions 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually write it like this:

- Warning messages shouldn't open UI in command-line mode (bsc#1036440)

mode
- 3.1.155.8

-------------------------------------------------------------------
Tue May 9 11:44:54 UTC 2017 - mvidner@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2.spec
Expand Up @@ -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

Expand Down