Please review pull request #761: Bug/master/13559 two console output styles enter one style leaves opened by (cprice-puppet)
Description:
This just adds one commit to Daniel's previous series, to make sure that we log the context with a log message if there is one.
- Opened: Fri May 11 19:36:36 UTC 2012
- Based on: puppetlabs:master (80c8f0a7b3a8b944388f506861c06efe6656020a)
- Requested merge: cprice-puppet:bug/master/13559-two-console-output-styles-enter-one-style-leaves (eba38472e8fa2c16a7cd48427301f610c8a52f0d)
Diff follows:
diff --git a/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb b/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
index c9ff3e4..4d7c431 100644
--- a/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
+++ b/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
@@ -33,7 +33,7 @@
manifest << node_names.map do |node_name|
%Q[
node "#{node_name}" {
- exec { "echo #{success_message}": }
+ exec { "echo #{success_message}": logoutput => true }
}
]
end.join("\n")
diff --git a/acceptance/tests/allow_arbitrary_node_name_for_apply.rb b/acceptance/tests/allow_arbitrary_node_name_for_apply.rb
index 4daa8a6..f620cd9 100644
--- a/acceptance/tests/allow_arbitrary_node_name_for_apply.rb
+++ b/acceptance/tests/allow_arbitrary_node_name_for_apply.rb
@@ -8,7 +8,7 @@
exec { "false": }
}
node a_different_node_name {
- exec { "echo #{success_message}": }
+ exec { "echo #{success_message}": logoutput => true }
}
]
diff --git a/acceptance/tests/apply/hashes/should_not_reassign.rb b/acceptance/tests/apply/hashes/should_not_reassign.rb
index dd8fc1c..2b0f9cc 100755
--- a/acceptance/tests/apply/hashes/should_not_reassign.rb
+++ b/acceptance/tests/apply/hashes/should_not_reassign.rb
@@ -6,5 +6,5 @@
apply_manifest_on(agents, manifest, :acceptable_exit_codes => [1]) do
fail_test "didn't find the failure" unless
- stdout.include? "Assigning to the hash 'my_hash' with an existing key 'one'"
+ stderr.include? "Assigning to the hash 'my_hash' with an existing key 'one'"
end
diff --git a/acceptance/tests/cycle_detection.rb b/acceptance/tests/cycle_detection.rb
index c59ca9e..44fb11d 100644
--- a/acceptance/tests/cycle_detection.rb
+++ b/acceptance/tests/cycle_detection.rb
@@ -7,7 +7,7 @@
EOT
apply_manifest_on(agents, manifest) do
- assert_match(/Found 1 dependency cycle/, stdout,
+ assert_match(/Found 1 dependency cycle/, stderr,
"found and reported the cycle correctly")
end
@@ -21,6 +21,6 @@
EOT
apply_manifest_on(agents, manifest) do
- assert_match(/Found 2 dependency cycles/, stdout,
+ assert_match(/Found 2 dependency cycles/, stderr,
"found and reported the cycle correctly")
end
diff --git a/acceptance/tests/file_hello_world.rb b/acceptance/tests/file_hello_world.rb
index b64ddd7..11f66e9 100644
--- a/acceptance/tests/file_hello_world.rb
+++ b/acceptance/tests/file_hello_world.rb
@@ -12,7 +12,7 @@
step "run the manifest itself"
apply_manifest_on(agent, manifest) do
fail_test "the expected notice of action was missing" unless
- stdout.index "File[#{filename}]/ensure: defined content as"
+ stderr.index "File[#{filename}]/ensure: defined content as"
end
step "verify the content of the generated files."
diff --git a/acceptance/tests/helpful_error_message_when_hostname_not_match_server_certificate.rb b/acceptance/tests/helpful_error_message_when_hostname_not_match_server_certificate.rb
index 40818a8..02c4664 100644
--- a/acceptance/tests/helpful_error_message_when_hostname_not_match_server_certificate.rb
+++ b/acceptance/tests/helpful_error_message_when_hostname_not_match_server_certificate.rb
@@ -4,6 +4,6 @@
with_master_running_on(master, "--certname foobar_not_my_hostname --dns_alt_names one_cert,two_cert,red_cert,blue_cert --autosign true") do
run_agent_on(agents, "--no-usecacheonfailure --no-daemonize --verbose --onetime --server #{master}", :acceptable_exit_codes => (1..255)) do
msg = "Server hostname '#{master}' did not match server certificate; expected one of foobar_not_my_hostname, DNS:blue_cert, DNS:foobar_not_my_hostname, DNS:one_cert, DNS:red_cert, DNS:two_cert"
- assert_match(msg, stdout)
+ assert_match(msg, stderr)
end
end
diff --git a/acceptance/tests/jeff_append_to_array.rb b/acceptance/tests/jeff_append_to_array.rb
index 20f4366..1fab8ad 100644
--- a/acceptance/tests/jeff_append_to_array.rb
+++ b/acceptance/tests/jeff_append_to_array.rb
@@ -14,7 +14,7 @@ class parent::child inherits parent {
agents.each do |host|
apply_manifest_on(host, manifest) do
- assert_match(/notice: parent array element/, stdout, "#{host}: parent missing")
- assert_match(/notice: child array element/, stdout, "#{host}: child missing")
+ assert_match(/parent array element/, stdout, "#{host}: parent missing")
+ assert_match(/child array element/, stdout, "#{host}: child missing")
end
end
diff --git a/acceptance/tests/language/resource_refs_with_nested_arrays.rb b/acceptance/tests/language/resource_refs_with_nested_arrays.rb
index ead35a3..5b11854 100644
--- a/acceptance/tests/language/resource_refs_with_nested_arrays.rb
+++ b/acceptance/tests/language/resource_refs_with_nested_arrays.rb
@@ -22,6 +22,6 @@
MANIFEST
apply_manifest_on agent, test_manifest do
- assert_match(/Exec\[third\].*the final command/, "#{stdout}")
+ assert_match(/Exec\[third\].*the final command/, stdout)
end
end
diff --git a/acceptance/tests/puppet_apply_basics.rb b/acceptance/tests/puppet_apply_basics.rb
index 23c57d8..c11546e 100644
--- a/acceptance/tests/puppet_apply_basics.rb
+++ b/acceptance/tests/puppet_apply_basics.rb
@@ -7,7 +7,7 @@
step "check that puppet apply displays notices"
agents.each do |host|
apply_manifest_on(host, "notice 'Hello World'") do
- assert_match(/notice:.*Hello World/, stdout, "#{host}: missing notice!")
+ assert_match(/Hello World/, stdout, "#{host}: missing notice!")
end
end
diff --git a/acceptance/tests/puppet_apply_should_show_a_notice.rb b/acceptance/tests/puppet_apply_should_show_a_notice.rb
index 757d29b..df3922f 100644
--- a/acceptance/tests/puppet_apply_should_show_a_notice.rb
+++ b/acceptance/tests/puppet_apply_should_show_a_notice.rb
@@ -2,6 +2,6 @@
agents.each do |host|
apply_manifest_on(host, "notice 'Hello World'") do
- assert_match(/notice: .*: Hello World/, stdout, "#{host}: the notice didn't show")
+ assert_match(/.*: Hello World/, stdout, "#{host}: the notice didn't show")
end
end
diff --git a/acceptance/tests/resource/cron/should_remove_cron.rb b/acceptance/tests/resource/cron/should_remove_cron.rb
index fa72f5a..0e8a30d 100755
--- a/acceptance/tests/resource/cron/should_remove_cron.rb
+++ b/acceptance/tests/resource/cron/should_remove_cron.rb
@@ -29,7 +29,7 @@
step "verify that crontab -l contains what you expected"
run_cron_on(host, :list, tmpuser) do
- assert_no_match(/\/bin\/true/, stdout, "Error: Found entry for #{tmpuser} on #{host}")
+ assert_no_match(/\/bin\/true/, stderr, "Error: Found entry for #{tmpuser} on #{host}")
end
step "remove the crontab file for that user"
diff --git a/acceptance/tests/resource/cron/should_remove_matching.rb b/acceptance/tests/resource/cron/should_remove_matching.rb
index f480867..9f6524d 100755
--- a/acceptance/tests/resource/cron/should_remove_matching.rb
+++ b/acceptance/tests/resource/cron/should_remove_matching.rb
@@ -26,7 +26,7 @@
step "Remove cron resource"
on(host, puppet_resource("cron", "bogus", "user=#{tmpuser}",
"command=/bin/true", "ensure=absent")) do
- assert_match(/bogus\D+ensure: removed/, stdout, "Removing cron entry failed for #{tmpuser} on #{host}")
+ assert_match(/bogus\D+ensure: removed/, stderr, "Removing cron entry failed for #{tmpuser} on #{host}")
end
step "verify that crontab -l contains what you expected"
diff --git a/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb b/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb
index 6cef569..a918f8c 100644
--- a/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb
+++ b/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb
@@ -16,7 +16,7 @@
step "query for all files, which should return nothing"
on(agent, puppet_resource('file'), :acceptable_exit_codes => [1]) do
- assert_match(%r{Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc}, stdout)
+ assert_match(%r{Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc}, stderr)
end
["/", "/etc"].each do |file|
diff --git a/acceptance/tests/resource/group/should_not_destoy_unexisting.rb b/acceptance/tests/resource/group/should_not_destoy_unexisting.rb
index 287bb0f..53147f0 100755
--- a/acceptance/tests/resource/group/should_not_destoy_unexisting.rb
+++ b/acceptance/tests/resource/group/should_not_destoy_unexisting.rb
@@ -10,6 +10,6 @@
step "verify that we don't remove the group when it doesn't exist"
on(agents, puppet_resource('group', name, 'ensure=absent')) do
fail_test "it looks like we tried to remove the group" if
- stdout.include? "notice: /Group[#{name}]/ensure: removed"
+ stdout.include? "/Group[#{name}]/ensure: removed"
end
diff --git a/acceptance/tests/resource/host/should_not_create_existing.rb b/acceptance/tests/resource/host/should_not_create_existing.rb
index 2f2161c..89a052b 100755
--- a/acceptance/tests/resource/host/should_not_create_existing.rb
+++ b/acceptance/tests/resource/host/should_not_create_existing.rb
@@ -10,7 +10,7 @@
on(agent, puppet_resource('host', 'test', "target=#{file}",
'ensure=present', 'ip=127.0.0.2', 'host_aliases=alias')) do
fail_test "darn, we created the host record" if
- stdout.include? 'notice: /Host[test1]/ensure: created'
+ stdout.include? '/Host[test1]/ensure: created'
end
step "clean up after we created things"
diff --git a/acceptance/tests/resource/host/ticket_4131_should_not_create_without_ip.rb b/acceptance/tests/resource/host/ticket_4131_should_not_create_without_ip.rb
index 3926784..5d88c83 100755
--- a/acceptance/tests/resource/host/ticket_4131_should_not_create_without_ip.rb
+++ b/acceptance/tests/resource/host/ticket_4131_should_not_create_without_ip.rb
@@ -13,7 +13,7 @@
on(agent, puppet_resource('host', 'test', "target=#{file}",
"host_aliases=alias")) do
fail_test "puppet didn't complain about the missing attribute" unless
- stdout.include? 'ip is a required attribute for hosts'
+ stderr.include? 'ip is a required attribute for hosts'
end
step "verify that the host was not added to the file"
diff --git a/acceptance/tests/ticket_3961_puppet_ca_should_produce_certs.rb b/acceptance/tests/ticket_3961_puppet_ca_should_produce_certs.rb
index 3f41aeb..d6871cb 100644
--- a/acceptance/tests/ticket_3961_puppet_ca_should_produce_certs.rb
+++ b/acceptance/tests/ticket_3961_puppet_ca_should_produce_certs.rb
@@ -2,11 +2,11 @@
target = "working3961.example.org"
-expect = ['notice: Signed certificate request for ca',
- 'notice: Rebuilding inventory file',
- 'notice: working3961.example.org has a waiting certificate request',
- 'notice: Signed certificate request for working3961.example.org',
- 'notice: Removing file Puppet::SSL::CertificateRequest working3961.example.org']
+expect = ['Signed certificate request for ca',
+ 'Rebuilding inventory file',
+ 'working3961.example.org has a waiting certificate request',
+ 'Signed certificate request for working3961.example.org',
+ 'Removing file Puppet::SSL::CertificateRequest working3961.example.org']
agents.each do |agent|
if agent['platform'].include?('windows')
diff --git a/acceptance/tests/ticket_4233_resource_with_a_newline.rb b/acceptance/tests/ticket_4233_resource_with_a_newline.rb
index 61da2f7..b2e3745 100644
--- a/acceptance/tests/ticket_4233_resource_with_a_newline.rb
+++ b/acceptance/tests/ticket_4233_resource_with_a_newline.rb
@@ -12,6 +12,6 @@
agents.each do |host|
resource = host.echo('-e "\nHello World\n"')
apply_manifest_on(host, "exec { '#{resource}': }") do
- assert_match(/notice:.*Hello World.*success/, stdout)
+ assert_match(/Hello World.*success/, stdout)
end
end
diff --git a/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb b/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb
index f5a1c16..5328063 100644
--- a/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb
+++ b/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb
@@ -9,6 +9,6 @@
agents.each do |host|
apply_manifest_on(host, manifest) do
- assert_match(/notice: No issue here.../, stdout, "didn't get the expected notice on #{host}")
+ assert_match(/No issue here.../, stdout, "didn't get the expected notice on #{host}")
end
end
diff --git a/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb b/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb
index aa42fd4..18aca2b 100644
--- a/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb
+++ b/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb
@@ -18,6 +18,6 @@ class foo {
agents.each do |host|
apply_manifest_on(host, manifest) do
- assert_match(/notice.*?Foo::Do_notify.*?a_message_for_you/, stdout, "the notification didn't show up in stdout on #{host}")
+ assert_match(/.*?Foo::Do_notify.*?a_message_for_you/, stdout, "the notification didn't show up in stdout on #{host}")
end
end
diff --git a/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb b/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
index 7141bdc..3613edd 100644
--- a/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
+++ b/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
@@ -7,7 +7,7 @@
step "Master: use --parseonly on an invalid manifest, should return 1 and issue deprecation warning"
on master, puppet_master( %w{--parseonly /tmp/bad.pp} ), :acceptable_exit_codes => [ 1 ]
- assert_match(/--parseonly has been removed. Please use \'puppet parser validate <manifest>\'/, stdout, "Deprecation warning not issued for --parseonly on #{master}" )
+ assert_match(/--parseonly has been removed. Please use \'puppet parser validate <manifest>\'/, stderr, "Deprecation warning not issued for --parseonly on #{master}" )
step "Agents: create valid, invalid formatted manifests"
agents.each do |host|
@@ -19,7 +19,7 @@
step "Agents: use --parseonly on an invalid manifest, should return 1 and issue deprecation warning"
on(host, puppet('apply', '--parseonly', bad), :acceptable_exit_codes => [ 1 ]) do
- assert_match(/--parseonly has been removed. Please use \'puppet parser validate <manifest>\'/, stdout, "Deprecation warning not issued for --parseonly on #{host}" )
+ assert_match(/--parseonly has been removed. Please use \'puppet parser validate <manifest>\'/, stderr, "Deprecation warning not issued for --parseonly on #{host}" )
end
step "Test Face for 'parser validate' with good manifest -- should pass"
@@ -27,6 +27,6 @@
step "Test Faces for 'parser validate' with bad manifest -- should fail"
on(host, puppet('parser', 'validate', bad), :acceptable_exit_codes => [ 1 ]) do
- assert_match(/err: Could not parse for environment production/, stdout, "Bad manifest detection failed on #{host}" )
+ assert_match(/err: Could not parse for environment production/, stderr, "Bad manifest detection failed on #{host}" )
end
end
diff --git a/acceptance/tests/ticket_7728_don't_log_whits_on_failure.rb b/acceptance/tests/ticket_7728_don't_log_whits_on_failure.rb
index eaa95ff..c5fe7e4 100644
--- a/acceptance/tests/ticket_7728_don't_log_whits_on_failure.rb
+++ b/acceptance/tests/ticket_7728_don't_log_whits_on_failure.rb
@@ -12,7 +12,7 @@ class foo {
}
apply_manifest_on(agents, manifest) do
- assert_match(Regexp.new(Regexp.quote('notice: /Stage[main]/Foo/Notify[after]: Dependency Exec[test] has failures: true')), stdout, "the after dependency must be reported")
+ assert_match(Regexp.new(Regexp.quote('/Stage[main]/Foo/Notify[after]: Dependency Exec[test] has failures: true')), stdout, "the after dependency must be reported")
assert_no_match(Regexp.new(Regexp.quote('Class[Foo]')), stdout, 'the class should not be mentioned')
assert_no_match(Regexp.new(Regexp.quote('Stage[Main]')), stdout, 'the class should not be mentioned')
end
diff --git a/lib/puppet/application/module.rb b/lib/puppet/application/module.rb
index e107660..8526974 100644
--- a/lib/puppet/application/module.rb
+++ b/lib/puppet/application/module.rb
@@ -1,11 +1,4 @@
require 'puppet/application/face_base'
class Puppet::Application::Module < Puppet::Application::FaceBase
- def setup
- super
- if self.render_as.name == :console
- Puppet::Util::Log.close(:console)
- Puppet::Util::Log.newdestination(:telly_prototype_console)
- end
- end
end
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 4dc1da4..22fcb36 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -94,24 +94,6 @@ def handle(msg)
def initialize
# Flush output immediately.
- $stdout.sync = true
- end
-
- def handle(msg)
- if msg.source == "Puppet"
- puts colorize(msg.level, "#{msg.level}: #{msg}")
- else
- puts colorize(msg.level, "#{msg.level}: #{msg.source}: #{msg}")
- end
- end
-end
-
-Puppet::Util::Log.newdesttype :telly_prototype_console do
- require 'puppet/util/colors'
- include Puppet::Util::Colors
-
- def initialize
- # Flush output immediately.
$stderr.sync = true
$stdout.sync = true
end
@@ -126,6 +108,7 @@ def handle(msg)
}
str = msg.respond_to?(:multiline) ? msg.multiline : msg.to_s
+ str = msg.source == "Puppet" ? str : "#{msg.source}: #{str}"
case msg.level
when *error_levels.keys
diff --git a/spec/unit/application/facts_spec.rb b/spec/unit/application/facts_spec.rb
index 00f79fb..1c4e832 100755
--- a/spec/unit/application/facts_spec.rb
+++ b/spec/unit/application/facts_spec.rb
@@ -11,7 +11,7 @@
subject.command_line.stubs(:args).returns %w{find}
expect {
expect { subject.run }.to exit_with 1
- }.to have_printed /err: puppet facts find takes 1 argument, but you gave 0/
+ }.to have_printed /Error: puppet facts find takes 1 argument, but you gave 0/
@logs.first.to_s.should =~ /puppet facts find takes 1 argument, but you gave 0/
end
diff --git a/spec/unit/util/log/destinations_spec.rb b/spec/unit/util/log/destinations_spec.rb
index b037e84..d6a9c7a 100755
--- a/spec/unit/util/log/destinations_spec.rb
+++ b/spec/unit/util/log/destinations_spec.rb
@@ -108,6 +108,8 @@
end
describe Puppet::Util::Log.desttypes[:console] do
+ let (:klass) { Puppet::Util::Log.desttypes[:console] }
+
describe "when color is available" do
before :each do
Puppet.features.stubs(:ansicolor?).returns(true)
@@ -133,34 +135,14 @@
vstring = subject.colorize(:reset, 'version')
subject.colorize(:green, "(#{vstring})").should == "\e[0;32m(\e[mversion\e[0;32m)\e[0m"
end
- end
-end
-
-describe Puppet::Util::Log.desttypes[:telly_prototype_console] do
- describe "when color is available" do
- before :each do
- Puppet.features.stubs(:ansicolor?).returns(true)
- end
- it "should support color output" do
- Puppet[:color] = true
- subject.colorize(:red, 'version').should == "\e[0;31mversion\e[0m"
- end
- it "should withhold color output when not appropriate" do
+ it "should include the log message's source/context in the output when available" do
Puppet[:color] = false
- subject.colorize(:red, 'version').should == "version"
- end
-
- it "should handle multiple overlapping colors in a stack-like way" do
- Puppet[:color] = true
- vstring = subject.colorize(:red, 'version')
- subject.colorize(:green, "(#{vstring})").should == "\e[0;32m(\e[0;31mversion\e[0;32m)\e[0m"
- end
+ $stdout.expects(:puts).with("Info: a hitchhiker: don't panic")
- it "should handle resets in a stack-like way" do
- Puppet[:color] = true
- vstring = subject.colorize(:reset, 'version')
- subject.colorize(:green, "(#{vstring})").should == "\e[0;32m(\e[mversion\e[0;32m)\e[0m"
+ msg = Puppet::Util::Log.new(:level => :info, :message => "don't panic", :source => "a hitchhiker")
+ dest = klass.new
+ dest.handle(msg)
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.
