Allow `yield` to logger methods & bail early on no-op messages (#6315)

Merge pull request 6315
This commit is contained in:
Parker Moore 2017-08-18 12:45:23 -04:00 committed by jekyllbot
parent 08840946f3
commit 232ec4679a
5 changed files with 78 additions and 38 deletions

View File

@ -2,7 +2,7 @@
module Jekyll module Jekyll
class LogAdapter class LogAdapter
attr_reader :writer, :messages attr_reader :writer, :messages, :level
LOG_LEVELS = { LOG_LEVELS = {
:debug => ::Logger::DEBUG, :debug => ::Logger::DEBUG,
@ -30,6 +30,7 @@ module Jekyll
# Returns nothing # Returns nothing
def log_level=(level) def log_level=(level)
writer.level = LOG_LEVELS.fetch(level) writer.level = LOG_LEVELS.fetch(level)
@level = level
end end
def adjust_verbosity(options = {}) def adjust_verbosity(options = {})
@ -48,8 +49,8 @@ module Jekyll
# message - the message detail # message - the message detail
# #
# Returns nothing # Returns nothing
def debug(topic, message = nil) def debug(topic, message = nil, &block)
writer.debug(message(topic, message)) write(:debug, topic, message, &block)
end end
# Public: Print a message # Public: Print a message
@ -58,8 +59,8 @@ module Jekyll
# message - the message detail # message - the message detail
# #
# Returns nothing # Returns nothing
def info(topic, message = nil) def info(topic, message = nil, &block)
writer.info(message(topic, message)) write(:info, topic, message, &block)
end end
# Public: Print a message # Public: Print a message
@ -68,8 +69,8 @@ module Jekyll
# message - the message detail # message - the message detail
# #
# Returns nothing # Returns nothing
def warn(topic, message = nil) def warn(topic, message = nil, &block)
writer.warn(message(topic, message)) write(:warn, topic, message, &block)
end end
# Public: Print an error message # Public: Print an error message
@ -78,8 +79,8 @@ module Jekyll
# message - the message detail # message - the message detail
# #
# Returns nothing # Returns nothing
def error(topic, message = nil) def error(topic, message = nil, &block)
writer.error(message(topic, message)) write(:error, topic, message, &block)
end end
# Public: Print an error message and immediately abort the process # Public: Print an error message and immediately abort the process
@ -88,8 +89,8 @@ module Jekyll
# message - the message detail (can be omitted) # message - the message detail (can be omitted)
# #
# Returns nothing # Returns nothing
def abort_with(topic, message = nil) def abort_with(topic, message = nil, &block)
error(topic, message) error(topic, message, &block)
abort abort
end end
@ -99,19 +100,48 @@ module Jekyll
# message - the message detail # message - the message detail
# #
# Returns the formatted message # Returns the formatted message
def message(topic, message) def message(topic, message = nil)
msg = formatted_topic(topic) + message.to_s.gsub(%r!\s+!, " ") raise ArgumentError, "block or message, not both" if block_given? && message
messages << msg
msg message = yield if block_given?
message = message.to_s.gsub(%r!\s+!, " ")
topic = formatted_topic(topic, block_given?)
out = topic + message
messages << out
out
end end
# Internal: Format the topic # Internal: Format the topic
# #
# topic - the topic of the message, e.g. "Configuration file", "Deprecation", etc. # topic - the topic of the message, e.g. "Configuration file", "Deprecation", etc.
# colon -
# #
# Returns the formatted topic statement # Returns the formatted topic statement
def formatted_topic(topic) def formatted_topic(topic, colon = false)
"#{topic} ".rjust(20) "#{topic}#{colon ? ": " : " "}".rjust(20)
end
# Internal: Check if the message should be written given the log level.
#
# level_of_message - the Symbol level of message, one of :debug, :info, :warn, :error
#
# Returns whether the message should be written.
def write_message?(level_of_message)
LOG_LEVELS.fetch(level) <= LOG_LEVELS.fetch(level_of_message)
end
# Internal: Log a message.
#
# level_of_message - the Symbol level of message, one of :debug, :info, :warn, :error
# topic - the String topic or full message
# message - the String message (optional)
# block - a block containing the message (optional)
#
# Returns false if the message was not written, otherwise returns the value of calling
# the appropriate writer method, e.g. writer.info.
def write(level_of_message, topic, message = nil, &block)
return false unless write_message?(level_of_message)
writer.public_send(level_of_message, message(topic, message, &block))
end end
end end
end end

View File

@ -32,7 +32,7 @@ require "minitest/profile"
require "rspec/mocks" require "rspec/mocks"
require_relative "../lib/jekyll.rb" require_relative "../lib/jekyll.rb"
Jekyll.logger = Logger.new(StringIO.new) Jekyll.logger = Logger.new(StringIO.new, :error)
unless jruby? unless jruby?
require "rdiscount" require "rdiscount"
@ -168,12 +168,15 @@ class JekyllUnitTest < Minitest::Test
ENV[key] = old_value ENV[key] = old_value
end end
def capture_output def capture_output(level = :debug)
buffer = StringIO.new buffer = StringIO.new
Jekyll.logger = Logger.new(buffer) Jekyll.logger = Logger.new(buffer)
Jekyll.logger.log_level = level
yield yield
buffer.rewind buffer.rewind
buffer.string.to_s buffer.string.to_s
ensure
Jekyll.logger = Logger.new(StringIO.new, :error)
end end
alias_method :capture_stdout, :capture_output alias_method :capture_stdout, :capture_output
alias_method :capture_stderr, :capture_output alias_method :capture_stderr, :capture_output

View File

@ -53,7 +53,7 @@ class TestLogAdapter < JekyllUnitTest
should "call #debug on writer return true" do should "call #debug on writer return true" do
writer = LoggerDouble.new writer = LoggerDouble.new
logger = Jekyll::LogAdapter.new(writer) logger = Jekyll::LogAdapter.new(writer, :debug)
allow(writer).to receive(:debug).and_return(true) allow(writer).to receive(:debug).and_return(true)
assert logger.adjust_verbosity assert logger.adjust_verbosity
end end
@ -62,7 +62,7 @@ class TestLogAdapter < JekyllUnitTest
context "#debug" do context "#debug" do
should "call #debug on writer return true" do should "call #debug on writer return true" do
writer = LoggerDouble.new writer = LoggerDouble.new
logger = Jekyll::LogAdapter.new(writer) logger = Jekyll::LogAdapter.new(writer, :debug)
allow(writer).to receive(:debug) allow(writer).to receive(:debug)
.with("topic ".rjust(20) + "log message").and_return(true) .with("topic ".rjust(20) + "log message").and_return(true)
assert logger.debug("topic", "log message") assert logger.debug("topic", "log message")
@ -72,7 +72,7 @@ class TestLogAdapter < JekyllUnitTest
context "#info" do context "#info" do
should "call #info on writer return true" do should "call #info on writer return true" do
writer = LoggerDouble.new writer = LoggerDouble.new
logger = Jekyll::LogAdapter.new(writer) logger = Jekyll::LogAdapter.new(writer, :info)
allow(writer).to receive(:info) allow(writer).to receive(:info)
.with("topic ".rjust(20) + "log message").and_return(true) .with("topic ".rjust(20) + "log message").and_return(true)
assert logger.info("topic", "log message") assert logger.info("topic", "log message")
@ -82,7 +82,7 @@ class TestLogAdapter < JekyllUnitTest
context "#warn" do context "#warn" do
should "call #warn on writer return true" do should "call #warn on writer return true" do
writer = LoggerDouble.new writer = LoggerDouble.new
logger = Jekyll::LogAdapter.new(writer) logger = Jekyll::LogAdapter.new(writer, :warn)
allow(writer).to receive(:warn) allow(writer).to receive(:warn)
.with("topic ".rjust(20) + "log message").and_return(true) .with("topic ".rjust(20) + "log message").and_return(true)
assert logger.warn("topic", "log message") assert logger.warn("topic", "log message")
@ -92,7 +92,7 @@ class TestLogAdapter < JekyllUnitTest
context "#error" do context "#error" do
should "call #error on writer return true" do should "call #error on writer return true" do
writer = LoggerDouble.new writer = LoggerDouble.new
logger = Jekyll::LogAdapter.new(writer) logger = Jekyll::LogAdapter.new(writer, :error)
allow(writer).to receive(:error) allow(writer).to receive(:error)
.with("topic ".rjust(20) + "log message").and_return(true) .with("topic ".rjust(20) + "log message").and_return(true)
assert logger.error("topic", "log message") assert logger.error("topic", "log message")
@ -101,7 +101,7 @@ class TestLogAdapter < JekyllUnitTest
context "#abort_with" do context "#abort_with" do
should "call #error and abort" do should "call #error and abort" do
logger = Jekyll::LogAdapter.new(LoggerDouble.new) logger = Jekyll::LogAdapter.new(LoggerDouble.new, :error)
allow(logger).to receive(:error).with("topic", "log message").and_return(true) allow(logger).to receive(:error).with("topic", "log message").and_return(true)
assert_raises(SystemExit) { logger.abort_with("topic", "log message") } assert_raises(SystemExit) { logger.abort_with("topic", "log message") }
end end
@ -113,7 +113,7 @@ class TestLogAdapter < JekyllUnitTest
end end
should "store each log value in the array" do should "store each log value in the array" do
logger = Jekyll::LogAdapter.new(LoggerDouble.new) logger = Jekyll::LogAdapter.new(LoggerDouble.new, :debug)
values = %w(one two three four) values = %w(one two three four)
logger.debug(values[0]) logger.debug(values[0])
logger.info(values[1]) logger.info(values[1])
@ -122,4 +122,14 @@ class TestLogAdapter < JekyllUnitTest
assert_equal values.map { |value| "#{value} ".rjust(20) }, logger.messages assert_equal values.map { |value| "#{value} ".rjust(20) }, logger.messages
end end
end end
context "#write_message?" do
should "return false up to the desired logging level" do
subject = Jekyll::LogAdapter.new(LoggerDouble.new, :warn)
refute subject.write_message?(:debug), "Should not print debug messages"
refute subject.write_message?(:info), "Should not print info messages"
assert subject.write_message?(:warn), "Should print warn messages"
assert subject.write_message?(:error), "Should print error messages"
end
end
end end

View File

@ -22,7 +22,7 @@ class TestNewCommand < JekyllUnitTest
end end
teardown do teardown do
FileUtils.rm_r @full_path FileUtils.rm_r @full_path if File.directory?(@full_path)
end end
should "create a new directory" do should "create a new directory" do
@ -41,8 +41,7 @@ class TestNewCommand < JekyllUnitTest
end end
should "display a success message" do should "display a success message" do
Jekyll::Commands::New.process(@args) output = capture_output { Jekyll::Commands::New.process(@args) }
output = Jekyll.logger.messages
success_message = "New jekyll site installed in #{@full_path.cyan}. " success_message = "New jekyll site installed in #{@full_path.cyan}. "
bundle_message = "Running bundle install in #{@full_path.cyan}... " bundle_message = "Running bundle install in #{@full_path.cyan}... "
assert_includes output, success_message assert_includes output, success_message
@ -88,8 +87,7 @@ class TestNewCommand < JekyllUnitTest
should "create blank project" do should "create blank project" do
blank_contents = %w(/_drafts /_layouts /_posts /index.html) blank_contents = %w(/_drafts /_layouts /_posts /index.html)
capture_output { Jekyll::Commands::New.process(@args, "--blank") } output = capture_output { Jekyll::Commands::New.process(@args, "--blank") }
output = Jekyll.logger.messages.last
bundle_message = "Running bundle install in #{@full_path.cyan}..." bundle_message = "Running bundle install in #{@full_path.cyan}..."
assert_same_elements blank_contents, dir_contents(@full_path) assert_same_elements blank_contents, dir_contents(@full_path)
refute_includes output, bundle_message refute_includes output, bundle_message
@ -98,12 +96,11 @@ class TestNewCommand < JekyllUnitTest
should "force created folder" do should "force created folder" do
capture_output { Jekyll::Commands::New.process(@args) } capture_output { Jekyll::Commands::New.process(@args) }
output = capture_output { Jekyll::Commands::New.process(@args, "--force") } output = capture_output { Jekyll::Commands::New.process(@args, "--force") }
assert_match(%r!New jekyll site installed in!, output) assert_match %r!New jekyll site installed in!, output
end end
should "skip bundle install when opted to" do should "skip bundle install when opted to" do
capture_output { Jekyll::Commands::New.process(@args, "--skip-bundle") } output = capture_output { Jekyll::Commands::New.process(@args, "--skip-bundle") }
output = Jekyll.logger.messages.last
bundle_message = "Bundle install skipped." bundle_message = "Bundle install skipped."
assert_includes output, bundle_message assert_includes output, bundle_message
end end

View File

@ -562,13 +562,13 @@ class TestSite < JekyllUnitTest
end end
expected_msg = "Theme: value of 'theme' in config should be String " \ expected_msg = "Theme: value of 'theme' in config should be String " \
"to use gem-based themes, but got Hash\n" "to use gem-based themes, but got Hash\n"
assert output.end_with?(expected_msg), assert_includes output, expected_msg
"Expected #{output.inspect} to end with #{expected_msg.inspect}"
end end
should "set a theme if the config is a string" do should "set a theme if the config is a string" do
expect($stderr).not_to receive(:puts) [:debug, :info, :warn, :error].each do |level|
expect($stdout).not_to receive(:puts) expect(Jekyll.logger.writer).not_to receive(level)
end
site = fixture_site({ "theme" => "test-theme" }) site = fixture_site({ "theme" => "test-theme" })
assert_instance_of Jekyll::Theme, site.theme assert_instance_of Jekyll::Theme, site.theme
assert_equal "test-theme", site.theme.name assert_equal "test-theme", site.theme.name