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

[Review] Request from 'schubi2' @ 'yast/yast-yast2/review_160825_timeout_error_reports' #492

Merged
merged 8 commits into from
Aug 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions library/general/src/modules/Report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ def main
@display_yesno_messages = true

# timeouts
@timeout_errors = 0
@timeout_warnings = 0
@timeout_messages = 0
@timeout_yesno_messages = 0
# AutoYaST has different timeout (bnc#887397)
@default_timeout = (Mode.auto || Mode.config) ? 10 : 0
@timeout_errors = 0 # default: Errors stop the installation
@timeout_warnings = @default_timeout
@timeout_messages = @default_timeout
@timeout_yesno_messages = @default_timeout

# logging flags
@log_errors = true
Expand Down Expand Up @@ -230,12 +232,14 @@ def Import(settings)

# timeouts
@timeout_errors = Ops.get_integer(@error_settings, "timeout", 0)
@timeout_warnings = Ops.get_integer(@warning_settings, "timeout", 0)
@timeout_messages = Ops.get_integer(@message_settings, "timeout", 0)
@timeout_warnings = Ops.get_integer(@warning_settings, "timeout",
@default_timeout)
@timeout_messages = Ops.get_integer(@message_settings, "timeout",
@default_timeout)
@timeout_yesno_messages = Ops.get_integer(
@yesno_message_settings,
"timeout",
0
@default_timeout
)

# logging flags
Expand Down
17 changes: 17 additions & 0 deletions library/general/test/data/ay_installation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"errors":
log: true
show: true
timeout: 10
"messages":
log: true
show: true
timeout: 10
"warnings":
log: true
show: true
timeout: 20
"yesno_messages":
log: true
show: true
timeout: 40
13 changes: 13 additions & 0 deletions library/general/test/data/ay_profile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"errors":
log: true
show: true
timeout: 10
"warnings":
log: true
show: true
timeout: 20
"yesno_messages":
log: true
show: true
timeout: 40
17 changes: 17 additions & 0 deletions library/general/test/data/default_ay_installation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"errors":
log: true
show: true
timeout: 0
"messages":
log: true
show: true
timeout: 10
"warnings":
log: true
show: true
timeout: 10
"yesno_messages":
log: true
show: true
timeout: 10
17 changes: 17 additions & 0 deletions library/general/test/data/default_normal_installation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"errors":
log: true
show: true
timeout: 0
"messages":
log: true
show: true
timeout: 0
"warnings":
log: true
show: true
timeout: 0
"yesno_messages":
log: true
show: true
timeout: 0
50 changes: 50 additions & 0 deletions library/general/test/report_test.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#! /usr/bin/env rspec

require_relative "test_helper"
require "yaml"

Yast.import "Report"
Yast.import "Mode"

describe Yast::Report do
before { subject.ClearAll }
Expand Down Expand Up @@ -121,4 +123,52 @@
expect(subject.GetMessages(0, 1, 0, 0)).to match(/Message/)
end
end

describe ".Settings" do
Copy link
Contributor

Choose a reason for hiding this comment

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

You're checking .Export instead of .Settings, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, not only Export but also default settings while normal installation. True, I am using Export for checking the values :-)

DATA_DIR = File.join(File.expand_path(File.dirname(__FILE__)), "data")
let(:ay_profile) { YAML.load_file(File.join(DATA_DIR, "ay_profile.yml")) }
let(:default_normal) { YAML.load_file(File.join(DATA_DIR, "default_normal_installation.yml")) }
let(:default_ay) { YAML.load_file(File.join(DATA_DIR, "default_ay_installation.yml")) }
let(:result_ay) { YAML.load_file(File.join(DATA_DIR, "ay_installation.yml")) }

context "while normal installation" do
it "check default entries" do
allow(Yast::Mode).to receive(:mode).and_return("installation")
subject.main
expect(subject.Export()).to match(default_normal)
end
end

context "while AutoYaST installation" do
before(:each) do
allow(Yast::Mode).to receive(:mode).and_return("autoinstallation")
subject.main
end

it "sets default entries" do
expect(subject.Export()).to match(default_ay)
end
it "check if default entries are not overwritten by empty import" do
subject.Import({})
expect(subject.Export()).to match(default_ay)
end
it "set flags via AutoYaST profile" do
subject.Import(ay_profile)
expect(subject.Export()).to match(result_ay)
end
end

context "while AutoYaST cloning system" do
before(:each) do
allow(Yast::Mode).to receive(:mode).and_return("autoinst_config")
subject.main
end

it "AutoYaST default entries will be cloned" do
# Set timeout for autoyast to 10 seconds (bnc#887397)
expect(subject.Export()).to match(default_ay)
end
end
end

end
8 changes: 8 additions & 0 deletions package/yast2.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
-------------------------------------------------------------------
Thu Aug 25 14:17:13 CEST 2016 - schubi@suse.de

- Set AutoYaST default timeout entries for reporting errors
correctly and do not overwrite AutoYaST profile settings.
(bnc#988949)
- 3.1.204

-------------------------------------------------------------------
Tue Aug 9 11:31:45 UTC 2016 - igonzalezsosa@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2.spec
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


Name: yast2
Version: 3.1.203
Version: 3.1.204
Release: 0
Summary: YaST2 - Main Package
License: GPL-2.0
Expand Down