Skip to content

Bug/master/13559 two console output styles enter one style leaves #761

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.

Sign up for GitHub

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

Merged
slippycheeze merged 3 commits into from
May 11, 2012
Merged

Bug/master/13559 two console output styles enter one style leaves #761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a020b90
Fix acceptance test assumption about exec logging.
May 8, 2012
537343d
(#13559) Promote new console output prototype to official.
Apr 19, 2012
a645639
(#13559) added resource context to log messages when available
cprice404 May 11, 2012
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
2 changes: 1 addition & 1 deletion acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/allow_arbitrary_node_name_for_apply.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
exec { "false": }
}
node a_different_node_name {
exec { "echo #{success_message}": }
exec { "echo #{success_message}": logoutput => true }
}
]

Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/apply/hashes/should_not_reassign.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions acceptance/tests/cycle_detection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
3 changes: 1 addition & 2 deletions acceptance/tests/file_hello_world.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,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"
assert_match("File[#{filename}]/ensure: defined content as", stdout, "the expected notice of action was missing")
end

step "verify the content of the generated files."
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/helpful_error_message_when_hostname_not_match_server_certificate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions acceptance/tests/jeff_append_to_array.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion acceptance/tests/language/resource_refs_with_nested_arrays.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion acceptance/tests/puppet_apply_basics.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/puppet_apply_should_show_a_notice.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion acceptance/tests/resource/cron/should_remove_cron.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/resource/group/should_not_destoy_unexisting.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion acceptance/tests/resource/host/should_not_create_existing.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/resource/host/ticket_4131_should_not_create_without_ip.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 5 additions & 5 deletions acceptance/tests/ticket_3961_puppet_ca_should_produce_certs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/ticket_4233_resource_with_a_newline.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion ...ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(/Error: Could not parse for environment production/, stderr, "Bad manifest detection failed on #{host}" )
end
end
2 changes: 1 addition & 1 deletion acceptance/tests/ticket_7728_don't_log_whits_on_failure.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 0 additions & 7 deletions lib/puppet/application/module.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
19 changes: 1 addition & 18 deletions lib/puppet/util/log/destinations.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,24 +92,6 @@ def handle(msg)
require 'puppet/util/colors'
include Puppet::Util::Colors

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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/application/facts_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 7 additions & 26 deletions spec/unit/util/log/destinations_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
subject.stubs(:console_has_color?).returns(true)
Expand All @@ -134,35 +136,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
subject.stubs(:console_has_color?).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