Please review pull request #197: Ticket/1.6.x/14338 trace should work within setcode opened by (kbarber)
Description:
This patch fixes the case whereby you want to see stack traces for failures
within a setcode block. Errors were hard to follow without this, and the option
--trace should have really worked but I don't think it ever did for this
particular case.
Now, we have a global option for tracing in Facter - and when it is enabled
Facter::Util::Resolution modifies its returned error during a rescue to include
the full backtrace debugging instead of just the main error.
This also correct the behaviour whereby warnings weren't raised as
Facter.warn, they were instead using kernel warnings so they were not
managed properly.
To facilitate the Facter#tracing method I have cherry-picked the bitcheck
feature from a development branch as its more robust and requires less copy and
paste coding. Future code was going to use this anyway.
- Opened: Sun May 06 23:24:31 UTC 2012
- Based on: puppetlabs:1.6.x (0f84b54df8f955c80dcb5de59adda985be2dc9b1)
- Requested merge: kbarber:ticket/1.6.x/14338_trace_should_work_within_setcode (9d9720fb16a7f9f32e920f4789362322ab34c551)
Diff follows:
diff --git a/lib/facter.rb b/lib/facter.rb
index b197c75..c76f02b 100644
--- a/lib/facter.rb
+++ b/lib/facter.rb
@@ -48,6 +48,7 @@ module Util; end
@@debug = 0
@@timing = 0
@@messages = {}
+ @@tracing = 0
# module methods
@@ -86,6 +87,10 @@ def self.timing?
@@timing != 0
end
+ def self.tracing?
+ @@tracing != 0
+ end
+
# Return a fact object by name. If you use this, you still have to call
# 'value' on it to retrieve the actual value.
def self.[](name)
@@ -209,6 +214,38 @@ def self.timing(bit)
end
end
+ # Turn tracing on or off
+ def self.tracing(bit)
+ @@tracing = bitcheck(bit)
+ end
+
+ # This convenience method allows us to support the bit setting methodology
+ # for global facter settings.
+ def self.bitcheck(bit)
+ if bit
+ case bit
+ when TrueClass; return 1
+ when FalseClass; return 0
+ when Fixnum
+ if bit > 0
+ return 1
+ else
+ return 0
+ end
+ when String;
+ if bit.downcase == 'off'
+ return 0
+ else
+ return 1
+ end
+ else
+ return 0
+ end
+ else
+ return 0
+ end
+ end
+
def self.warn(msg)
if Facter.debugging? and msg and not msg.empty?
msg = [msg] unless msg.respond_to? :each
diff --git a/lib/facter/application.rb b/lib/facter/application.rb
index 3f0f375..d5992c1 100644
--- a/lib/facter/application.rb
+++ b/lib/facter/application.rb
@@ -58,7 +58,7 @@ def self.run(argv)
end
rescue => e
- if options && options[:trace]
+ if Facter.tracing?
raise e
else
$stderr.puts "Error: #{e}"
@@ -73,7 +73,7 @@ def self.parse(argv)
OptionParser.new do |opts|
opts.on("-y", "--yaml") { |v| options[:yaml] = v }
opts.on("-j", "--json") { |v| options[:json] = v }
- opts.on( "--trace") { |v| options[:trace] = v }
+ opts.on( "--trace") { |v| Facter.tracing(1) }
opts.on("-d", "--debug") { |v| Facter.debugging(1) }
opts.on("-t", "--timing") { |v| Facter.timing(1) }
opts.on("-p", "--puppet") { |v| load_puppet }
diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb
index 0829b10..ee2a729 100644
--- a/lib/facter/util/resolution.rb
+++ b/lib/facter/util/resolution.rb
@@ -165,7 +165,7 @@ def value
end
end
rescue Timeout::Error => detail
- warn "Timed out seeking value for %s" % self.name
+ Facter.warn "Timed out seeking value for %s" % self.name
# This call avoids zombies -- basically, create a thread that will
# dezombify all of the child processes that we're ignoring because
@@ -173,7 +173,9 @@ def value
Thread.new { Process.waitall }
return nil
rescue => details
- warn "Could not retrieve %s: %s" % [self.name, details]
+ base_error = ["Could not retrieve %s: %s" % [self.name, details]]
+ base_error.push(*details.backtrace) if Facter.tracing?
+ Facter.warn(base_error)
return nil
end
diff --git a/spec/unit/facter_spec.rb b/spec/unit/facter_spec.rb
index c6cedd3..6129491 100755
--- a/spec/unit/facter_spec.rb
+++ b/spec/unit/facter_spec.rb
@@ -208,6 +208,27 @@
end
end
+ describe "when using bitcheck method" do
+ it "should return true if set to 1" do
+ Facter.bitcheck(1).should == 1
+ end
+ it "should return true if set to true" do
+ Facter.bitcheck(1).should == 1
+ end
+ it "should return true if any string except off" do
+ Facter.bitcheck('aaaaa').should == 1
+ end
+ it "should return false if set to 0" do
+ Facter.bitcheck(0).should be_zero
+ end
+ it "should return false if set to false" do
+ Facter.bitcheck(false).should be_zero
+ end
+ it "should return false if set to off" do
+ Facter.bitcheck('off').should be_zero
+ end
+ end
+
describe "when setting debugging mode" do
it "should have debugging enabled using 1" do
Facter.debugging(1)
@@ -254,6 +275,25 @@
end
end
+ describe "when setting tracing mode" do
+ it "should have tracing enabled using 1" do
+ Facter.tracing(1)
+ Facter.should be_tracing
+ end
+ it "should have tracing enabled using true" do
+ Facter.tracing(true)
+ Facter.should be_tracing
+ end
+ it "should have tracing disabled using 0" do
+ Facter.tracing(0)
+ Facter.should_not be_tracing
+ end
+ it "should have tracing disabled using false" do
+ Facter.tracing(false)
+ Facter.should_not be_tracing
+ end
+ end
+
describe "when registering directories to search" do
after { Facter.instance_variable_set("@search_path", []) }
diff --git a/spec/unit/util/resolution_spec.rb b/spec/unit/util/resolution_spec.rb
index eba0352..35335bd 100755
--- a/spec/unit/util/resolution_spec.rb
+++ b/spec/unit/util/resolution_spec.rb
@@ -146,7 +146,7 @@
describe "and the code is a block" do
it "should warn but not fail if the code fails" do
@resolve.setcode { raise "feh" }
- @resolve.expects(:warn)
+ Facter.expects(:warn)
@resolve.value.should be_nil
end
@@ -175,7 +175,7 @@
end
it "should timeout after the provided timeout" do
- @resolve.expects(:warn)
+ Facter.expects(:warn)
@resolve.timeout = 0.1
@resolve.setcode { sleep 2; raise "This is a test" }
Thread.expects(:new).yields
@@ -193,6 +193,28 @@
@resolve.value
end
+
+ it 'include a stack trace when tracing is on' do
+ Facter.tracing(1)
+ @resolve.setcode { raise 'This is a test' }
+
+ # This basically ensures the array returns isn't 1 (indicating no stack
+ # trace) its not great, but I can't see a better way to do this in
+ # mocha.
+ Facter.expects(:warn).with(Not(responds_with(:length, 1)))
+
+ @resolve.value
+ end
+
+ it 'do not include a stack trace when tracing is off' do
+ Facter.tracing(0)
+ @resolve.setcode { raise 'This is a test' }
+
+ # This basically ensures the array length is only 1
+ Facter.expects(:warn).with(responds_with(:length, 1))
+
+ @resolve.value
+ end
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
