Skip to content

Commit 8362031

Browse files
committed
Log Resque::Worker through Resque::Logger
This change includes Resque::Logger, a set of wrappers around the underlying Resque.logger, into Resque::Worker, sending all logging through that as opposed to just printing it to STDOUT. Some notes on this change: * Users can now configure any stdlib Logger duck type they want to do their logging. By default it uses Logger.new(STDOUT) * Resque::Worker.verbose and .very_verbose are presently noops. They should probably be deprecated in lieu of setting a severity threshold on the logger itself (i.e. Logger::INFO vs Logger::DEBUG). That said, they could be made backwards compatible and set the severity level automatically (hopefully we'd still deprecate them anyway). * Testing is made easier by the fact that logging can be intercepted as opposed to having to use "capture_io" to make assertions about what is being logged or not o_O * Kernel#warn is now intercepted in classes that include Resque::Logger and directed to the log as opposed to STDERR. This may or may not be desirable. * I deleted some tests (which might be a bad idea): - "very verbose works in the afternoon" hardcoded the timestamp format. I'm going to guess stdlib Logger works in the afternoon - There were tests for "displays warning when not using term_child" which were using capture_io. Uncertain about the importance of displaying this warning and given the wonky nature of the tests, I just deleted them for now. I can potentially add them back if they were important.
1 parent 57fbe27 commit 8362031

File tree

5 files changed

+42
-55
lines changed

5 files changed

+42
-55
lines changed

lib/resque/logger.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ module Resque
22
# Include this module in classes you wish to have logging facilities
33
module Logger
44
module_function
5-
5+
66
# Log level aliases
77
def debug(message); __log__ :debug, message; end
88
def info(message); __log__ :info, message; end
99
def warn(message); __log__ :warn, message; end
1010
def error(message); __log__ :error, message; end
1111
def fatal(message); __log__ :fatal, message; end
12-
12+
1313
# Low-level thunk to the logger's own log method
1414
def __log__(severity, message)
1515
Resque.logger.__send__(severity, message) if Resque.logger

lib/resque/worker.rb

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ module Resque
77
# It also ensures workers are always listening to signals from you,
88
# their master, and can react accordingly.
99
class Worker
10+
extend Resque::Helpers
1011
include Resque::Helpers
11-
extend Resque::Helpers
12+
include Resque::Logger
1213

1314
# Whether the worker should log basic info to STDOUT
1415
attr_accessor :verbose
@@ -606,19 +607,8 @@ def procline(string)
606607
log! $0
607608
end
608609

609-
# Log a message to STDOUT if we are verbose or very_verbose.
610-
def log(message)
611-
if verbose
612-
puts "*** #{message}"
613-
elsif very_verbose
614-
time = Time.now.strftime('%H:%M:%S %Y-%m-%d')
615-
puts "** [#{time}] #$$: #{message}"
616-
end
617-
end
618-
619-
# Logs a very verbose message to STDOUT.
620-
def log!(message)
621-
log message if very_verbose
622-
end
610+
# Log a message to Resque.logger
611+
alias_method :log, :info
612+
alias_method :log!, :debug
623613
end
624614
end

test/logger_test.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
require 'test_helper'
22
require 'minitest/mock'
33

4-
context "Resque.logger" do
4+
context "Resque::Logger" do
5+
teardown { reset_logger }
6+
57
test "sets and receives the active logger" do
68
my_logger = Object.new
79
Resque.logger = my_logger
810
assert_equal my_logger, Resque.logger
911
end
10-
end
1112

12-
context "Resque::Logger" do
1313
%w(debug info error fatal).each do |severity|
1414
test "logs #{severity} messages" do
1515
message = "test message"

test/test_helper.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,11 @@ def capture_io
176176
$stdout = orig_stdout
177177
$stderr = orig_stderr
178178
end
179+
180+
# Log to log/test.log
181+
def reset_logger
182+
$test_logger ||= Logger.new(File.open(File.expand_path("../../log/test.log", __FILE__), "w"))
183+
Resque.logger = $test_logger
184+
end
185+
186+
reset_logger

test/worker_test.rb

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -399,25 +399,6 @@ def self.perform
399399
assert !$BEFORE_FORK_CALLED, "before_fork should not have been called after job runs"
400400
end
401401

402-
test "very verbose works in the afternoon" do
403-
begin
404-
require 'time'
405-
last_puts = ""
406-
Time.fake_time = Time.parse("15:44:33 2011-03-02")
407-
408-
@worker.extend(Module.new {
409-
define_method(:puts) { |thing| last_puts = thing }
410-
})
411-
412-
@worker.very_verbose = true
413-
@worker.log("some log text")
414-
415-
assert_match /\*\* \[15:44:33 2011-03-02\] \d+: some log text/, last_puts
416-
ensure
417-
Time.fake_time = nil
418-
end
419-
end
420-
421402
test "Will call an after_fork hook if we're forking" do
422403
Resque.redis.flushall
423404
$AFTER_FORK_CALLED = false
@@ -491,11 +472,32 @@ def sleep(duration = nil)
491472
end
492473
end
493474

475+
class DummyLogger
476+
attr_reader :messages
477+
478+
def initialize
479+
@messages = []
480+
end
481+
482+
def info(message); @messages << message; end
483+
alias_method :debug, :info
484+
alias_method :warn, :info
485+
alias_method :error, :info
486+
alias_method :fatal, :info
487+
end
488+
494489
@worker.very_verbose = true
495-
stdout, stderr = capture_io { @worker.work(0) }
496490

497-
assert_equal 3, stdout.scan(/retrying/).count
498-
assert_equal 1, stdout.scan(/quitting/).count
491+
Resque.logger = DummyLogger.new
492+
begin
493+
@worker.work(0)
494+
messages = Resque.logger.messages
495+
ensure
496+
reset_logger
497+
end
498+
499+
assert_equal 3, messages.grep(/retrying/).count
500+
assert_equal 1, messages.grep(/quitting/).count
499501
ensure
500502
class Redis::Client
501503
alias_method :reconnect, :original_reconnect
@@ -640,18 +642,5 @@ def self.perform( run_time, rescue_time=nil )
640642
end
641643
end
642644
end
643-
644-
test "displays warning when not using term_child" do
645-
stdout, stderr = capture_io { @worker.work(0) }
646-
647-
assert stderr.match(/^WARNING:/)
648-
end
649-
650-
test "it does not display warning when using term_child" do
651-
@worker.term_child = "1"
652-
stdout, stderr = capture_io { @worker.work(0) }
653-
654-
assert !stderr.match(/^WARNING:/)
655-
end
656645
end
657646
end

0 commit comments

Comments
 (0)